[External] : Re: Unnecessary layouts; TLDR; new method "requestLocalLayout"

[John Hendrikx]

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.hendrikx at gmail.com>
> *Date: *Wednesday, April 16, 2025 at 16:13
> *To: *Nir Lisker <nlisker at gmail.com>, Andy Goryachev
> <andy.goryachev at oracle.com>
> *Cc: *openjfx-dev at openjdk.org <openjfx-dev at 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.goryachev at 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-retn at openjdk.org> on behalf
>         of John Hendrikx <john.hendrikx at gmail.com>
>         *Date: *Monday, April 14, 2025 at 08:56
>         *To: *openjfx-dev at openjdk.org <openjfx-dev at 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20250423/b3ab394e/attachment-0001.htm>