On Fri, 24 Sep 2021 12:42:31 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> This PR fixes an issue which is probably in JavaFX since VirtualFlow exists. >> While horizontal scrolling any VirtualFlow control the left blue border >> sometimes disappear/gets smaller. (see also image below) >> >> This can be fixed by **snapping** the scroll bar value (similar like e.g. a >> **ScrollPane** does). >> The same needs to be done on the table header as otherwise the column lines >> might be 1 px off to the cell lines. >> As a side effect this also fixes that the column lines sometimes get's >> blurry when horizontal scrolling (see second image). >> >> While testing with **-Dglass.win.uiScale** I found out that the problem is >> not fixed for a scale like 1.25 or 1.5, while it is fixed for 1 or 2. The >> border sometimes disappears only when the snapped value is a decimal number >> (which obviously does not happen on a scale of 1 or 2), e.g. something like >> 12.6 but it will never disappear when it's a normal number, so e.g. just 12. >> >> That's why something like **Math.round(..)** or just a **cast** to an >> **int** instead of snapping fixes this problem for all scales. I also didn't >> notice any side effect. But not sure if this the right fix then. >> How does JavaFX render a **node** when e.g. the x is a decimal number? And >> does a decimal number make sense (Why we e.g. don't round the value)? >> >> Another explanation could also be that there is an issue somewhere deep >> inside the node layout/snapping/Clip/Group/pixel rendering and to simply >> round/cast the value just fixes the symptom here. >> >> In any case I'm open for any feedback, help or explaination. >> I'm also glad for anything which might help identify the root cause here, if >> any. >> >> --- >> 1. >>  >> --- >> 2. >>  > > modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java > line 3082: > >> 3080: double snappedClipX = snapPositionX(clipX); >> 3081: setLayoutX(-snappedClipX); >> 3082: clipRect.setLayoutX(snappedClipX); > > This is likely not the right place to snap the coordinates to a pixel > boundary. This is usually done as part of layout. I'm also skeptical of the > fact that you added it to `setClipX` but not `setClipY`. I didn't set **clipY** because it's not part of the story and **clipY** is always set to 0 when the `VirtualFlow` is vertical (which is the default for all `VirtualFlowContainer`. Only `ListView` **can** be non-vertical - This should be another bug and test case). That's why there is no issue with it normally. I'm not sure if this is the wrong place. E.g. `ScrollPaneSkin` also listens on the scrollbar **valueProperty** and sets the **snapped** scrollbar value with **setLayoutX** - which is very similar then here. We set the **layoutX** here as well (for clip and the container). Finally: While I understand it does not fix every scale, isn't it at least a good first step? (When we agree this is the correct location here) ------------- PR: https://git.openjdk.java.net/jfx/pull/630
