RFR: 8185887: TableRowSkinBase fails to correctly virtualize cells in horizontal direction [v4]

[Jose Pereda]

On Thu, 16 Jan 2025 08:24:26 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

>> modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewVirtualizationTest.java line 177:
>> 
>>> 175:         tableView.getColumns().addFirst(tableColumn);
>>> 176: 
>>> 177:         // Needs a double pulse so that the viewport breadth is correctly calculated.
>> 
>> This worries me a bit (the double pulse happens on a few places in the new tests). I would expect the platform to deal with is, so when a test requires 2 explicit pulse requests, it sounds like an issue.
>> Why can't this be done with a single request? (and then waiting until the pulse and pending runnables on the FX Thread have finished)?
>
> Unfortunately I don't think there is any other way. The `VirtualFlow` needs two pulses (in real life applications) as the first time, the layout is not yet correct for some cases (e.g. for `No ScrollBar` -> `ScrollBar`). I even used some watchpoints to confirm this behavior.
> 
> You can see the same thing in the `VirtualFlowTest.setUp` method, which initializes the flow and does two `pulse` calls after. So I think this is a premature problem, which probably can be fixed, but more refactorings/optimzations are needed (some of which I want to file a PR when I have more time). 
> Maybe at some point we can completely eliminate this problem, but I don't think I can do that here in this PR (unless you have an idea, which is very much welcome!)

Maybe the name `pulse` in the test you point out is misleading, since it actually calls `flow.layout()`, and two layout passes is not the same as two pulses.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1644#discussion_r1918271233