[201_63] 文本选中悬浮框拆分-功能初步实现 by Yuki-Nagori · Pull Request #2943 · MoganLab/mogan

Yuki-Nagori · 2026-03-06T04:08:10Z

- Remove Q_INIT_RESOURCE(images) in QTMTextToolbar constructor (images resource not used in PR1 framework) - Remove get_shortcut_suffix declaration from edit_interface.hpp (implementation not found, appears to be split residue) These cleanups ensure PR1 only contains necessary code for the text toolbar framework without unused remnants.

1. Add caching for should_show_text_toolbar(): - Cache result for 100ms to reduce Scheme call overhead - Store last_check time and last_result in class fields 2. Eliminate redundant selection_active_any() calls: - Merge checks in get_text_selection_rect() - Single check at function start 3. Simplify update_text_toolbar(): - Remove redundant min/max calculations - Early return for invalid rectangles - Cleaner control flow Performance impact: Reduces 4 Scheme calls per mouse move to 1 call per 100ms when cursor is in same context.

Add documentation for the performance improvements: - Cache mechanism for should_show_text_toolbar() - Elimination of redundant selection_active_any() calls - Simplification of update_text_toolbar() calculations - Code cleanup (removed unused declarations) Following devel/x_y.md format with What/Why/How sections.

Translate all English comments added during optimization to Chinese: - 缓存结果100ms，避免过多的Scheme调用 - 单次检查selection_active_any，避免重复调用 - 使用原始坐标检查选区是否在视图内 - 文本工具栏缓存，用于性能优化保持代码与项目中其他注释语言风格一致。

Address code review feedback: 1. Replace static_cast with dynamic_cast for safer type conversion - Add null checks after dynamic_cast in show/hide/is_point functions 2. Add invalidate_text_toolbar_cache() method for cache invalidation - Allows immediate cache reset when selection changes 3. Add unit tests in tests/Edit/Interface/text_toolbar_test.cpp - Test cache mechanism and invalidation - Test coordinate conversion consistency - Test rectangle validation Note: ./bin/format skipped (elvish not available) Test: tests/Edit/Interface/text_toolbar_test.cpp Related: PR #2943

Address code review feedback: 1. **Unify coordinate conversion** (qt_utilities.hpp/cpp): - Add si_to_pixel(), si_to_qpoint(), si_to_qrect() helper functions - Replace repeated 'inv_unit = 1.0 / 256.0' patterns - Ensures consistent coordinate conversion across codebase 2. **Update qt_simple_widget.cpp**: - Use unified si_to_qpoint() in is_point_in_text_toolbar() 3. **Update devel/201_63.md**: - Add section for code quality improvements - Document coordinate unification changes 4. **Format code** with ./bin/format (clang-format-19) Test: tests/Edit/Interface/text_toolbar_test.cpp Related: PR #2943

Added cpp test instructions for text toolbar.

Add detailed test cases based on should_show_text_toolbar() logic: - List all conditions for showing/hiding toolbar - Manual test scenarios - Cache mechanism testing - Cpp test commands and coverage

Copilot

Pull request overview

This PR introduces an initial implementation of a “text selection floating toolbar” in the Qt frontend, wiring it into editor mouse/keyboard/scroll/selection updates and adding theme styling, docs, and a basic test scaffold.

Changes:

Add QTMTextToolbar Qt widget and integrate it into qt_simple_widget_rep (create/show/hide/scroll/hit-test).
Add editor-side logic in edit_interface_rep to decide when to show the toolbar, compute selection rect, and update toolbar on mouse/keyboard/selection changes.
Add theme CSS, development notes, and a new QtTest-based test file.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
tests/Edit/Interface/text_toolbar_test.cpp	Adds a new unit test scaffold for toolbar-related logic.
src/Plugins/Qt/qt_simple_widget.hpp	Declares text-toolbar support APIs and stores a `QPointer<QTMTextToolbar>`.
src/Plugins/Qt/qt_simple_widget.cpp	Implements create/show/hide/scroll/hit-test plumbing for the toolbar.
src/Plugins/Qt/QTMWidget.cpp	Updates toolbar position/state on scroll events.
src/Plugins/Qt/QTMTextToolbar.hpp	Introduces the new `QTMTextToolbar` widget interface and cached positioning fields.
src/Plugins/Qt/QTMTextToolbar.cpp	Implements toolbar widget creation, positioning, sizing, and visibility logic.
src/Edit/Interface/edit_mouse.cpp	Updates toolbar visibility on mouse move/press and adds toolbar-specific support methods.
src/Edit/Interface/edit_keyboard.cpp	Updates toolbar visibility after keyboard input.
src/Edit/Interface/edit_interface.hpp	Adds cache fields and declares toolbar support methods.
src/Edit/Interface/edit_interface.cpp	Triggers toolbar updates when selection changes are applied.
devel/201_63.md	Documents manual/automatic testing and design notes for the feature.
TeXmacs/misc/themes/liii.css	Adds light-theme styling for the toolbar widget/buttons.
TeXmacs/misc/themes/liii-night.css	Adds dark-theme styling for the toolbar widget/buttons.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-10T01:45:45Z

