Snap overlays to device instead of UI pixels on web - #4281
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces hardcoded coordinate rounding and half-pixel offsets with dynamic snapping helper methods (snap_to_physical_pixel and snap_to_physical_pixel_center) that scale with the viewport. A review comment points out an inconsistency in push_path where the starting point is snapped to the physical pixel boundary, while subsequent points are snapped to the physical pixel center, which could cause visual misalignment.
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 start_point = transform.transform_point2(point_to_dvec2(first.start())); | ||
| let start_point = self.snap_to_physical_pixel(start_point); |
There was a problem hiding this comment.
In push_path, the starting point of the path (start_point) is snapped to the physical pixel boundary using snap_to_physical_pixel, whereas all subsequent points in the path (lines, quadratic curves, cubic curves) are snapped to the physical pixel center using snap_to_physical_pixel_center.
This inconsistency causes the first segment of the path to be misaligned or skewed by half a physical pixel relative to the rest of the path, which is especially noticeable when the path is stroked or closed. Snapping the start point to the physical pixel center ensures visual consistency across the entire path.
| let start_point = self.snap_to_physical_pixel(start_point); | |
| let start_point = self.snap_to_physical_pixel_center(start_point); |
There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 3/5
- In
editor/src/messages/portfolio/document/overlays/utility_types_web.rs, the path uses mixed snapping methods (snap_to_physical_pixelforstart_pointvssnap_to_physical_pixel_c...for later points), which can create subtle geometry discontinuities or visible jitter at segment joins in rendered overlays; align all path points to the same snapping strategy and verify with a quick visual regression check before merging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="editor/src/messages/portfolio/document/overlays/utility_types_web.rs">
<violation number="1" location="editor/src/messages/portfolio/document/overlays/utility_types_web.rs:946">
P2: Inconsistent snapping: `start_point` is snapped with `snap_to_physical_pixel` while all subsequent points in the path (line endpoints, quadratic/cubic control and end points) are snapped with `snap_to_physical_pixel_center`. This causes the first segment to be offset by half a physical pixel relative to the rest of the path. Other methods like `draw_line` and `draw_polygon` consistently use `snap_to_physical_pixel_center` for all points including `move_to` — this should do the same.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| }; | ||
|
|
||
| let start_point = transform.transform_point2(point_to_dvec2(first.start())); | ||
| let start_point = self.snap_to_physical_pixel(start_point); |
There was a problem hiding this comment.
P2: Inconsistent snapping: start_point is snapped with snap_to_physical_pixel while all subsequent points in the path (line endpoints, quadratic/cubic control and end points) are snapped with snap_to_physical_pixel_center. This causes the first segment to be offset by half a physical pixel relative to the rest of the path. Other methods like draw_line and draw_polygon consistently use snap_to_physical_pixel_center for all points including move_to — this should do the same.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/portfolio/document/overlays/utility_types_web.rs, line 946:
<comment>Inconsistent snapping: `start_point` is snapped with `snap_to_physical_pixel` while all subsequent points in the path (line endpoints, quadratic/cubic control and end points) are snapped with `snap_to_physical_pixel_center`. This causes the first segment to be offset by half a physical pixel relative to the rest of the path. Other methods like `draw_line` and `draw_polygon` consistently use `snap_to_physical_pixel_center` for all points including `move_to` — this should do the same.</comment>
<file context>
@@ -941,29 +943,30 @@ impl OverlayContext {
};
let start_point = transform.transform_point2(point_to_dvec2(first.start()));
+ let start_point = self.snap_to_physical_pixel(start_point);
self.render_context.move_to(start_point.x, start_point.y);
</file context>
| let start_point = self.snap_to_physical_pixel(start_point); | |
| let start_point = self.snap_to_physical_pixel_center(start_point); |
Web version of #3493