Open
Conversation
Yuki-Nagori
pushed a commit
that referenced
this pull request
Mar 6, 2026
- 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.
- 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，避免重复调用 - 使用原始坐标检查选区是否在视图内 - 文本工具栏缓存，用于性能优化 保持代码与项目中其他注释语言风格一致。
Yuki-Nagori
force-pushed
the
yuki/201_63/floating-window/1
branch
from
bc39a41 to
68a8015
Compare
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
Yuki-Nagori
pushed a commit
that referenced
this pull request
Mar 6, 2026
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
Yuki-Nagori
force-pushed
the
yuki/201_63/floating-window/1
branch
from
9177e1d to
e56a44b
Compare
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
基于 #2674