On Mon, 6 Mar 2023 16:04:02 GMT, Florian Kirmaier <fkirma...@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
