Measure with exact measurement when stretch is defined#193
Conversation
|
I'm assuming my environment is set up differently than the person who last generated the code. That is why so many lines are whitespace removal. |
|
@rigdern Would love it if you took a look |
|
|
||
| // If child has no defined size in the cross axis and is set to stretch, set the cross | ||
| // axis to be measured exactly with the available inner width | ||
| if (!isMainAxisRow && !isUndefined(availableInnerWidth) && !isStyleDimDefined(child, CSS_FLEX_DIRECTION_ROW) && widthMeasureMode == CSS_MEASURE_MODE_EXACTLY && getAlignItem(node, child) == CSS_ALIGN_STRETCH) { |
There was a problem hiding this comment.
Maybe add a comment to indicate that these branches are for an optimization rather than a functional requirement.
There was a problem hiding this comment.
It is not just for perf though. It is also more correct to measure children with EXACTLY as that is what and alignment of stretch enforces.
|
@emilsjolander Where does the performance win come from? Do you have an example layout? In case you weren't aware, you can view the changes while ignoring whitespace by adding |
|
I tried to describe it in the PR description but I'll expand on it a bit. In a simple layout such as <View style={{width: 1000}}>
<Text>zZzZ</Text>
<Text>zZzZzZzZ</Text>
<Text>zZzZzZzZzZzZ</Text>
</View>
|
|
|
||
| // If child has no defined size in the cross axis and is set to stretch, set the cross | ||
| // axis to be measured exactly with the available inner width | ||
| if (!isMainAxisRow && !isUndefined(availableInnerWidth) && !isStyleDimDefined(child, CSS_FLEX_DIRECTION_ROW) && widthMeasureMode == CSS_MEASURE_MODE_EXACTLY && getAlignItem(node, child) == CSS_ALIGN_STRETCH) { |
There was a problem hiding this comment.
Break this down into multiple lines?
|
@emilsjolander Sorry for the delay. I tested this change today in our app and didn't notice any issues. Your change looks good to me. |
|
Thanks Adam!
|
| childWidth = updatedMainSize + getMarginAxis(currentRelativeChild, CSS_FLEX_DIRECTION_ROW); | ||
| childWidthMeasureMode = CSS_MEASURE_MODE_EXACTLY; | ||
|
|
||
| if (!isStyleDimDefined(currentRelativeChild, CSS_FLEX_DIRECTION_COLUMN)) { |
|
Looks good with nits fixed. |
Summary: Import latest master from css-layout. This includes the following pull requests: react/yoga#192 react/yoga#193 react/yoga#195 react/yoga#196 Reviewed By: javache Differential Revision: D3411535 fbshipit-source-id: 95bee9bd0282f98f6fafa15335b910b644395ad3
…en stretch is defined) react#193 react@2a816bf This doesn't make any more tests pass or fail.
3 participants
An optimization which will measure with EXACTLY when possible given stretch alignment. Text measurement specifically is much more performant when it knows the exact measurement up front and given that stretch alignment is the default this has quite a big performance impact on certain types of layouts.