Skip to content

Rework pixel snapping, take two.#1803

Draft
xStrom wants to merge 5 commits into
linebender:mainfrom
xStrom:compose-snap-layout
Draft

Rework pixel snapping, take two.#1803
xStrom wants to merge 5 commits into
linebender:mainfrom
xStrom:compose-snap-layout

Conversation

@xStrom
Copy link
Copy Markdown
Member

@xStrom xStrom commented May 25, 2026

Background

The old pixel snapping system rounded both the origin (top-left) and end point (bottom-right) of the layout border-box in the parent widget's layout border-box coordinate space. Specifically this happened during the layout pass, when the parent called place_child.

The major benefit of this approach is that adjacent geometry will be pixel snapped in a compatible way where there won't be any overlap or empty gaps. This has the promise of a very polished and sharp looking render.

The cost of this approach is that the layout size and pixel snapped size may diverge. The box might shrink, remain the same, or grow.

The problems keep coming

As we've lived with this system, more problems have become clear.

Masonry supports arbitrary transforms on widgets. Doing the pixel snapping in the parent's coordinate space means that none of the transforms that convert from the parent's space to the window's space are taken into account. This is a critical flaw, because the idea of snapping is to align to device pixels, not to parent widget local pixels.

Because snapping happens in the parent's space, there is no consistent rounding of boxes across nested widget levels. It's easy to run into situations where both parent and child have the exact same fractional size, yet they get snapped to different integer sizes.

We've known that the cost of this approach is that snapped box sizes will differ from layout sizes. However, what has not been as obvious and definitely not documented, is that this also effectively introduces a whole new coordinate space. The (1,1) of the layout box isn't the same window pixel location as the (1,1) of the snapped box. Yet there is a bunch of code that ignores this shift and mixes layout and snapped coordinates. Sometimes that can be ok, but thus far very little of it seems intentional.

Improving the situation

We can achieve much improved pixel snapping if we resolve the layout border-box via the full window transform, apply the DPI scale factor to get device pixels, round, and map the snapped box back into local layout border-box space. The rounding will also be consistent across arbitrary nested transforms, so equally sized parent-child combos will result in equal snapped sizes. Pixel snapping will be skipped for a specific widget if the transform has rotation or shear, as it will just add noise and won't be correct. We can also skip the snapping of specific widgets during animation.

Going beyond the first attempt

#1792 implemented the improvements, but notably kept the the differences between layout and snapped coordinates. It works, but the extra comprehension required to make sense of all the coordinate systems is non-trivial. That is where this PR here comes in. Now the pixel snapping is not only transform-aware and consistent-across-nesting-levels, but it also happens earlier in the box lifecycle, even before Widget::layout runs. This allows us to greatly simplify the box lifecycle and coordinate system complexity.

Changes in this PR

  • Replaced LayoutCtx::run_layout(size) and LayoutCtx::place_child(origin) with LayoutCtx::layout_child(origin, size) and an optional LayoutCtx::move_child(origin). This enables pixel snapping to happen before Widget::layout is called, while still enabling the widget to be moved later.
  • Added a private LayoutCtx::snap_transform which is a stripped down version of window_transform. It doesn't contain translations which can be guaranteed to be integer device pixels, allowing snapping to account for transforms while still enabling the widget to be moved (by an integer device pixel delta) either via LayoutCtx::move_child or via scrolling.
  • Added ability to disable pixel snapping for a widget branch, useful for animations.
  • Removed the concept of effective box as it isn't really a local box change like the others, it's just a coordinate space change.
  • Removed the concept of aligned box as now layout boxes are already pixel snapped. This also means that we can generally use prefixless names like "border-box" and it will mean the pixel snapped box, which is used for layout, hit testing, painting, everything.
  • Removed ComposeCtx::set_animated_child_scroll_translation as the regular set_child_scroll_translation is now universal.
  • WindowEvent::Rescale handler now also requests layout and runs rewrite passes so snapped geometry is up-to-date before the next pointer event.
  • Tweaked widget code as needed.
  • Added a bunch of pixel snap related tests.

Screenshot changes

All test screenshot changes are due to pixel snapping rounding changes. Specifically that pixel snapping is now consistent across widget nesting levels as it is done in the window's coordinate space.

Follow-up work

These are related things that are either certainly or potentially broken that didn't receive full attention in this PR here yet, in order to limit PR size.

  • Baseline snapping continues to be busted.
  • Various helpers like for hairlines don't yet exist.
  • Border-box inset handling might be broken in a few places when the content-box was already zero or close to zero and pixel snapping reduced it below zero.

Draft because this PR depends on #1811, #1813, #1814, #1815 and currently contains those commits as well.

@xStrom xStrom added masonry Issues relating to the Masonry widget layer blocked Progress is blocked by some other task. labels May 25, 2026
@xStrom xStrom force-pushed the compose-snap-layout branch 2 times, most recently from 995d59f to dfd8e97 Compare May 27, 2026 16:09
@xStrom xStrom force-pushed the compose-snap-layout branch from dfd8e97 to e5a5ed0 Compare May 28, 2026 16:06
@xStrom xStrom mentioned this pull request May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked Progress is blocked by some other task. masonry Issues relating to the Masonry widget layer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant