On Thu, 23 Oct 2025 16:55:32 GMT, John Hendrikx <[email protected]> wrote:
>> This new check is much more accurate to detect whether a parent is currently >> laying out its children. The previous code almost never worked, resulting in >> additional unnecessary layouts. > > John Hendrikx has updated the pull request incrementally with one additional > commit since the last revision: > > Fix ToolBarSkinTest > > Reusing a toolbar as part of several scenes, in combination with the > StubToolkit that doesn't handle pulses makes this test fail with the relayout > detection fix. > > would agree that adding a whole battery of functional tests for layout is > > out of scope and better done as a follow-on. > > I'm not sure I agree with this. Personally, I consider the battery of > functional tests to be more important than the fix, so without the tests, I > would be very conservative. However, we're still relatively early in the 26 > release cycle, and we won't do a 26 LTS, so risks on real-world drama's are > limited. We can still revert this PR (if it got merged) and the previous one > later in the 26-development phase. > > I'll look into a more deterministic test scenario -- the suggestion from Andy > with 3 levels make sense (although reality is much more complex, with > Platform.runLater() running in between 2 pulses, and _that_ is > non-deterministic as it depends on processor speed etc. It would actually be much easier to create deterministic tests **after** this change (unless we're dealing with a converging layout that can sometimes happen with biased containers). In the before situation, you should check if another pulse is scheduled after the layout pass, and execute it as well to be sure that the layout is stable. In the after situation, this will almost never be the case and the presence of a pulse request can be asserted to be false. In any case, I'd still recommend either reverting https://bugs.openjdk.org/browse/JDK-8364049 or combining it with this fix. The intermediate situation is still fine for most cases, but there can be nasty surprises if left in by itself (albeit only in somewhat self-contradicting layout code). I think having both the fixes guarded behind a system property averts most risks. I'd recommend setting it on by default though, to get real world feedback, as I think we do want to incorporate this fix permanently if it works out. I think it definitely much closer matches the intent of the original implementation. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1945#issuecomment-3448345419
