[ASDisplayNode] Consolidate main thread initialization and allow apps to invoke it manually instead of +load. #798
Conversation
…ps to invoke it manually instead of +load. Additionally this has a few minor fixes for Yoga support, and adds some basic but universally valuable callbacks like -nodeDidLoad to ASNodeController.
| _pendingDisplayNodeLayout = std::make_shared<ASDisplayNodeLayout>(layout, ASSizeRangeUnconstrained, parentSize, 0); | ||
| // For the root node in a Yoga tree, make sure to preserve the constrainedSize originally provided. | ||
| // This will be used for all relayouts triggered by children, since they escalate to root. | ||
| ASSizeRange range = parentNode ? ASSizeRangeUnconstrained : self.constrainedSizeForCalculatedLayout; |
There was a problem hiding this comment.
Without this fix, we had a pretty serious issue where a simple relayout would cause a Yoga tree to jump to a different layout, because it used Unconstrained instead of its original constrained size.
By calling through to self.constrainedSizeForCalculatedLayout, the behavior should be a pretty compatible integration with the Texture layout system.
My app has been running with these fixes to Yoga for several months in production, so at least for the limited scope of apps using Yoga, I'm pretty confident they are an improvement over prior behavior.
| @@ -235,9 +238,18 @@ - (void)updateYogaMeasureFuncIfNeeded | |||
| - (void)invalidateCalculatedYogaLayout | |||
| { | |||
| YGNodeRef yogaNode = self.style.yogaNode; | |||
| if (yogaNode && YGNodeGetMeasureFunc(yogaNode)) { | |||
| if (yogaNode && [self shouldHaveYogaMeasureFunc]) { | |||
| // Yoga internally asserts that MarkDirty() may only be called on nodes with a measurement function. | |||
There was a problem hiding this comment.
This is really quite ridiculous. In any case, this behavior was required after an earlier change that un-set the measurement function unless we are about to call into Yoga to do a measurement. This is beneficial for memory management and automaticallyManagesSubnodes, but means that we need to slightly abuse the API in order to ensure we are allowed to invalidate the layout on the long-lived YGNodeRef.
| @@ -226,6 +226,13 @@ + (void)initialize | |||
| class_replaceMethod(self, @selector(_staticInitialize), staticInitialize, "v:@"); | |||
| } | |||
|
|
|||
| #if !AS_INITIALIZE_FRAMEWORK_MANUALLY | |||
| + (void)load | |||
There was a problem hiding this comment.
This will run for all apps by default, but simply replaces a +load that used to be in _ASPendingState. Now it is also possible to defer this until after the app's critical request (often started before calling UIApplicationMain()), which does confer a measurable benefit to the amount of time for sending that request compared to performing activity in +load since it is the first thing to touch various parts of the binary.
Source/Private/ASInternalHelpers.m
Outdated
| { | ||
| ASDisplayNodeThreadIsMain(); | ||
| // Ensure this value is cached on the main thread before needed in the background. | ||
| ASScreenScale(); |
There was a problem hiding this comment.
Technically this is no longer needed with the new UIGraphicsContext-based implementation of ASScreenScale. We could remove this iff we're confident that change won't ever be reverted.
There was a problem hiding this comment.
Let's remove this. I don't think it's likely the change will ever be reverted.
| // For the root node in a Yoga tree, make sure to preserve the constrainedSize originally provided. | ||
| // This will be used for all relayouts triggered by children, since they escalate to root. | ||
| ASSizeRange range = parentNode ? ASSizeRangeUnconstrained : self.constrainedSizeForCalculatedLayout; | ||
| _pendingDisplayNodeLayout = std::make_shared<ASDisplayNodeLayout>(layout, range, parentSize, _layoutVersion); |
There was a problem hiding this comment.
@nguyenhuy is using _layoutVersion the correct way to handle this? I have actually not merged _layoutVersion into my branch and so was running this change without it, so this would be the biggest risk in the PR in my estimation.
|
@Adlai-Holler cool, ready for a final review! |
|
@maicki @Adlai-Holler Thanks guys! Could you hit merge for me? I'm not able to get past the CI being broken... |
… to invoke it manually instead of +load. (TextureGroup#798) * - [ASDisplayNode] Consolidate main thread initialization and allow apps to invoke it manually instead of +load. Additionally this has a few minor fixes for Yoga support, and adds some basic but universally valuable callbacks like -nodeDidLoad to ASNodeController. * Small fix for handling _layoutVersion. * Remove poking the scale accessor
Additionally this has a few minor fixes for Yoga support, and adds some basic but universally valuable callbacks like -nodeDidLoad to ASNodeController.