Remove reliance on shared_ptr for ASDisplayNodeLayouts - #1131
Conversation
| constrainedSize, | ||
| constrainedSize.max, | ||
| newLayoutVersion); | ||
| var pendingLayout = ASDisplayNodeLayout(newLayout, |
There was a problem hiding this comment.
Do we change the pendingLayout somewhere to change it to var?
There was a problem hiding this comment.
You're right – in a previous version, setCalculatedLayout mutated the input (to apply the unflattenedLayout thing) but I changed it to copy the input and never committed reverting this to let.
| * Returns whether this is valid for a given version | ||
| */ | ||
| BOOL isValid(NSUInteger version); | ||
| BOOL isValid(NSUInteger versionArg) { |
There was a problem hiding this comment.
You think it's worth it to inline?
There was a problem hiding this comment.
I figure there's no real downside. What do you think?
| * Previous layout to transition from | ||
| */ | ||
| @property (nonatomic, readonly) std::shared_ptr<ASDisplayNodeLayout> previousLayout; | ||
| @property (nonatomic, readonly) const ASDisplayNodeLayout &previousLayout; |
There was a problem hiding this comment.
Are we sure the ref lifes for the full lifetime of the transition?
There was a problem hiding this comment.
It's an ivar on the transition object (it's copied in -init) so it'll live as long as the transition lives.
There was a problem hiding this comment.
There is a small risk here, which is you read this ref and store it/don't copy it, and then you release the transition while you're holding on to the ref. Our current usage is OK. We could actually change this to be a pointer, and use NS_RETURNS_INNER_POINTER to prevent that … I might try it.
EDIT: Apparently NS_RETURNS_INNER_POINTER works with C++ refs, so I added it.
| * Returns whether this is valid for a given version | ||
| */ | ||
| BOOL isValid(NSUInteger version); | ||
| BOOL isValid(NSUInteger versionArg) { |
…1131) * Remove reliance on shared_ptr for ASDisplayNodeLayouts * Fix up * Fix in yoga * Back to let * Returns inner pointer * Trivial change to kick the CI
3 participants
No description provided.