src/Plugins/Qt/qt_simple_widget.cpp

+  // 将逻辑坐标转换为像素坐标
+  double inv_unit= 1.0 / 256.0;
+  int    px      = int (std::round (x * inv_unit));
+  int    py      = int (std::round (y * inv_unit));
+
+  // 获取工具栏的几何位置
+  QRect toolbarRect= textToolbar->geometry ();
+
+  // 检查点是否在工具栏内
+  return toolbarRect.contains (px, py);
+}


is_point_in_text_toolbar converts document logical coordinates to pixels by dividing by 256, but it ignores scroll offsets, magnification (magf), canvas origin, and Y inversion (all of which are applied when positioning the toolbar). As a result, hit-testing will be incorrect (often always false), so clicks on the toolbar may hide it. Use the same logical->Qt coordinate transform as QTMTextToolbar::getCachedPosition/selectionInView (including scroll_x/scroll_y, canvas_x/canvas_y, magf, and blank-top correction), or delegate hit-testing to the toolbar using its cached transform.

Copilot · 2026-03-10T01:45:45Z

src/Plugins/Qt/QTMTextToolbar.cpp

+void
+QTMTextToolbar::showTextToolbar (qt_renderer_rep* ren, rectangle selr,
+                                 double magf, int scroll_x, int scroll_y,
+                                 int canvas_x, int canvas_y) {
+  cachePosition (selr, magf, scroll_x, scroll_y, canvas_x, canvas_y);
+  if (!selectionInView ()) {
+    hide ();
+    return;
+  }
+  updatePosition (ren);
+  show ();
+  raise ();
+}


QTMTextToolbar::showTextToolbar updates the cached magf in cachePosition, but it never calls autoSize(). Since autoSize() depends on cached_magf and also refreshes cached_width/cached_height, the toolbar size (and position clamping math) won't update correctly when zoom/magnification changes. Call autoSize() after caching (before updatePosition) or otherwise recompute cached_width/cached_height when magf changes.

Copilot · 2026-03-10T01:45:45Z

src/Plugins/Qt/QTMTextToolbar.cpp

+  // 添加阴影效果
+  QGraphicsDropShadowEffect* effect= new QGraphicsDropShadowEffect (this);
+  effect->setBlurRadius (40);
+  effect->setOffset (0, 4);
+  effect->setColor (QColor (0, 0, 0, 120));
+  this->setGraphicsEffect (effect);


In the constructor, a local effect variable shadows the effect member, leaving the member uninitialized. Either assign to this->effect (and remove the local) or remove the unused member from the header to avoid confusion and future misuse.

Copilot · 2026-03-10T01:45:45Z

src/Edit/Interface/edit_interface.cpp

    }
+    // 选区改变后更新文本工具栏
+    update_text_toolbar ();
  }


apply_changes() calls update_text_toolbar() on THE_SELECTION, but should_show_text_toolbar() caches its result for 100ms. Without invalidating the cache here, a selection change can be ignored until the cache expires, causing the toolbar to show/hide late. Call invalidate_text_toolbar_cache() (or otherwise force a recheck) before update_text_toolbar() when the selection changes.

Copilot · 2026-03-10T01:45:45Z

tests/Edit/Interface/text_toolbar_test.cpp

+#include "base.hpp"
+#include "edit_interface.hpp"
+#include <QtTest/QtTest>
+


This test uses std::round but doesn't include <cmath>, which is not guaranteed to be provided by the current includes. Add #include <cmath> (or another header that guarantees std::round) to avoid build failures on stricter toolchains.

Copilot · 2026-03-10T01:45:46Z

