On Thu, 16 Jan 2025 08:24:26 GMT, Marius Hanl <mh...@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

Reply via email to