Measure with exact measurement when stretch is defined #193

2016-06-02T09:08:29-07:00

emilsjolander commented

2016-06-02 09:08:29 -07:00

(Migrated from github.com)

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.

emilsjolander commented

2016-06-02 09:17:22 -07:00

(Migrated from github.com)

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.

emilsjolander commented

2016-06-02 09:24:47 -07:00

(Migrated from github.com)

@rigdern Would love it if you took a look

rigdern commented

2016-06-02 10:04:24 -07:00

(Migrated from github.com)

@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 ?w=1 to the end of the URL. For example: https://github.com/facebook/css-layout/pull/193/files?w=1. However, I don't think you can write or view comments in this view :(

@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 `?w=1` to the end of the URL. For example: https://github.com/facebook/css-layout/pull/193/files?w=1. However, I don't think you can write or view comments in this view :(

emilsjolander commented

2016-06-02 10:36:35 -07:00

(Migrated from github.com)

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>

View defaults to alignItems: 'stretch' which previous to this diff would initially measure children with AT_MOST width. Not only is this wrong as they should be measured with an exact width due to there alignment it is also bad for performance as the text itself needs to figure out how big it is instead of being told a size.

I tried to describe it in the PR description but I'll expand on it a bit. In a simple layout such as ``` html <View style={{width: 1000}}> <Text>zZzZ</Text> <Text>zZzZzZzZ</Text> <Text>zZzZzZzZzZzZ</Text> </View> ``` `View` defaults to `alignItems: 'stretch'` which previous to this diff would initially measure children with `AT_MOST` width. Not only is this wrong as they should be measured with an exact width due to there alignment it is also bad for performance as the text itself needs to figure out how big it is instead of being told a size.

rigdern commented

2016-06-07 20:35:31 -07:00

(Migrated from github.com)

@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.

@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.

emilsjolander commented

2016-06-07 20:40:16 -07:00

(Migrated from github.com)

Thanks Adam!
On Tue, 7 Jun 2016 at 23:35, Adam Comella notifications@github.com wrote:

@emilsjolander https://github.com/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.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/facebook/css-layout/pull/193#issuecomment-224478929,
or mute the thread
https://github.com/notifications/unsubscribe/ABdIpG-GDc3bUTJjTvxucFyBt20h45xjks5qJjiHgaJpZM4IsuJM
.

Thanks Adam! On Tue, 7 Jun 2016 at 23:35, Adam Comella notifications@github.com wrote: > @emilsjolander https://github.com/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. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > https://github.com/facebook/css-layout/pull/193#issuecomment-224478929, > or mute the thread > https://github.com/notifications/unsubscribe/ABdIpG-GDc3bUTJjTvxucFyBt20h45xjks5qJjiHgaJpZM4IsuJM > .

lucasr commented

2016-06-08 10:04:49 -07:00

(Migrated from github.com)

Looks good with nits fixed.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#193