On Sun, 15 Jun 2025 14:23:27 GMT, Marius Hanl <mh...@openjdk.org> wrote:
> When calling `refresh()` on virtualized Controls (`ListView`, `TreeView`,
> `TableView`, `TreeTableView`), all cells will be recreated completely,
> instead of just refreshing them.
>
> This is because `recreateCells()` of the `VirtualFlow` is called when
> `refresh()` was called. This is not needed, since refreshing the cells can be
> done much cheaper with `rebuildCells()`.
>
> This will reset all cells (`index = -1`), add them to the pile and fill them
> back in the viewport with an index again. This ensures `updateItem()` is
> called.
>
> The contract of `refresh()` is also a big vague, stating:
>
> Calling {@code refresh()} forces the XXX control to recreate and repopulate
> the cells
> necessary to populate the visual bounds of the control.
> In other words, this forces the XXX to update what it is showing to the user.
> This is useful in cases where the underlying data source has changed in a way
> that is not observed by the XXX itself.
>
>
> As written above, recreating is not needed in order to fulfull the contract
> of updating what is shown to the user in case the underlying data source
> changed without JavaFX noticing (e.g. calling a normal Setter without any
> Property and therefore listener involved).
It is indeed true that refresh() is often a very expensive operation. Whenever
`VirtualFlow.recreateCells()` is called, most of the internal state of the
VirtualFlow is destroyed, and everything is recalculated from scratch. I
believe that is not a problem, as long as recreateCells() is not called (by the
controls) unless really needed. In that light, this PR seems interesting, as it
will limit the number of rebuild-from-scratch cases.
I ran the basic controls tests after applying the PR, and (a bit to my
surprise) they all passed, which is great.
However, it is very likely that some code out there implicitly rely on the
rebuild-from-scratch logic, and that code will then work different after
applying this PR. I believe it would be good to find such a case where the
behavior (which, I agree, is often a bit vaguely defined) changes, so that we
can discuss this with a concrete example.
Hence, while I like the idea here (avoiding unneeded heavy-cost operations in
VirtualFlow), I would like to avoid a number of follow-up issues once this is
merged -- driven by a change in "expected" behavior.
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1830#issuecomment-2979258772
