-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[ASDisplayNode] Consolidate main thread initialization and allow apps to invoke it manually instead of +load. #798
Changes from 2 commits
97f3417
1f1ff26
6a7e0c0
bd11435
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -157,6 +157,8 @@ - (ASLayout *)layoutForYogaNode | |
|
||
- (void)setupYogaCalculatedLayout | ||
{ | ||
ASDN::MutexLocker l(__instanceLock__); | ||
|
||
YGNodeRef yogaNode = self.style.yogaNode; | ||
uint32_t childCount = YGNodeGetChildCount(yogaNode); | ||
ASDisplayNodeAssert(childCount == self.yogaChildren.count, | ||
|
@@ -210,7 +212,10 @@ - (void)setupYogaCalculatedLayout | |
parentSize.width = YGNodeLayoutGetWidth(parentNode); | ||
parentSize.height = YGNodeLayoutGetHeight(parentNode); | ||
} | ||
_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; | ||
_pendingDisplayNodeLayout = std::make_shared<ASDisplayNodeLayout>(layout, range, parentSize, _layoutVersion); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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. |
||
} | ||
} | ||
|
||
|
@@ -235,9 +240,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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
BOOL needsTemporaryMeasureFunc = (YGNodeGetMeasureFunc(yogaNode) == NULL); | ||
if (needsTemporaryMeasureFunc) { | ||
ASDisplayNodeAssert(self.yogaLayoutInProgress == NO, | ||
@"shouldHaveYogaMeasureFunc == YES, and inside a layout pass, but no measure func pointer! %@", self); | ||
YGNodeSetMeasureFunc(yogaNode, &ASLayoutElementYogaMeasureFunc); | ||
} | ||
YGNodeMarkDirty(yogaNode); | ||
if (needsTemporaryMeasureFunc) { | ||
YGNodeSetMeasureFunc(yogaNode, NULL); | ||
} | ||
} | ||
self.yogaCalculatedLayout = nil; | ||
} | ||
|
@@ -305,7 +319,7 @@ - (void)calculateLayoutFromYogaRoot:(ASSizeRange)rootConstrainedSize | |
NSLog(@"node = %@", node); | ||
NSLog(@"style = %@", node.style); | ||
NSLog(@"layout = %@", node.yogaCalculatedLayout); | ||
YGNodePrint(node.style.yogaNode, (YGPrintOptions)(YGPrintOptionsStyle | YGPrintOptionsLayout)); | ||
YGNodePrint(node.yogaNode, (YGPrintOptions)(YGPrintOptionsStyle | YGPrintOptionsLayout)); | ||
}); | ||
} | ||
#endif /* YOGA_LAYOUT_LOGGING */ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
{ | ||
ASInitializeFrameworkMainThread(); | ||
} | ||
#endif | ||
|
||
+ (Class)viewClass | ||
{ | ||
return [_ASDisplayView class]; | ||
|
@@ -629,6 +636,8 @@ - (void)_didLoad | |
for (ASDisplayNodeDidLoadBlock block in onDidLoadBlocks) { | ||
block(self); | ||
} | ||
|
||
[_interfaceStateDelegate nodeDidLoad]; | ||
} | ||
|
||
- (void)didLoad | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,29 @@ | |
#import <AsyncDisplayKit/ASRunLoopQueue.h> | ||
#import <AsyncDisplayKit/ASThread.h> | ||
|
||
static BOOL defaultAllowsGroupOpacity = YES; | ||
static BOOL defaultAllowsEdgeAntialiasing = NO; | ||
|
||
void ASInitializeFrameworkMainThread(void) | ||
{ | ||
ASDisplayNodeThreadIsMain(); | ||
// Ensure this value is cached on the main thread before needed in the background. | ||
ASScreenScale(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's remove this. I don't think it's likely the change will ever be reverted. |
||
CALayer *layer = [[[UIView alloc] init] layer]; | ||
defaultAllowsGroupOpacity = layer.allowsGroupOpacity; | ||
defaultAllowsEdgeAntialiasing = layer.allowsEdgeAntialiasing; | ||
} | ||
|
||
BOOL ASDefaultAllowsGroupOpacity(void) | ||
{ | ||
return defaultAllowsGroupOpacity; | ||
} | ||
|
||
BOOL ASDefaultAllowsEdgeAntialiasing(void) | ||
{ | ||
return defaultAllowsEdgeAntialiasing; | ||
} | ||
|
||
BOOL ASSubclassOverridesSelector(Class superclass, Class subclass, SEL selector) | ||
{ | ||
if (superclass == subclass) return NO; // Even if the class implements the selector, it doesn't override itself. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.