tests/Edit/Interface/text_toolbar_test.cpp

+void
+TestTextToolbar::test_should_show_text_toolbar_cache () {
+  // 测试缓存机制：连续调用应该返回相同结果
+  // 注意：这里使用模拟数据，实际测试需要完整的 edit_interface_rep 实例
+
+  // 验证缓存初始状态
+  time_t initial_check= 0;
+  QVERIFY (initial_check == 0);
+
+  // 模拟缓存更新
+  time_t now   = texmacs_time ();
+  initial_check= now;
+  QVERIFY (initial_check > 0);
+
+  // 模拟100ms内的重复调用应该使用缓存
+  // 实际测试中应该验证 should_show_text_toolbar() 的行为
+}
+
+void
+TestTextToolbar::test_invalidate_text_toolbar_cache () {
+  // 测试缓存失效机制
+  time_t cache_time= texmacs_time ();
+  QVERIFY (cache_time > 0);
+
+  // 模拟 invalidate_text_toolbar_cache() 的行为
+  cache_time= 0;
+  QVERIFY (cache_time == 0);
+
+  // 验证下次调用会重新计算
+}


The cache-related tests here don't exercise the production code (e.g., should_show_text_toolbar() / invalidate_text_toolbar_cache()), they only mutate local variables, so they won't catch regressions. Consider instantiating a minimal edit_interface_rep test double (or extracting the caching logic into a unit-testable helper) and asserting observable behavior (cache hit within 100ms, cache invalidation forces recompute).

Copilot · 2026-03-10T01:45:46Z

src/Plugins/Qt/qt_simple_widget.cpp

+qt_simple_widget_rep::show_text_toolbar (rectangle selr, double magf,
+                                         int scroll_x, int scroll_y,
+                                         int canvas_x, int canvas_y) {
+  ensure_text_toolbar ();
+  qt_renderer_rep* ren= the_qt_renderer ();
+  textToolbar->showTextToolbar (ren, selr, magf, scroll_x, scroll_y, canvas_x,
+                                canvas_y);
+}


show_text_toolbar assumes ensure_text_toolbar() always creates textToolbar, but ensure_text_toolbar() early-returns when canvas() is null. That can leave textToolbar null and cause a crash on textToolbar->showTextToolbar(...). Add a null check after ensure_text_toolbar() (or have ensure_text_toolbar() return a bool) and bail out if the toolbar can't be created.

MoonL79

LGTM

Yuki-Nagori and others added 7 commits March 6, 2026 13:20

文本选中悬浮框拆分-功能初步实现

e245196

wip

05f69c6

加回错误的删除（ tree get_shortcut_suffix (string cmd_s);）

68a8015

Yuki-Nagori force-pushed the yuki/201_63/floating-window/1 branch from bc39a41 to 68a8015 Compare March 6, 2026 05:32

Anon (千早爱音) and others added 2 commits March 6, 2026 13:49

format

6d190c3

wip

e56a44b

Yuki-Nagori force-pushed the yuki/201_63/floating-window/1 branch from 9177e1d to e56a44b Compare March 6, 2026 06:15

Yuki-Nagori and others added 2 commits March 6, 2026 14:24

Update testing instructions in 201_63.md

a77ad83

Added cpp test instructions for text toolbar.

docs: Expand testing instructions in 201_63.md

7a2fbdd

Add detailed test cases based on should_show_text_toolbar() logic: - List all conditions for showing/hiding toolbar - Manual test scenarios - Cache mechanism testing - Cpp test commands and coverage

Yuki-Nagori marked this pull request as ready for review March 6, 2026 06:36

MoonL79 requested a review from Copilot March 10, 2026 01:40

Copilot started reviewing on behalf of MoonL79 March 10, 2026 01:40 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

MoonL79 approved these changes Mar 13, 2026

View reviewed changes

MoonL79 merged commit a013562 into main Mar 13, 2026
8 checks passed

MoonL79 deleted the yuki/201_63/floating-window/1 branch March 13, 2026 07:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[201_63] 文本选中悬浮框拆分-功能初步实现#2943

[201_63] 文本选中悬浮框拆分-功能初步实现#2943
MoonL79 merged 12 commits intomainfrom
yuki/201_63/floating-window/1

Yuki-Nagori commented Mar 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

MoonL79 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Yuki-Nagori commented Mar 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

MoonL79 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants