FIX: Show local-axis grab measurement as a single tilted component#4326
FIX: Show local-axis grab measurement as a single tilted component#4326Chetansahney wants to merge 1 commit into
Conversation
When grabbing (G) and pressing X/Y a second time to constrain to a local axis, the measurement overlay drew an axis-aligned box showing both the X and Y screen components (and printed a typed value on both axes) instead of a single line along the tilted local axis. In the Grabbing overlay branch, when constrained to a single axis in local space, draw one dashed line along the (already tilted) translation vector with a single measurement label, mirroring how Scale (S) already behaves. The unconstrained and global-axis cases are unchanged. Closes GraphiteEditor#3341
Chetansahney
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the grab transform operation overlay. When translating a layer constrained to a single axis in local space, it now draws a single dashed line with a measurement label along that axis instead of an axis-aligned box. The reviewer suggested reusing the existing format_rounded helper to format the measurement label to maintain consistency and reduce code duplication.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| let label = match typed_string { | ||
| Some(typed_string) => typed_string, | ||
| None => format!("{length:.2}").trim_end_matches('0').trim_end_matches('.').to_string(), | ||
| }; |
There was a problem hiding this comment.
Instead of manually matching on typed_string and duplicating the formatting logic, you can reuse the existing format_rounded helper closure defined at the beginning of the Overlays handler (line 218). This keeps the code concise and consistent with other transform operations (like scaling and rotating).
let label = format_rounded(length, 2);
| // axis-aligned box, which would incorrectly show both the X and Y screen-space components. | ||
| if self.state.is_transforming_in_local_space && axis_constraint != Axis::Both { | ||
| let end_point = pivot + translation_viewport; | ||
| overlay_context.dashed_line(pivot, end_point, None, None, Some(2.), Some(2.), Some(0.5)); |
There was a problem hiding this comment.
I would expect the drawing of the overlays to be implemented in the fn translation_box code.
| let end_point = pivot + translation_viewport; | ||
| overlay_context.dashed_line(pivot, end_point, None, None, Some(2.), Some(2.), Some(0.5)); | ||
|
|
||
| let axis_direction = self.state.constraint_axis(axis_constraint).unwrap_or(DVec2::X).normalize_or(DVec2::X); |
There was a problem hiding this comment.
You can just make this the overall condition e.g.
if let Some(axis_contraint) = self.state.constraint_axis(axis_constraint)
&& self.state.is_transforming_in_local_space {
// Logic for constraint in local space
}| overlay_context.dashed_line(pivot, end_point, None, None, Some(2.), Some(2.), Some(0.5)); | ||
|
|
||
| let axis_direction = self.state.constraint_axis(axis_constraint).unwrap_or(DVec2::X).normalize_or(DVec2::X); | ||
| let length = translation_viewport.dot(axis_direction) / document_to_viewport.matrix2.y_axis.length(); |
There was a problem hiding this comment.
This doesn't really make any sense to me? Why take the length of a column of the document to viewport matrix?
I guess it works given that you only rotate and scale in a uniform fashion the viewport.
I would suggest you do something like document_to_viewport.transform_vector2(translation_viewport)
| let text_position = pivot.midpoint(end_point) + translation_viewport.perp().normalize_or(DVec2::X) * 24.; | ||
| let transform = DAffine2::from_translation(text_position); | ||
| overlay_context.text(&label, COLOR_OVERLAY_BLUE, None, transform, 16., [Pivot::Middle, Pivot::Middle]); |
There was a problem hiding this comment.
Magic constants 16 and 24 should be given names since I don't know what they do.
2 participants
Closes #3341
When grabbing (G) and pressing X/Y a second time to switch from a global-axis to a local-axis constraint, the on-canvas measurement overlay was drawn incorrectly. It showed an axis-aligned box with both the X and Y screen-space components, and when typing a value it printed that number on both axes, instead of a single line along the tilted local axis.
This implements the change requested in #3527 (comment).
Change
In the
Grabbingoverlay branch, when the transform is constrained to a single axis in local space, draw one dashed line from the pivot along the (already correctly tilted)translation_viewportvector, with a single measurement label — instead of the screen-alignedtranslation_box.The unconstrained and global-axis cases are untouched and still use
translation_box. The equivalent overlay for Scale (S) already behaved this way, so this brings Grab in line with it.