On Wed, 10 Jan 2024 19:21:16 GMT, Marius Hanl <mh...@openjdk.org> wrote:
> This PR fixes the border glitch/gap as explained in both linked tickets. > > I noted that the `ScrollPane` control does not suffer from this problem, > although the code is mostly the same as in `VirtualFlow`. The `ScrollPane` > snaps the content correctly, no matter which scale. I carefully checked the > code and it seems that the container structure in `ScrollPane` was explicitly > written to handle this perfectly. There was definitely some thought on that. > > So to finally fix this issue, I rewrote the `VirtualFlow` container/scrolling > code to be **exactly** the same as in `ScrollPane`(Skin). > And this also fixes the issue, while behaving the same as before. > > In the future it may makes sense to try to somewhat unify the code from > `ScrollPane` and `VirtualFlow`. I already have some ideas. > > Unfortunately though, one more container is introduced to `VirtualFlow`, so > the css needs to be changed since it is very strictly written in the first > place: > Before: `.list-view:focused > .virtual-flow > .clipped-container > .sheet > > .list-cell` > After: `.list-view:focused > .virtual-flow > .viewport > .clipped-container > > .sheet > .list-cell` > > To better understand this, the container structure (tree) is shown below: > > - ScrollPane > - viewRect -> `setClip` -> clipRect (viewContent size) > - viewContent -> `setLayoutX` > - Content > - vsb > - hsb > - corner > > --- > - VirtualFlow > - viewRect **(->NEW IN THIS PR<-)** -> `setClip` -> clipRect > (clippedContainer size) > - clippedContainer/clipView -> `setLayoutX` (when scrolling) > - sheet > - Cell > - vsb > - hsb > - corner Ok, so I found out the following: When a `Rectangle` is used as clip without any effect or opacity modification, the rendering goes another (probably faster) route with rendering the clip. That's why setting the opacity to `0.99` fixes the issue - another route will be used for the rendering. This happens at the low level (`NGNode`) side of JavaFX. See here `NGNode`:  `renderRectClip` seems to be doing something wrong. I see that the clip rect is floored there, which seems to be the cause. Rounding it is fixing this for me. The bug always appears when I scroll and the clip `RectBounds` are something like: `RectBounds { minX:6.999996, minY:37.0, maxX:289.00003, maxY:194.0} (w:282.00003, h:157.0)` while this does not happen when the value is something like: `RectBounds { minX:7.000004, minY:37.0, maxX:289.0, maxY:194.0} (w:282.0, h:157.0)` Note that the `minX` is floored down, that explains the 1px gap. And now I fully understand why this only happens for the `VirtualFlow`: This is the only container and location where the clip is layouted as well! (`setLayoutX`/`setLayoutY`). In `ScrollPaneSkin`, there was the decision, that the clip is set and the content is layouted only, not the clip. It may still make sense to me to actually imrpove the `VirtualFlow` here - it seems like a good choice to just set the clip and never touch it again and only change the underlying content layout, might be even a bit faster. But there is obviously a bug on the low level `NGNode` side as well. Not sure where to go from here. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1330#issuecomment-1987232133
