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

Reply via email to