<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Johan, I had a closer look.</p>
<p>TLDR: </p>
<p>- FX currently almost always does two layout passes, with the 2nd
pass not changing anything, due to a flaw in Node's relayout
trigger logic<br>
- I think it may be possible to fix this, and as a side effect
also fixes your infinite layout loop scenario <br>
</p>
<p>Longer version:<br>
</p>
<p>Although I think the original fix is correct (as it fixes broken
state tracking), I do see that the fix may cause existing flaws,
that were swept under the rug before, to become more problematic.</p>
<p>I think the fundamental reason for the relayout trigger code in
Node on LayoutX/Y changes is to catch cases when these are
modified OUTSIDE the scope of its direct Parent's layoutChildren
call. In fact, it seemingly does its best to detect when a change
occurs as part of a Parent's layoutChildren call as seen in this
code and the clarifying comment:</p>
<div style="background-color:#ffffff;padding:0px 0px 0px 2px;">
<div
style="color:#000000;background-color:#ffffff;font-family:"Consolas";font-size:11pt;white-space:pre;"><p
style="margin:0;"><span style="color:#000000;"> </span><span
style="color:#3f7f5f;">// Propagate layout if this change isn't triggered by its parent</span></p><p
style="margin:0;"><span style="color:#000000;"> </span><span
style="color:#0000a0;font-weight:bold;">if</span><span
style="color:#000000;"> (p != </span><span
style="color:#0000a0;font-weight:bold;">null</span><span
style="color:#000000;"> && !p.isCurrentLayoutChild(Node.</span><span
style="color:#0000a0;font-weight:bold;">this</span><span
style="color:#000000;">)) {</span></p></div>
</div>
<p></p>
<p>However, this check utterly fails in almost all situations, for
the simple reason that the child node which has its layout X/Y
changed is NOT considered the current layout child by its parent.
None of the containers will update the current layout child before
doing each layoutX/Y change (as that would be very cumbersome...)
However, we're not actually interested in that. All we want to
know is: </p>
<p> *** Is this layout X/Y change performed while the direct
Parent's layoutChildren() call is higher up in the call stack? ***
<br>
</p>
<p>There are two ways to answer that question:</p>
<p>- One can ask the *grandparent* if the child's parent is the
current layout child -> the answer would be yes, BUT it only
means its layout() is running, and often also its layoutChildren()
(so this isn't perfect)<br>
- Another option is to ask the direct parent if it is currently
performing layout -> the answer would be yes, and it normally
means its layoutChildren is running, but the flag isn't cleared
immediately after the call completes, so this is also a bit
flawed; excerpt from Parent::layout():<br>
</p>
<div style="background-color:#ffffff;padding:0px 0px 0px 2px;">
<div
style="color:#000000;background-color:#ffffff;font-family:"Consolas";font-size:11pt;white-space:pre;"><p
style="margin:0;"><span style="color:#000000;"> </span><span
style="color:#0000c0;">performingLayout</span><span
style="color:#000000;"> = </span><span
style="color:#0000a0;font-weight:bold;">true</span><span
style="color:#000000;">;</span></p><p style="margin:0;"><span
style="color:#000000;"> layoutChildren();
... (more code that triggers layouts for the rest of the sub tree) ...
</span></p><p style="margin:0;"><span style="color:#000000;"> </span><span
style="color:#0000c0;">performingLayout</span><span
style="color:#000000;"> = </span><span
style="color:#0000a0;font-weight:bold;">false</span><span
style="color:#000000;">;</span></p><span style="color:#000000;"></span><p
style="margin:0;"><span style="color:#000000;">
</span></p></div>
</div>
<p></p>
<p>The flag however is incredibly close to what we need, and an
extra tighter flag that is reset immediately after
`layoutChildren` completes (instead of after the whole subtree
completes layout) would be exactly what we need.<br>
<br>
So now if Node wants to know if a layout X/Y is changed because
the parent's layout children triggered it, then all it needs to
ask the parent node is if this new flag is set.<br>
</p>
<p>With this fix, your scenario would also no longer trigger a
layout loop, assuming the node you are modifying is a direct child
of the node on which layoutChildren is overridden.<br>
</p>
The only reason that currently this broken logic is not already
triggering infinite layouts is that a 2nd pass will set all values
to the same values, and Node will not trigger its logic if the value
was unchanged. However, its already broken that a 2nd pass is
necessary at all. In effect, JavaFX is currently almost always
doing **two** layout passes; one that changes all the positions and
sizes, and another that "confirms" these unnecessarily.<br>
<br>
As it is always dangerous to play with the layout logic, this will
need to be well tested, but I think we really need to fix this, or
revert the JDK-8360940 change as I think it may otherwise cause too
much problems, even though it is fixing problems and just exposing
new ones...
<p>--John<br>
</p>
<p><br>
</p>
<p><br>
</p>
<div class="moz-cite-prefix">On 21/10/2025 21:42, Johan Vos wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CABxFH2G75jWSCnqVSNpBcMnhOXSLmDJP9-UfNokx_2tTn7az7g@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div dir="ltr">
<div>I recently noticed a performance degradation in a JavaFX
application, that I could trace to the fix for JDK-8360940
[1]. I believe the fix is correct, but it can cause
infinite pulses in some cases. It is somehow discussed in the
second of the 2 errors in Parent, in [2], but I believe this
pattern is used often, and will now lead to infinite pulses</div>
<div><br>
</div>
<div>For example, a class extending Parent and implementing
layoutChildren that does something like this:</div>
<div><br>
</div>
@Override <br>
protected void layoutChildren() {
<div> super.layoutChildren();</div>
<div> if (someSpecificCondition) {</div>
<div> someManagedNode.relocate(someX, someY);</div>
<div> }</div>
<div>}<br>
<div><br>
</div>
<div>Support the someManagedNode is at (x0, y) before the
pulse runs.</div>
<div>The call to super.layoutChildren() will set the layoutX/Y
to e.g. superX, superY and it will request a new pulse (in
case superX != x0 or superY != y).</div>
<div>Next, the someManagedNode is conditionally repositioned
to someX, someY .</div>
<div>In the next pulse (immediately following this pulse), the
super.layoutChildren will again set the layoutX/Y to superX,
superY, and trigger a new pulse, no matter what happens
afterwards. The conditional code will again reset the
layoutX/Y to someX, someY (which is what they had before
this pulse started layoutChildren()), but the next pulse is
scheduled AND it will visit this Node.</div>
<div><br>
</div>
<div>Even though in the end the layoutX/layoutY properties of
someManagedNode are not changed as the result of
layoutChildren, a new pulse will be requested because there
was an intermediate value (which never got rendered, as it
was changed again even before the layoutChildren returns)
that triggered the requestNextPulse.</div>
<div><br>
</div>
<div>I think this pattern is used in a number of external
controls. It worked "by accident" until the issue in
JDK-8360940 was fixed. In defense of the pattern above, one
could argue that a requestPulse should only be triggered if
the triggering property was altered at the end of the
layoutPhase (that is, if there is a difference in value
between the start and the end of the layoutPhase).</div>
<div><br>
</div>
<div>In summary, I don't propose to change anything, and I
agree with the commit in [1], but it might need some
attention in the release notes or docs.</div>
<div><br>
</div>
<div>- Johan</div>
<div><br>
</div>
<div><br>
</div>
<div>[1] <a
href="https://github.com/openjdk/jfx/commit/5682424bebae7947f665fcb11429c3d2e069e8e5"
moz-do-not-send="true" class="moz-txt-link-freetext">https://github.com/openjdk/jfx/commit/5682424bebae7947f665fcb11429c3d2e069e8e5</a></div>
</div>
<div>[2] <a
href="https://bugs.openjdk.org/browse/JDK-8360940?focusedId=14811022"
moz-do-not-send="true" class="moz-txt-link-freetext">https://bugs.openjdk.org/browse/JDK-8360940?focusedId=14811022</a></div>
</div>
</blockquote>
</body>
</html>