On Thu, 19 Dec 2024 16:03:02 GMT, Marius Hanl <mh...@openjdk.org> wrote:
>> This PR fixes the horizontal virtualization performed in the >> `TableRowSkinBase`, which significantly improves performance with many >> columns (and thus many cells). Scrolling up and down as well as scrolling >> left and right feels much more snappy. >> >> In order to do that, there are multiple things needed: >> - the `isColumnPartiallyOrFullyVisible` needs to be fixed to take the >> `VirtualFlow` into account, not the `TableView`. Since rows are inside the >> `VirtualFlow`, we need to use the width from there >> - The wrong implementation right now leads to >> [JDK-8276326](https://bugs.openjdk.org/browse/JDK-8276326), where if you >> scroll to the right with many columns, cells on the left start to be empty >> (since they are removed when they shouldn't be) >> - It also does not help performance when scrolled to the left, as the >> right cells are not removed (only when scrolling to the right, cells on the >> left are removed) >> - To improve performance, `isColumnPartiallyOrFullyVisible` was refactored >> to take in everything that is needed directly, without reiterating through >> all columns >> - As before, the `TableRow` adds or removes cells that are visible or not. >> Note that this is only done when a fixed cell size is set. >> The reason for that is that we always know the row height. If not set, we >> need all cells so we can iterate over them to get the max height. I'm not >> sure if this can be improved, but this is not the goal of this PR and can be >> checked later >> - The other issue mentioned in >> [JDK-8276326](https://bugs.openjdk.org/browse/JDK-8276326) happens only when >> a fixed cell size is set (empty rows). This is related and also fixed: >> - Cells start to be empty when scolling around a lot. This is because >> cells that are in the pile (ready to be reused) will not receive any layout >> requests (`requestLayout`) while all cells in the viewport will receive >> them. As soon as they are reused, they are visually outdated and not >> updated, leading to empty cells >> Fix is to request layout to those cells as well, and as soon as they are >> reused, they will layout themself >> - This has revealed another error: Cells that are not used anymore (added >> to the pile and invisible) are STILL inside the `VirtualFlow` sheet (as >> children). This will cause them to actually do the layout when requested, >> and especially when the height of the `TableView` changed drastically (e.g. >> from 50 visible cells to just 10), we have 40 cells laying around, receiving >> the layout request I added to fix [JDK-82763... > > Marius Hanl has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains four commits: > > - Merge branch 'master' of https://github.com/openjdk/jfx into > 8185887-virtualization > > # Conflicts: > # > modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/VirtualFlowTest.java > - Merge branch 'master' of https://github.com/openjdk/jfx into > 8185887-virtualization > > # Conflicts: > # > modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java > # > modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java > # > modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java > - 8185887: Reset disclosureNodeDirty in updateDisclosureNodeAndGraphic() > - 8185887: TableRowSkinBase fails to correctly virtualize cells in > horizontal direction Hey Guys, i am just new here, pls anyone help with my observation: Bug Report: JavaFX TreeView - Incorrect Cell Index Update on Expand/Collapse Issue: When expanding or collapsing a TreeItem in a TreeView, an incorrect cell index is updated. Specifically, a cell index mirrored vertically around the center of the visible area is updated instead of the correct cell. This indicates a flaw in the cell index update logic or the visible cell list update mechanism. This issue is particularly noticeable when cells are styled using CSS, as the incorrect update results in a visual glitch or flicker. Even without CSS, the problem manifests as a subtle graphical irregularity due to the rapid update process. Steps to Reproduce: Create a TreeView with enough items to enable scrolling. Scroll to make a specific range of indices visible (e.g., indices 4 to 13). Apply distinct CSS styles to cells to make updates visually apparent. Expand or collapse a TreeItem within the visible range (e.g., index 5). Observe that the mirrored index (e.g., index 12) is updated instead of the correct index. Observed Behavior: When a TreeItem at index i is expanded/collapsed, the index updated is: Mirrored Index = 2 ⋅ Center − 𝑖 Examples: Expanding/collapsing index 5 incorrectly updates index 12. Expanding/collapsing index 10 incorrectly updates index 3. Expected Behavior: Only the cell corresponding to the expanded/collapsed TreeItem should be updated. Impact: Visual inconsistencies and graphical glitches in the TreeView. Subtle flickering or graphical irregularities during expand/collapse actions, especially noticeable with styled cells. Suggested Cause: A miscalculation or incorrect implementation in the logic for updating cell indices or the visible cell list during expand/collapse operations. Environment: Observed in JavaFX versions 16 through 23. Present across different platforms and configurations. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1644#issuecomment-2582944486
