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
