RFR: 8303680 Virtual Flow freezes after calling scrollTo and scrollPixels in succession

[Florian Kirmaier]

On Mon, 6 Mar 2023 16:04:02 GMT, Florian Kirmaier <fkirmaier at openjdk.org> wrote:

> Possible fix for VirtualFlow freeze.
> 
> I encountered the problem when experimenting with VirtualFlow.
> 
> Guess @johanvos should take a look.
> All tests are still green, so with some luck, this doesn't break anything but fixes some known and unknown bugs.

Some explanation:

The layout method contains logic, to skip the layouting.


            if (width == lastWidth &&
                height == lastHeight &&
                cellCount == lastCellCount &&
                isVertical == lastVertical &&
                position == lastPosition &&
                ! cellSizeChanged)
            {
                // TODO this happens to work around the problem tested by
                // testCellLayout_LayoutWithoutChangingThingsUsesCellsInSameOrderAsBefore
                // but isn't a proper solution. Really what we need to do is, when
                // laying out cells, we need to make sure that if a cell is pressed
                // AND we are doing a full rebuild then we need to make sure we
                // use that cell in the same physical location as before so that
                // it gets the mouse release event.
                return;
            }


So, when the position is changed, then the content should be layout again.
But when we reset the oldPosition, to the current position, somewhere else in the contain - then we interfere with this logic.
This has the effect, that the layout is skipped.
For that reason, it seems wrong to me, to change the "last-something" values outside of this method.

So I removed the resetting of the lastPosition in the scrollPixels method.
`lastPosition = getPosition();`
I also don't see a reason, why the lastPosition should be set anyways.

But I have to admit, that I don't understand the class well enough to ensure this is correct based on logic. This fixes the unit test and doesn't seem to break tests or something obvious in applications according to my tests.

-------------

PR: https://git.openjdk.org/jfx/pull/1052