Yeah, that's what I'm seeing, it happens on all kinds of actions, none of which are triggering a resize that requires layout (like opening/closing a TitledPane or resizing the whole Window).
--John On 23/04/2025 16:45, Andy Goryachev wrote: > > Possibly related: https://bugs.openjdk.org/browse/JDK-8089992 > > > > -andy > > > > > > *From: *John Hendrikx <john.hendr...@gmail.com> > *Date: *Wednesday, April 16, 2025 at 16:13 > *To: *Nir Lisker <nlis...@gmail.com>, Andy Goryachev > <andy.goryac...@oracle.com> > *Cc: *openjfx-dev@openjdk.org <openjfx-dev@openjdk.org> > *Subject: *[External] : Re: Unnecessary layouts; TLDR; new method > "requestLocalLayout" > > I tested this with several controls that were triggering layouts (like > on cursor movements), and I saw no adverse effects. Basically, any > time you interact with a control and it triggers a full layout but its > bounds didn't change (ie. nothing in the UI changed position or size) > the full layout was unnecessary. > > Most Skins/Controls do the simple thing of registering listeners on > any properties that may change their appearance and calling > requestLayout, while calling requestLayout should really be reserved > for things that change their computeMin/Pref/Max values. If there were > no changes in any of those, then the parent layout won't have changes > either (and so on) so the final layout result will be the exact same > as before, yet tens of thousands of calculations will have been done. > Because of how say HBox calculates its size, it also queries any > siblings, which in turn may be containers... > > The only things "stopping" layout propagation are things like scroll > panes. This is why TextArea is a lot less likely to trigger a layout > all the way to the root vs TextField. > > --John > > On 16/04/2025 17:04, Nir Lisker wrote: > > Sounds good. Have you tried a prototype implementation for a > built-in JavaFX control/Pane, just to see how well it works? > > > > On Wed, Apr 16, 2025 at 5:50 PM Andy Goryachev > <andy.goryac...@oracle.com> wrote: > > This might be a good idea from an API perspective, but please > be careful - this optimization might break the behavior. For > instance, the scroll bar might change as a result of a key > event in the TextArea, so the text layout is still needed, > however expensive. > > > > (and I like Michael's suggestion of naming the method > requestLayoutChildren()) > > > > -andy > > > > > > > > *From: *openjfx-dev <openjfx-dev-r...@openjdk.org> on behalf > of John Hendrikx <john.hendr...@gmail.com> > *Date: *Monday, April 14, 2025 at 08:56 > *To: *openjfx-dev@openjdk.org <openjfx-dev@openjdk.org> > *Subject: *Unnecessary layouts; TLDR; new method > "requestLocalLayout" > > I've been writing a container that does layout, and I've been > using it > extensively in my latest project. > > I noticed that many skins and controls will call > requestLayout(), not > realizing that this will mark the current node + all parent > nodes with > `NEEDS_LAYOUT`. This causes all those containers to call > `compute` > methods and execute their `layoutChildren`, even though your > control may > only have changed something that does NOT change its layout > bounds (like > a color, background, alignment or even things like a cursor > shape or > position). These computations are expensive, involving > querying of all > children of each container to find out their min/pref/max > sizes, do > content bias calculations, splitting space over each control > and many > many snapXYZ calls -- all leading to no visual layout change... > > For example, a TextArea or TextField will call requestLayout > on every > character typed, every cursor movement, and every text content > change. > None of those affects their bounds (at least, in my > experience, these > controls are not continuously resizing themselves when I > scroll or type > things...). TextField will even change its cursor shape every > time its > value is updated, even if that value is simply bound to a > Slider and the > field doesn't have focus at all -- this field will then > trigger layout > on itself and all its ancestors even if it is in a completely > unrelated > area of the UI (not close to the slider). > > It seems that in many cases these controls and skins just want > their > layoutChildren method to be called, as their main layout logic is > located there -- duplicating this logic partially for every minor > property change that doesn't affect its bounds is error prone, > so I can > completely follow this reasoning. However, using > requestLayout to get > layoutChildren called is very expensive. > > There is a better way: call setNeedsLayout(true) -- this is a > protected > method that any Node has access to, and basically will only call > layoutChildren on your own Node. It marks all the parent nodes as > `DIRTY_BRANCH`, which means that on a layout pass it will > traverse down > to see which nodes actually needs layout (it won't call > layoutChildren > for each ancestor, which is a big win). > > Because of its protected nature (and its required parameter > which must > be true), it is a bit hard to use. I'm thinking it might be a > good idea > to introduce a new method here, a request layout call that > schedules a > Node for layout without forcing all ancestors to do the same. > This way > Skin and Control designers can clearly see the two options and > choose > what is required: > > requestLayout -- my bounds likely have changed (font change, > border/padding change, spacing change), so please call compute > methods > and redo the entire layout > > requestLocalLayout -- my bounds have not changed (color > changes, > background changes, content changes within a ScrollPane, > cursor changes, > cursor position changes, alignment changes) > > What do you think? > > --John >
