measure_func is called too many times when minWidth/maxWidth is involved

[vkurchatkin-miro]

taffy version

0.3.13

Platform

WASM

What you did

I can try to make a standalone reproduction, but here is a structure that I have:

<Box
    style={{
	    minWidth: 116,
	    maxWidth: 176,
	    minHeight: 108,
	    borderColor: '#e7e7e7',
	    backgroundColor: 'white',
	    borderWidth: 1,
	    borderRadius: 4,
	    paddingVertical: 12,
	    paddingHorizontal: 16,
	    flexDirection: 'column',
    }}
    >
    
    <Text
	    text={text}
	    style={{fontSize: 12, fontWeight: 'bold', lineHeight: 18, marginBottom: 12}}
    />
</Box>

What went wrong

measure_func is called 7 times. Here is a log of calls:

Called measure_func
 known_dimensions Size { width: None, height: None }
 available_space Size { width: MaxContent, height: MaxContent }


Called measure_func
 known_dimensions Size { width: None, height: None }
 available_space Size { width: MaxContent, height: MinContent }


Called measure_func
 known_dimensions Size { width: None, height: None }
 available_space Size { width: MaxContent, height: MaxContent }


 Called measure_func
 known_dimensions Size { width: None, height: None }
 available_space Size { width: MaxContent, height: MinContent }


Called measure_func
 known_dimensions Size { width: None, height: Some(22.0) }
 available_space Size { width: MinContent, height: Definite(22.0) }


Called measure_func
 known_dimensions Size { width: Some(82.0), height: None }
 available_space Size { width: Definite(82.0), height: Definite(70.0) }


Called measure_func
 known_dimensions Size { width: None, height: None }
 available_space Size { width: Definite(82.0), height: MinContent }

Some of the calls make no sense at all, e.g. after there is a known height it still gets called without known height. If I replace minWidth and maxWidth with width I only get 3 calls:

Called measure_func
 known_dimensions Size { width: Some(82.0), height: None }
 available_space Size { width: Definite(82.0), height: MaxContent }

 Called measure_func
 known_dimensions Size { width: None, height: None }
 available_space Size { width: Definite(82.0), height: MinContent }

Called measure_func
 known_dimensions Size { width: Some(82.0), height: None }
 available_space Size { width: Definite(82.0), height: Definite(70.0) }

Although it still doesn't make much sense why there is a call without known height after there is a call with it.

Additional information

Text layout is very resource demanding and I would really like to reduce calls to it as much as possible. It is unclear to me how it should work with minWidth/maxWidth (I understand if spec demands some shenaningans in this case, but 7 calls seem excessive), but in case of predefined width one call is definitely enough to figure the final.

For now it seems like the only way is caching, but even that is unpleasant, since the only way to pass something to measure_func is Rc<RefCell<>> or something like this

[nicoburns]

@vkurchatkin-miro You can try this branch #556. However, I would probably still recommend using your own caching for text. #490 should allow you to do this without Rc<RefCell<_>>.

Also, are you trying to use a browser's text layout with taffy-in-wasm? Because I suspect that will always be quite slow. If you use something like cosmic-text for text layout then I believe text layout should be relatively cheap (as splitting the text into runs can happen once, and relayout just needs to pack those runs into the specified width constraint).

[vkurchatkin-miro]

@nicoburns do you recommend switching to main in general? Because in order to try #556 I would have to refactor to support main anyway.

#490 should allow you to do this without Rc<RefCell<_>>.

Sure, that would be great when it is merged

If you use something like cosmic-text for text layout then I believe text layout should be relatively cheap (as splitting the text into runs can happen once, and relayout just needs to pack those runs into the specified width constraint).

I use fontdue for now and it doesn't do that, unfortunately, it always lays out from scratch. It seems like it is not an appropriate implementation if Taffy requests multiple configurations anyway, I'm going to try cosmic-text

Because I suspect that will always be quite slow

What makes you think that? I mean, it is slower then I would like it to be, but Taffy itself seems significantly slower than I expect it to be as well. That is in general very unfortunate, because in my current stress test involving 50000 nodes (~15000 text nodes) layout takes roughly 90% of all CPU time.

[vkurchatkin-miro]

Oh, by "browser's text layout" you mean something like measureText? Yeah, that I didn't even try, my overall idea is to minimize WASM<->JS calls

[alice-i-cecile]

We should be putting out a new release soon :) We're generally pretty active with those, but feel free to ping us if it's ever a hassle for you.

[vkurchatkin-miro]

Thanks. But there is still a question about this behaviour:

Called measure_func
 known_dimensions Size { width: Some(82.0), height: None }
 available_space Size { width: Definite(82.0), height: MaxContent }

 Called measure_func
 known_dimensions Size { width: None, height: None }
 available_space Size { width: Definite(82.0), height: MinContent }

Called measure_func
 known_dimensions Size { width: Some(82.0), height: None }
 available_space Size { width: Definite(82.0), height: Definite(70.0) }

2 and 3 call seem strictly redundant. During first call we already have known width and out of that we can figure out the appropriate height

[nicoburns]

2 and 3 call seem strictly redundant. During first call we already have known width and out of that we can figure out the appropriate height

That's often true, but not always. Consider the case of vertical text where the width is derived from the height rather than the other way around (a "column wrap" flexbox container also has this behaviour). I think it would be appropriate for you to implement your own cache for this (within the measure function) that takes advantage of the extra knowledge your code has about the content you are measuring.

[vkurchatkin-miro]

Thanks @nicoburns, it makes perfect sense, my statement is only valid if it is known that internal layout is left-to-right with wrapping. But still, what I fail to understand is how can known width turn into None on the next call? What does it even mean for it to be known?

I think it would be appropriate for you to implement your own cache for this (within the measure function) that takes advantage of the extra knowledge your code has about the content you are measuring.

I'm definitely going to do that.

[nicoburns]

But still, what I fail to understand is how can known width turn into None on the next call?

I believe that was actually a bug that was fixed by #545 (released in 0.3.14). I suspect you'll find the width is Some(82.0) for all 3 calls in the latest Taffy.

What does it even mean for it to be known?

It really means, "please layout the contents of this node and tell me what the height is assuming the the width is fixed at this size". This may be because the node has width style set, because stretch alignment is being applied, because you returned that size in a previous sizing, because the size was already determined through the flexing process, etc.