[JS] JavaScript Bindings missing for YGNodeHasNewLayout

[chrisregnier]

I'm wondering what others think about adding in some flags to mark which nodes were changed during the last calculateLayout. I believe a didChange and didTreeChange would be useful for walking the tree after the layout has been calculated and applying layout values to other systems.

didChange

Determines if any of the values in a node's calculated layout are different from before

didTreeChange

Determines if any of the node's descendants have changed during the last layout

I see these as the opposite of isDirty and isTreeDirty (which is basically what's requested here #425), so I suspect they could all be merged into bitflags to keep the memory footprint down. And I believe the did*Change flags should be easy to compute while calculating the layout since you'll have access to previous and new values at the same time to know if didChange is true, in which case you can set the parent's didTreeChange flag at the same time, so it's all O(1).

Use case

When I mark a node dirty and eventually do a calculateLayout, I then need to do a reflow event where I copy the layout from the nodes to the underlying system, but often most nodes haven't changed. I'd like to be able to walk down only the paths in the tree that have values that have changed and need to be transferred over. I think this could provide a huge performance improvement for any similar systems. Right now I have to keep an old version of all layouts around to compare with, and I have to walk over ALL nodes and compare them first to see if I need to copy values over.

Opinions?

[woehrl01]

Hi @chrisregnier thanks for your feature request. The feature you would like to have already exists. You simply have to use YGNodeGetHasNewLayout to detect which have changed and reset this via YGNodeSetHasNewLayout(node, false) after your specific handling.

What you don't have is a specific didTreeChange but the parent nodes also marked as having a new layout, so YGNodeSetHasNewLayout is the way to go.

You only need to traverse all the nodes which have those set.

[chrisregnier]

Thanks for the reply @woehrl01. I've just updated to the newest yoga-layout 1.8.0 version on npm (released yesterday) and this function doesn't look like it's available at all?

And you're right, with each of the parents in the tree marked true for YGNodeGetHasNewLayout, I get the information needed to walk down only specific trees, but I still think the info about whether that specific node has changed is also fairly relevant. As I mentioned before, you'd have to keep a duplicate of all layouts and repeat the dirty comparisons for each 'changed' node, whereas if it were baked into the main algorithm then I think it could be much more efficient for developers. And correct me if I'm wrong, but I also imagine most people are likely doing a similar algorithm to copy over node layouts (unless they use the values directly), so this isn't just a selfish "help me" request ;P So I still think the didTreeChange portion could be very useful.

[chrisregnier]

Just took a look through the yoga headers and I do see the decl for YGNodeGetHasNewLayout, and it appears that it's been there for a long time, but the javascript version doesn't give access to the property at all. So I'd love to see this property (and any others that are missing like setting context data) added to the javascript lib.

[woehrl01]

I see, it looks like that YGNodeGetHasNewLayout and YGNodeSetHasNewLayout hasn't been added to the js binding. So feel free to provide PR for this. 😉

YGNodeGetHasNewLayout is not only set for nodes which have new layouted children, it also is set for any node which has a new layout. You don't need to have a duplicate tree to detect which values have changed.

The react-native library, the java jni wrapper for yoga and many other projects are relying on that flag for exactly the same purpose you are suggesting.

You probably only need to adjust Node.cc and Node.hh similar to

    void markDirty(void);
    bool isDirty(void) const;

[Adlai-Holler]

Would it not be YGNodeSetHasNewLayout(node, false) to reset the flag? true means it was updated right?

[woehrl01]

@Adlai-Holler ups, of course! I updated my comment. Thank you!