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

[ASDisplayNode] Consolidate main thread initialization and allow apps to invoke it manually instead of +load. #798

Merged
merged 4 commits into from
Feb 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## master
* Add your own contributions to the next release on the line below this with your name.
- [ASDisplayNode] Consolidate main thread initialization and allow apps to invoke it manually instead of +load.
- [ASRunloopQueue] Introduce new runloop queue(ASCATransactionQueue) to coalesce Interface state update calls for view controller transitions.
- [ASRangeController] Fix stability of "minimum" rangeMode if the app has more than one layout before scrolling.
- **Important** ASDisplayNode's cornerRadius is a new thread-safe bridged property that should be preferred over CALayer's. Use the latter at your own risk! [Huy Nguyen](https://github.com/nguyenhuy) [#749](https://github.com/TextureGroup/Texture/pull/749).
Expand Down
2 changes: 2 additions & 0 deletions Source/ASDisplayNode+Beta.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@ extern void ASDisplayNodePerformBlockOnEveryYogaChild(ASDisplayNode * _Nullable
@interface ASLayoutElementStyle (Yoga)

- (YGNodeRef)yogaNodeCreateIfNeeded;
- (void)destroyYogaNode;

@property (nonatomic, assign, readonly) YGNodeRef yogaNode;

@property (nonatomic, assign, readwrite) ASStackLayoutDirection flexDirection;
Expand Down
7 changes: 7 additions & 0 deletions Source/ASDisplayNode+Subclasses.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ NS_ASSUME_NONNULL_BEGIN
*/
- (void)nodeDidLayout;

/**
* @abstract Called when the node loads.
* @discussion Can be used for operations that are performed after the node's view is available.
* @note This method is guaranteed to be called on main.
*/
- (void)nodeDidLoad;

@end

@interface ASDisplayNode (Subclassing) <ASInterfaceStateDelegate>
Expand Down
20 changes: 17 additions & 3 deletions Source/ASDisplayNode+Yoga.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Copy link
Member Author

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.

_pendingDisplayNodeLayout = std::make_shared<ASDisplayNodeLayout>(layout, range, parentSize, _layoutVersion);
Copy link
Member Author

Choose a reason for hiding this comment

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

}
}

Expand All @@ -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.
Copy link
Member Author

Choose a reason for hiding this comment

The 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;
}
Expand Down Expand Up @@ -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 */
Expand Down
9 changes: 9 additions & 0 deletions Source/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,13 @@ + (void)initialize
class_replaceMethod(self, @selector(_staticInitialize), staticInitialize, "v:@");
}

#if !AS_INITIALIZE_FRAMEWORK_MANUALLY
+ (void)load
Copy link
Member Author

Choose a reason for hiding this comment

The 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];
Expand Down Expand Up @@ -629,6 +636,8 @@ - (void)_didLoad
for (ASDisplayNodeDidLoadBlock block in onDidLoadBlocks) {
block(self);
}

[_interfaceStateDelegate nodeDidLoad];
}

- (void)didLoad
Expand Down
3 changes: 3 additions & 0 deletions Source/ASNodeController+Beta.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
- (void)loadNode;

// for descriptions see <ASInterfaceState> definition
- (void)nodeDidLoad ASDISPLAYNODE_REQUIRES_SUPER;
- (void)nodeDidLayout ASDISPLAYNODE_REQUIRES_SUPER;

- (void)didEnterVisibleState ASDISPLAYNODE_REQUIRES_SUPER;
- (void)didExitVisibleState ASDISPLAYNODE_REQUIRES_SUPER;

Expand Down
1 change: 1 addition & 0 deletions Source/ASNodeController+Beta.m
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ - (void)setShouldInvertStrongReference:(BOOL)shouldInvertStrongReference
}

// subclass overrides
- (void)nodeDidLoad {}
- (void)nodeDidLayout {}

- (void)didEnterVisibleState {}
Expand Down
1 change: 1 addition & 0 deletions Source/AsyncDisplayKit.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
//

#import <AsyncDisplayKit/ASAvailability.h>
#import <AsyncDisplayKit/ASBaseDefines.h>
#import <AsyncDisplayKit/ASDisplayNode.h>
#import <AsyncDisplayKit/ASDisplayNode+Ancestry.h>
#import <AsyncDisplayKit/ASDisplayNode+Convenience.h>
Expand Down
9 changes: 9 additions & 0 deletions Source/Private/ASInternalHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ NS_ASSUME_NONNULL_BEGIN

ASDISPLAYNODE_EXTERN_C_BEGIN

void ASInitializeFrameworkMainThread(void);

BOOL ASDefaultAllowsGroupOpacity(void);
BOOL ASDefaultAllowsEdgeAntialiasing(void);

BOOL ASSubclassOverridesSelector(Class superclass, Class subclass, SEL selector);
BOOL ASSubclassOverridesClassSelector(Class superclass, Class subclass, SEL selector);

Expand Down Expand Up @@ -100,3 +105,7 @@ ASDISPLAYNODE_INLINE void ASBoundsAndPositionForFrame(CGRect rect, CGPoint origi
@end

NS_ASSUME_NONNULL_END

#ifndef AS_INITIALIZE_FRAMEWORK_MANUALLY
#define AS_INITIALIZE_FRAMEWORK_MANUALLY 0
#endif
22 changes: 22 additions & 0 deletions Source/Private/ASInternalHelpers.m
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,28 @@
#import <AsyncDisplayKit/ASRunLoopQueue.h>
#import <AsyncDisplayKit/ASThread.h>

static BOOL defaultAllowsGroupOpacity = YES;
static BOOL defaultAllowsEdgeAntialiasing = NO;

void ASInitializeFrameworkMainThread(void)
{
ASDisplayNodeThreadIsMain();
// Ensure these values are cached on the main thread before needed in the background.
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.
Expand Down
17 changes: 2 additions & 15 deletions Source/Private/_ASPendingState.mm
Original file line number Diff line number Diff line change
Expand Up @@ -212,19 +212,6 @@ ASDISPLAYNODE_INLINE void ASPendingStateApplyMetricsToLayer(_ASPendingState *sta

static CGColorRef blackColorRef = NULL;
static UIColor *defaultTintColor = nil;
static BOOL defaultAllowsGroupOpacity = YES;
static BOOL defaultAllowsEdgeAntialiasing = NO;

+ (void)load
{
// Create temporary view to read default values that are based on linked SDK and Info.plist values
// Ensure this values cached on the main thread before needed
ASDisplayNodeCAssertMainThread();
UIView *view = [[UIView alloc] init];
defaultAllowsGroupOpacity = view.layer.allowsGroupOpacity;
defaultAllowsEdgeAntialiasing = view.layer.allowsEdgeAntialiasing;
}


- (instancetype)init
{
Expand All @@ -251,8 +238,8 @@ - (instancetype)init
tintColor = defaultTintColor;
isHidden = NO;
needsDisplayOnBoundsChange = NO;
allowsGroupOpacity = defaultAllowsGroupOpacity;
allowsEdgeAntialiasing = defaultAllowsEdgeAntialiasing;
allowsGroupOpacity = ASDefaultAllowsGroupOpacity();
allowsEdgeAntialiasing = ASDefaultAllowsEdgeAntialiasing();
autoresizesSubviews = YES;
alpha = 1.0f;
cornerRadius = 0.0f;
Expand Down