On Tue, 30 May 2023 10:39:28 GMT, Johan Vos <[email protected]> wrote:
>> Only update the VirtualFlow parameters in case the size of a cell has
>> changed.
>>
>> The fixes for JDK-8298728 and JDK-8277785 introduced an unconditional
>> recalculation in case the size of a cell is set. This recalculation is only
>> needed in case the size of that specific cell has changed.
>> Fix for JDK-8293836
>
> Johan Vos has updated the pull request incrementally with one additional
> commit since the last revision:
>
> remove newline
Have one question. Otherwise looks good to me. Tested with a lot of data + with
and without fixed cell size.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
line 3083:
> 3081: if (itemSizeCache.size() > cellIndex) {
> 3082: Double oldSize = itemSizeCache.get(cellIndex);
> 3083: double newSize = isVertical() ?
> cell.getLayoutBounds().getHeight() : cell.getLayoutBounds().getWidth();
If we have a fixed cell size set, can we save the call to get the layout bounds
here?
Since this is also done above in `getOrCreateCellSize` (`getFixedCellSize()`).
And the layout bounds are calculated lazily, so this may improve the
performance here as well.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
line 3085:
> 3083: double newSize = isVertical() ?
> cell.getLayoutBounds().getHeight() : cell.getLayoutBounds().getWidth();
> 3084: itemSizeCache.set(cellIndex, newSize);
> 3085: if ((oldSize != null) && !oldSize.equals(newSize)) {
Minor: Braces are not needed here
modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
line 3088:
> 3086: int currentIndex = computeCurrentIndex();
> 3087: double oldOffset = computeViewportOffset(getPosition());
> 3088: if ((cellIndex == currentIndex) && (oldOffset != 0)) {
Minor: Braces are not needed here
-------------
PR Review: https://git.openjdk.org/jfx/pull/1098#pullrequestreview-1456254974
PR Review Comment: https://git.openjdk.org/jfx/pull/1098#discussion_r1213648251
PR Review Comment: https://git.openjdk.org/jfx/pull/1098#discussion_r1213645083
PR Review Comment: https://git.openjdk.org/jfx/pull/1098#discussion_r1213645163
