-
Notifications
You must be signed in to change notification settings - Fork 53
[201_70] 修复图片缩放的撤销问题 #2729
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[201_70] 修复图片缩放的撤销问题 #2729
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements table mouse scaling functionality, but there is a critical discrepancy: the PR title and description claim to fix image resize undo issues with changes to format-geometry-edit.scm, but the actual code changes are entirely about table scaling features with no modifications to image resize functionality.
Changes:
- Added table scaling fields (
hscale,vscale) to table and cell structures - Implemented mouse-based table scaling with visual handles (right, bottom, and corner handles)
- Added undo/redo support for table scaling operations using mark_start/mark_end
- Refactored table resizing code to distinguish between line resizing and scale resizing
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| devel/201_69.md | Documentation for table mouse scaling feature (contradicts PR title about images) |
| src/Typeset/Table/table.hpp | Added hscale/vscale fields to table_rep structure (not initialized in constructor) |
| src/Typeset/Table/cell.cpp | Added hscale/vscale fields to cell_rep with partial scaling implementation |
| src/Typeset/Env/env_default.cpp | Added default values for CELL_HSCALE and CELL_VSCALE environment variables |
| src/Typeset/Boxes/Composite/composite_boxes.cpp | Enhanced table message to include wide_flag for cell queries |
| src/Edit/Interface/edit_repaint.cpp | Added draw_table_resize_handles function to render scaling handles |
| src/Edit/Interface/edit_mouse.cpp | Implemented table scaling logic with mouse event handling and undo support |
| src/Edit/Interface/edit_interface.hpp | Added table scaling state variables and function declarations |
| moebius/moebius/vars.hpp | Declared CELL_HSCALE and CELL_VSCALE variables |
| moebius/moebius/vars.cpp | Defined CELL_HSCALE and CELL_VSCALE string constants |
| moebius/moebius/drd/drd_std.cpp | Registered CELL_HSCALE and CELL_VSCALE as TYPE_NUMERIC in DRD |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| if (vscale != 1.0) { | ||
| // TODO: adjust bh and th accordingly |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TODO comments indicate that bh and th should be adjusted when applying vertical scaling, but currently only mh is scaled. This may lead to incorrect bottom/top spacing or alignment when the vertical scale factor is applied. The same issue exists for horizontal scaling where lw and rw are not adjusted even though there's a TODO comment indicating they should be.
| // TODO: adjust bh and th accordingly | |
| bh *= vscale; | |
| th *= vscale; |
| if (var->contains (CELL_HSCALE)) | ||
| hscale = as_double (env->exec (var[CELL_HSCALE])); | ||
| else hscale = 1.0; | ||
| if (var->contains (CELL_VSCALE)) | ||
| vscale = as_double (env->exec (var[CELL_VSCALE])); | ||
| else vscale = 1.0; |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation is inconsistent with the surrounding code. All other if-else blocks in this function use 2-space indentation without extra spaces after the opening of the if statement, but lines 238 and 241 use 4 extra spaces after the if. They should follow the same indentation pattern as the surrounding code for consistency.
| void | ||
| edit_interface_rep::table_scale_start (SI x, SI y) { | ||
| SI x1= last_table_brec->x1; | ||
| SI y1= last_table_brec->y1 + 2 * last_table_hr; | ||
| SI x2= last_table_brec->x2 - 2 * last_table_hr; | ||
| SI y2= last_table_brec->y2; | ||
|
|
||
| table_resizing_type = 2; | ||
| table_scale_start_x = x; | ||
| table_scale_start_y = y; | ||
| table_scale_initial_width = x2 - x1; | ||
| table_scale_initial_height= y2 - y1; | ||
| table_scale_mark = new_marker (); | ||
| mark_start (table_scale_mark); | ||
| } |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The table_scale_path is only set during drawing in draw_table_resize_handles (line 267 in edit_repaint.cpp) but is not updated in table_scale_start. This means that table_scale_apply relies on the path being set from a previous draw call. If there's a timing issue where drawing hasn't occurred, or if the table structure has changed between drawing and the resize operation, the path could be stale or nil. Consider setting table_scale_path in table_scale_start similar to how table_line_path is set in table_line_start (lines 586-587 in edit_mouse.cpp).
| double hscale; // horizontal scaling factor | ||
| double vscale; // vertical scaling factor |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new hscale and vscale fields are added to table_rep but are not initialized in the constructor. This could lead to undefined behavior as these fields will contain garbage values. They should be initialized to 1.0 in the constructor initialization list, similar to how other fields are initialized.
| double hscale; // horizontal scaling factor | |
| double vscale; // vertical scaling factor | |
| double hscale = 1.0; // horizontal scaling factor | |
| double vscale = 1.0; // vertical scaling factor |
| double current_hscale = 1.0; | ||
| double current_vscale = 1.0; | ||
| string hscale_str = et->table_get_format ("cell-hscale"); | ||
| string vscale_str = et->table_get_format ("cell-vscale"); | ||
| if (hscale_str != "") current_hscale = as_double (hscale_str); | ||
| if (vscale_str != "") current_vscale = as_double (vscale_str); | ||
| double new_hscale = current_hscale * scale_x; | ||
| double new_vscale = current_vscale * scale_y; |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scaling calculation appears to compound the scale factor on each mouse movement. On each call to table_scale_apply, it reads the current scale values (lines 737-740), computes a delta scale based on mouse movement (lines 725-733), and then multiplies them together (lines 741-742). This means that during a single drag operation, the scale gets compounded with each mouse movement frame, leading to exponential rather than linear scaling behavior. The scale delta should be calculated relative to the initial state at the start of the drag, not relative to the current state. Consider storing the initial hscale/vscale values in table_scale_start and calculating the new scale as initial_scale * delta_scale rather than current_scale * delta_scale.
| @@ -0,0 +1,19 @@ | |||
| # [201_69] 表格支持鼠标缩放 | |||
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR title and description describe fixing image resizing undo issues and mention adding image-resize-mark in format-geometry-edit.scm. However, all the actual code changes in this PR are about table scaling functionality, not image resizing. The description mentions implementing mark/undo session handling for image resize, but the image resize code in format-geometry-edit.scm does not have such changes. This appears to be a significant discrepancy - either the PR description is incorrect, or the wrong changes were included in this PR.
| if (hscale != 1.0) { | ||
| // TODO: adjust lw and rw accordingly | ||
| mw *= hscale; | ||
| } |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TODO comments indicate that lw and rw should be adjusted when applying horizontal scaling, but currently only mw is scaled. This may lead to incorrect left/right spacing or alignment when the horizontal scale factor is applied. The same issue exists for vertical scaling where bh and th are not adjusted even though there's a TODO comment indicating they should be.
[201_70] 修复图片缩放的撤销问题
如何测试
Ctrl+Z撤销,应该只需要一次就能完全恢复到原始尺寸2026/1/30
What
在图片鼠标拖拽缩放功能中添加编辑会话标记,将整个拖拽过程作为一个事务处理,避免撤销时逐帧回退
Why
原实现中,鼠标拖拽缩放图片时,每帧修改都会调用
tree-set!触发独立的撤销记录,导致用户需要按多次Ctrl+Z才能完全撤销一次拖拽操作这不符合用户直觉,每次拖拽应该对应一次撤销操作,与表格格线拖动等操作保持一致
How
format-geometry-edit.scm中添加image-resize-mark全局变量(编辑会话标记)start-drag-left)调用(mark-new)创建标记并(mark-start)开始编辑会话end-drag-left)调用(mark-end)结束编辑会话,将整个拖拽过程合并为一个撤销记录