On Tue, 8 Nov 2022 21:00:00 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> I'm not sure about this. Other places do the same, and there are lot of >> places actually which do something like `snappedLeftInsets` + >> `snappedRightInsets` or `final double w = snapSizeX(getWidth()) - x - >> snappedRightInset();` or similar. >> I think this was for a reason. >> And I have only oriented myself to the existing code in this case. If you >> are right, then this was done wrong everywhere. >> >> Btw. I had a look and other container do snap their constraints, e.g. >> `H/VBox` its spacing, `GridPane` his `H/VGap`. So pretty sure we also should >> snap the anchor values. > > 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. ------------- PR: https://git.openjdk.org/jfx/pull/910