On Tue, 8 Nov 2022 21:13:27 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> I think the idea is this (and it only applies when snapToPixels=true). We >> only need to use snapped coordinates/sizes when placing components. In >> order to avoid accumulating errors, which can be seen during gradual >> resizing for example, we should avoid snapping of intermediary results, >> UNLESS there is a possibility of introducing visual artifacts. >> >> What I mean by this is imagine a container that aligns its children in a >> table-like layout (similar to >> https://github.com/andy-goryachev/FxDock/blob/master/doc/CPane.md). >> Depending on constraints, there might be a situation where rounding might >> shift certain children in relation to the vertical guide lines - we don't >> want that. >> >> So in this case we need to snap the guide lines as an intermediate step >> (i.e. compute the first guide line position, then compute the next column, >> snap, next column, etc.) >> >> Whereas in a simple case (one child node), we can simply use snapped*Inset() >> - we don't have intermediate computations. >> >> In the case of AnchorPane, I think, we should first sum insets and anchor, >> then snap - because developers might want to use or not to use snapToPixels, >> and might set a fractional anchor. > > I initially had the same thought that Andy had, but I want to take a closer > look -- especially at what similar layout panes do. > > As for the point about doing this only when `snapPixels == true`, yes, but > you don't need to worry about that if you call the `snapXXX` methods in > Region. All of them check the `snapToPixel` attribute. I looked at the other layout panes and they already do something similar to what this PR does, using the snapped insets for intermediate compuation. I also note that some of the computation is calling superclass methods that do snapping already, and expect it to be added to snapped insets (if there are insets), for example, `computeWidth()`. In this particular case -- `snappedLeftInset() + leftAnchor` -- I don't think `leftAnchor` is snapped, but it probably should be. ------------- PR: https://git.openjdk.org/jfx/pull/910
