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.

Sign up for GitHub

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

Remove reliance on shared_ptr for ASDisplayNodeLayouts #1131

Merged
merged 7 commits into from
Sep 19, 2018
Merged

Remove reliance on shared_ptr for ASDisplayNodeLayouts #1131

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
80f22c5
Remove reliance on shared_ptr for ASDisplayNodeLayouts
Adlai-Holler Sep 19, 2018
ee86467
Fix up
Adlai-Holler Sep 19, 2018
de6525e
Fix in yoga
Adlai-Holler Sep 19, 2018
2c97488
Merge branch 'master' into AHRemoveSharedPtr
Adlai-Holler Sep 19, 2018
444a2f9
Back to let
Adlai-Holler Sep 19, 2018
db64c07
Returns inner pointer
Adlai-Holler Sep 19, 2018
026d62e
Trivial change to kick the CI
Adlai-Holler Sep 19, 2018
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
4 changes: 0 additions & 4 deletions AsyncDisplayKit.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@
6947B0C01E36B4E30007C478 /* ASStackUnpositionedLayout.mm in Sources */ = {isa = PBXBuildFile; fileRef = 6947B0BD1E36B4E30007C478 /* ASStackUnpositionedLayout.mm */; };
6947B0C31E36B5040007C478 /* ASStackPositionedLayout.h in Headers */ = {isa = PBXBuildFile; fileRef = 6947B0C11E36B5040007C478 /* ASStackPositionedLayout.h */; settings = {ATTRIBUTES = (Private, ); }; };
6947B0C51E36B5040007C478 /* ASStackPositionedLayout.mm in Sources */ = {isa = PBXBuildFile; fileRef = 6947B0C21E36B5040007C478 /* ASStackPositionedLayout.mm */; };
6959433F1D70815300B0EE1F /* ASDisplayNodeLayout.mm in Sources */ = {isa = PBXBuildFile; fileRef = 6959433C1D70815300B0EE1F /* ASDisplayNodeLayout.mm */; };
695943401D70815300B0EE1F /* ASDisplayNodeLayout.h in Headers */ = {isa = PBXBuildFile; fileRef = 6959433D1D70815300B0EE1F /* ASDisplayNodeLayout.h */; settings = {ATTRIBUTES = (Private, ); }; };
695BE2551DC1245C008E6EA5 /* ASWrapperSpecSnapshotTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 695BE2541DC1245C008E6EA5 /* ASWrapperSpecSnapshotTests.mm */; };
696F01EC1DD2AF450049FBD5 /* ASEventLog.h in Headers */ = {isa = PBXBuildFile; fileRef = 696F01EA1DD2AF450049FBD5 /* ASEventLog.h */; settings = {ATTRIBUTES = (Public, ); }; };
Expand Down Expand Up @@ -705,7 +704,6 @@
6947B0BD1E36B4E30007C478 /* ASStackUnpositionedLayout.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASStackUnpositionedLayout.mm; sourceTree = "<group>"; };
6947B0C11E36B5040007C478 /* ASStackPositionedLayout.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASStackPositionedLayout.h; sourceTree = "<group>"; };
6947B0C21E36B5040007C478 /* ASStackPositionedLayout.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASStackPositionedLayout.mm; sourceTree = "<group>"; };
6959433C1D70815300B0EE1F /* ASDisplayNodeLayout.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASDisplayNodeLayout.mm; sourceTree = "<group>"; };
6959433D1D70815300B0EE1F /* ASDisplayNodeLayout.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASDisplayNodeLayout.h; sourceTree = "<group>"; };
695BE2541DC1245C008E6EA5 /* ASWrapperSpecSnapshotTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASWrapperSpecSnapshotTests.mm; sourceTree = "<group>"; };
696F01EA1DD2AF450049FBD5 /* ASEventLog.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASEventLog.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1490,7 +1488,6 @@
690BC8C020F6D3490052A434 /* ASDisplayNodeCornerLayerDelegate.m */,
058D0A0C195D050800B7D73C /* ASDisplayNodeInternal.h */,
6959433D1D70815300B0EE1F /* ASDisplayNodeLayout.h */,
6959433C1D70815300B0EE1F /* ASDisplayNodeLayout.mm */,
CCA282C61E9EB64B0037E8B7 /* ASDisplayNodeTipState.h */,
CCA282C71E9EB64B0037E8B7 /* ASDisplayNodeTipState.m */,
68B8A4DB1CBD911D007E4543 /* ASImageNode+AnimatedImagePrivate.h */,
Expand Down Expand Up @@ -2497,7 +2494,6 @@
DB78412E1C6BCE1600A9E2B4 /* _ASTransitionContext.m in Sources */,
B350620B1B010EFD0018CF92 /* ASTableView.mm in Sources */,
B350620E1B010EFD0018CF92 /* ASTextNode.mm in Sources */,
6959433F1D70815300B0EE1F /* ASDisplayNodeLayout.mm in Sources */,
68355B3E1CB57A60001D4E68 /* ASPINRemoteImageDownloader.m in Sources */,
CC034A141E649F1300626263 /* AsyncDisplayKit+IGListKitMethods.m in Sources */,
509E68661B3AEDD7009B9150 /* CoreGraphics+ASConvenience.m in Sources */,
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
- Small optimization to the layout spec & yoga layout systems by eliminating array copies. [Adlai Holler](https://github.com/Adlai-Holler)
- Optimize layout process by removing `ASRectMap`. [Adlai Holler](https://github.com/Adlai-Holler)
- Remove necessity to use view to access rangeController in ASTableNode, ASCollectionNode. [Michael Schneider](https://github.com/maicki)
- Remove display node's reliance on shared_ptr. [Adlai Holler](https://github.com/Adlai-Holler)

## 2.7
- Fix pager node for interface coalescing. [Max Wang](https://github.com/wsdwsd0829) [#877](https://github.com/TextureGroup/Texture/pull/877)
Expand Down
124 changes: 61 additions & 63 deletions Source/ASDisplayNode+Layout.mm
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Source/ASDisplayNode+Yoga.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ - (void)setupYogaCalculatedLayout
// 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);
_pendingDisplayNodeLayout = ASDisplayNodeLayout(layout, range, parentSize, _layoutVersion);
}
}

Expand Down
24 changes: 10 additions & 14 deletions Source/ASDisplayNode.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,6 @@ - (void)_initializeInstance

_primitiveTraitCollection = ASPrimitiveTraitCollectionMakeDefault();

_calculatedDisplayNodeLayout = std::make_shared<ASDisplayNodeLayout>();
_pendingDisplayNodeLayout = nullptr;
_layoutVersion = 1;

_defaultLayoutTransitionDuration = 0.2;
Expand Down Expand Up @@ -3812,21 +3810,19 @@ - (NSString *)detailedLayoutDescription
[props addObject:@{ @"layoutVersion": @(_layoutVersion.load()) }];
[props addObject:@{ @"bounds": [NSValue valueWithCGRect:self.bounds] }];

if (_calculatedDisplayNodeLayout != nullptr) {
ASDisplayNodeLayout c = *_calculatedDisplayNodeLayout;
[props addObject:@{ @"calculatedLayout": c.layout }];
[props addObject:@{ @"calculatedVersion": @(c.version) }];
[props addObject:@{ @"calculatedConstrainedSize" : NSStringFromASSizeRange(c.constrainedSize) }];
if (c.requestedLayoutFromAbove) {
if (_calculatedDisplayNodeLayout.layout) {
[props addObject:@{ @"calculatedLayout": _calculatedDisplayNodeLayout.layout }];
[props addObject:@{ @"calculatedVersion": @(_calculatedDisplayNodeLayout.version) }];
[props addObject:@{ @"calculatedConstrainedSize" : NSStringFromASSizeRange(_calculatedDisplayNodeLayout.constrainedSize) }];
if (_calculatedDisplayNodeLayout.requestedLayoutFromAbove) {
[props addObject:@{ @"calculatedRequestedLayoutFromAbove": @"YES" }];
}
}
if (_pendingDisplayNodeLayout != nullptr) {
ASDisplayNodeLayout p = *_pendingDisplayNodeLayout;
[props addObject:@{ @"pendingLayout": p.layout }];
[props addObject:@{ @"pendingVersion": @(p.version) }];
[props addObject:@{ @"pendingConstrainedSize" : NSStringFromASSizeRange(p.constrainedSize) }];
if (p.requestedLayoutFromAbove) {
if (_pendingDisplayNodeLayout.layout) {
[props addObject:@{ @"pendingLayout": _pendingDisplayNodeLayout.layout }];
[props addObject:@{ @"pendingVersion": @(_pendingDisplayNodeLayout.version) }];
[props addObject:@{ @"pendingConstrainedSize" : NSStringFromASSizeRange(_pendingDisplayNodeLayout.constrainedSize) }];
if (_pendingDisplayNodeLayout.requestedLayoutFromAbove) {
[props addObject:@{ @"pendingRequestedLayoutFromAbove": (id)kCFNull }];
}
}
Expand Down
4 changes: 2 additions & 2 deletions Source/Private/ASDisplayNodeInternal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ AS_EXTERN NSString * const ASRenderingEngineDidDisplayNodesScheduledBeforeTimest

std::atomic<int32_t> _pendingTransitionID;
ASLayoutTransition *_pendingLayoutTransition;
std::shared_ptr<ASDisplayNodeLayout> _calculatedDisplayNodeLayout;
std::shared_ptr<ASDisplayNodeLayout> _pendingDisplayNodeLayout;
ASDisplayNodeLayout _calculatedDisplayNodeLayout;
ASDisplayNodeLayout _pendingDisplayNodeLayout;

/// Sentinel for layout data. Incremented when we get -setNeedsLayout / -invalidateCalculatedLayout.
/// Starts at 1.
Expand Down
10 changes: 8 additions & 2 deletions Source/Private/ASDisplayNodeLayout.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,16 @@ struct ASDisplayNodeLayout {
/**
* Returns whether this is valid for a given version
*/
BOOL isValid(NSUInteger version);
BOOL isValid(NSUInteger versionArg) {
Copy link
Contributor

@maicki maicki Sep 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You think it's worth it to inline?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figure there's no real downside. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep agree

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

return layout != nil && version >= versionArg;
}

/**
* Returns whether this is valid for a given constrained size, parent size, and version
*/
BOOL isValid(ASSizeRange constrainedSize, CGSize parentSize, NSUInteger version);
BOOL isValid(ASSizeRange theConstrainedSize, CGSize theParentSize, NSUInteger versionArg) {
return isValid(versionArg)
&& CGSizeEqualToSize(parentSize, theParentSize)
&& ASSizeRangeEqualToSizeRange(constrainedSize, theConstrainedSize);
}
};
22 changes: 0 additions & 22 deletions Source/Private/ASDisplayNodeLayout.mm
View file
Open in desktop

This file was deleted.

10 changes: 4 additions & 6 deletions Source/Private/ASLayoutTransition.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
#import <AsyncDisplayKit/ASDisplayNode.h>
#import <AsyncDisplayKit/ASLayoutSpec.h>

#import <memory>

NS_ASSUME_NONNULL_BEGIN

#pragma mark - ASLayoutElementTransition
Expand Down Expand Up @@ -52,12 +50,12 @@ AS_SUBCLASSING_RESTRICTED
/**
* Previous layout to transition from
*/
@property (nonatomic, readonly) std::shared_ptr<ASDisplayNodeLayout> previousLayout;
@property (nonatomic, readonly) const ASDisplayNodeLayout &previousLayout NS_RETURNS_INNER_POINTER;

/**
* Pending layout to transition to
*/
@property (nonatomic, readonly) std::shared_ptr<ASDisplayNodeLayout> pendingLayout;
@property (nonatomic, readonly) const ASDisplayNodeLayout &pendingLayout NS_RETURNS_INNER_POINTER;

/**
* Returns if the layout transition needs to happen synchronously
Expand All @@ -68,8 +66,8 @@ AS_SUBCLASSING_RESTRICTED
* Returns a newly initialized layout transition
*/
- (instancetype)initWithNode:(ASDisplayNode *)node
pendingLayout:(std::shared_ptr<ASDisplayNodeLayout>)pendingLayout
previousLayout:(std::shared_ptr<ASDisplayNodeLayout>)previousLayout NS_DESIGNATED_INITIALIZER;
pendingLayout:(const ASDisplayNodeLayout &)pendingLayout
previousLayout:(const ASDisplayNodeLayout &)previousLayout NS_DESIGNATED_INITIALIZER;

/**
* Insert and remove subnodes that were added or removed between the previousLayout and the pendingLayout
Expand Down
20 changes: 11 additions & 9 deletions Source/Private/ASLayoutTransition.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,13 @@ @implementation ASLayoutTransition {
NSArray<ASDisplayNode *> *_removedSubnodes;
std::vector<NSUInteger> _insertedSubnodePositions;
std::vector<std::pair<ASDisplayNode *, NSUInteger>> _subnodeMoves;
ASDisplayNodeLayout _pendingLayout;
ASDisplayNodeLayout _previousLayout;
}

- (instancetype)initWithNode:(ASDisplayNode *)node
pendingLayout:(std::shared_ptr<ASDisplayNodeLayout>)pendingLayout
previousLayout:(std::shared_ptr<ASDisplayNodeLayout>)previousLayout
pendingLayout:(const ASDisplayNodeLayout &)pendingLayout
previousLayout:(const ASDisplayNodeLayout &)previousLayout
{
self = [super init];
if (self) {
Expand All @@ -80,7 +82,7 @@ - (instancetype)initWithNode:(ASDisplayNode *)node
- (BOOL)isSynchronous
{
ASDN::MutexSharedLocker l(__instanceLock__);
return !ASLayoutCanTransitionAsynchronous(_pendingLayout->layout);
return !ASLayoutCanTransitionAsynchronous(_pendingLayout.layout);
}

- (void)commitTransition
Expand Down Expand Up @@ -156,8 +158,8 @@ - (void)calculateSubnodeOperationsIfNeeded

// Create an activity even if no subnodes affected.
as_activity_create_for_scope("Calculate subnode operations");
ASLayout *previousLayout = _previousLayout->layout;
ASLayout *pendingLayout = _pendingLayout->layout;
ASLayout *previousLayout = _previousLayout.layout;
ASLayout *pendingLayout = _pendingLayout.layout;

if (previousLayout) {
#if AS_IG_LIST_KIT
Expand Down Expand Up @@ -226,9 +228,9 @@ - (ASLayout *)transitionContext:(_ASTransitionContext *)context layoutForKey:(NS
{
ASDN::MutexSharedLocker l(__instanceLock__);
if ([key isEqualToString:ASTransitionContextFromLayoutKey]) {
return _previousLayout->layout;
return _previousLayout.layout;
} else if ([key isEqualToString:ASTransitionContextToLayoutKey]) {
return _pendingLayout->layout;
return _pendingLayout.layout;
} else {
return nil;
}
Expand All @@ -238,9 +240,9 @@ - (ASSizeRange)transitionContext:(_ASTransitionContext *)context constrainedSize
{
ASDN::MutexSharedLocker l(__instanceLock__);
if ([key isEqualToString:ASTransitionContextFromLayoutKey]) {
return _previousLayout->constrainedSize;
return _previousLayout.constrainedSize;
} else if ([key isEqualToString:ASTransitionContextToLayoutKey]) {
return _pendingLayout->constrainedSize;
return _pendingLayout.constrainedSize;
} else {
return ASSizeRangeMake(CGSizeZero, CGSizeZero);
}
Expand Down