Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ASLayoutElementContext] Pending Layout should be in thread local storage #329

Open
hannahmbanana opened this issue Jun 5, 2017 · 2 comments

Comments

@hannahmbanana
Copy link
Contributor

hannahmbanana commented Jun 5, 2017

Issue: For any node, there is only one instance variable for pending layout (and when set, its constrained size is used for any new layout that is starting). This variable is thread safe, but it doesn't make sense for multiple threads to be using it at the same time.

Example diagram below. The main thread would be better off using size0, the last committed layout, than size1, which may be set in the middle of an unfinished layout pass.

img_5473 jpg

This is important because the flexbox layout calculation (e.g. ASStackLayoutSpec always generates multiple pending layouts, and capturing one of these intermediate size constraints on another thread might produce incorrect results.

Proposed Solution: ASLayoutElementContext already exists per thread. Move pending layout (which contains the last constrained size) to this ASLayoutElementContext and delete the node's pending layout instance variable. Because there is only one ASLayoutElementContext per thread, a map from node to pending layout will be needed so that the pending layout for each node being used on that thread is stored / can be referenced.

Ideas per discussion between @nguyenhuy @Adlai-Holler @appleguy.

@hannahmbanana
Copy link
Contributor Author

hannahmbanana commented Jun 6, 2017

Looks like ASLayoutElementContext is a struct and not an object. :(

@hannahmbanana hannahmbanana changed the title [ASLayoutContext] Pending Layout should be in thread local storage [ASLayoutElementContext] Pending Layout should be in thread local storage Jun 6, 2017
@Adlai-Holler
Copy link
Member

cc #344

hannahmbanana added a commit to hannahmbanana/Texture that referenced this issue Jun 13, 2017
…rage.

Preliminary stab at issue TextureGroup#329. It builds, but -[ASAbsoluteLayoutSpecSnapshotTests testChildrenMeasuredWithAutoMaxSize]` test case fails.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants