-
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
Optimize layout flattening #982
Changes from 2 commits
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 |
---|---|---|
|
@@ -55,22 +55,6 @@ ASDISPLAYNODE_INLINE AS_WARN_UNUSED_RESULT BOOL ASLayoutIsDisplayNodeType(ASLayo | |
return layout.type == ASLayoutElementTypeDisplayNode; | ||
} | ||
|
||
ASDISPLAYNODE_INLINE AS_WARN_UNUSED_RESULT BOOL ASLayoutIsFlattened(ASLayout *layout) | ||
{ | ||
// A layout is flattened if its position is null, and all of its sublayouts are of type displaynode with no sublayouts. | ||
if (! ASPointIsNull(layout.position)) { | ||
return NO; | ||
} | ||
|
||
for (ASLayout *sublayout in layout.sublayouts) { | ||
if (ASLayoutIsDisplayNodeType(sublayout) == NO || sublayout.sublayouts.count > 0) { | ||
return NO; | ||
} | ||
} | ||
|
||
return YES; | ||
} | ||
|
||
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. Had to make this a method so that it can see the |
||
@interface ASLayout () <ASDescriptionProvider> | ||
{ | ||
ASLayoutElementType _layoutElementType; | ||
|
@@ -216,52 +200,70 @@ - (void)setRetainSublayoutLayoutElements:(BOOL)retainSublayoutLayoutElements | |
|
||
#pragma mark - Layout Flattening | ||
|
||
- (BOOL)isFlattened | ||
{ | ||
// A layout is flattened if its position is null, and all of its sublayouts are of type displaynode with no sublayouts. | ||
if (! ASPointIsNull(_position)) { | ||
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. Remove whitespace? 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. Done |
||
return NO; | ||
} | ||
|
||
for (ASLayout *sublayout in _sublayouts) { | ||
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. Would 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. Fast enumeration doesn't retain the objects. |
||
if (ASLayoutIsDisplayNodeType(sublayout) == NO || sublayout->_sublayouts.count > 0) { | ||
return NO; | ||
} | ||
} | ||
|
||
return YES; | ||
} | ||
|
||
- (ASLayout *)filteredNodeLayoutTree NS_RETURNS_RETAINED | ||
{ | ||
if (ASLayoutIsFlattened(self)) { | ||
if ([self isFlattened]) { | ||
// All flattened layouts must have this flag enabled | ||
// to ensure sublayout elements are retained until the layouts are applied. | ||
self.retainSublayoutLayoutElements = YES; | ||
return self; | ||
} | ||
|
||
struct Context { | ||
ASLayout *layout; | ||
unowned ASLayout *layout; | ||
CGPoint absolutePosition; | ||
}; | ||
|
||
// Queue used to keep track of sublayouts while traversing this layout in a DFS fashion. | ||
std::deque<Context> queue; | ||
for (ASLayout *sublayout in self.sublayouts) { | ||
for (ASLayout *sublayout in _sublayouts) { | ||
queue.push_back({sublayout, sublayout.position}); | ||
} | ||
|
||
NSMutableArray *flattenedSublayouts = [[NSMutableArray alloc] init]; | ||
auto flattenedSublayouts = [[NSMutableArray<ASLayout *> alloc] init]; | ||
|
||
while (!queue.empty()) { | ||
const Context context = std::move(queue.front()); | ||
queue.pop_front(); | ||
|
||
ASLayout *layout = context.layout; | ||
const NSArray<ASLayout *> *sublayouts = layout.sublayouts; | ||
const NSUInteger sublayoutsCount = sublayouts.count; | ||
unowned ASLayout *layout = context.layout; | ||
const NSUInteger sublayoutsCount = layout->_sublayouts.count; | ||
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. Direct ivar access isn't so much about avoiding the message-send, it's about avoiding the retain/release pair. 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. Mind adding this comment in code? |
||
const CGPoint absolutePosition = context.absolutePosition; | ||
|
||
if (ASLayoutIsDisplayNodeType(layout)) { | ||
if (sublayoutsCount > 0 || CGPointEqualToPoint(ASCeilPointValues(absolutePosition), layout.position) == NO) { | ||
// Only create a new layout if the existing one can't be reused, which means it has either some sublayouts or an invalid absolute position. | ||
layout = [ASLayout layoutWithLayoutElement:layout.layoutElement | ||
size:layout.size | ||
position:absolutePosition | ||
sublayouts:@[]]; | ||
auto newLayout = [ASLayout layoutWithLayoutElement:layout->_layoutElement | ||
size:layout.size | ||
position:absolutePosition | ||
sublayouts:@[]]; | ||
[flattenedSublayouts addObject:newLayout]; | ||
} else { | ||
[flattenedSublayouts addObject:layout]; | ||
} | ||
[flattenedSublayouts addObject:layout]; | ||
} else if (sublayoutsCount > 0){ | ||
std::vector<Context> sublayoutContexts; | ||
for (ASLayout *sublayout in sublayouts) { | ||
sublayoutContexts.push_back({sublayout, absolutePosition + sublayout.position}); | ||
} else if (sublayoutsCount > 0) { | ||
// Fast-reverse-enumerate the sublayouts array by copying it into a C-array and push_front'ing each into the queue. | ||
unowned ASLayout *rawSublayouts[sublayoutsCount]; | ||
[layout->_sublayouts getObjects:rawSublayouts range:NSMakeRange(0, sublayoutsCount)]; | ||
for (NSInteger i = sublayoutsCount - 1; i >= 0; i--) { | ||
queue.push_front({rawSublayouts[i], absolutePosition + rawSublayouts[i].position}); | ||
} | ||
queue.insert(queue.cbegin(), sublayoutContexts.begin(), sublayoutContexts.end()); | ||
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. The bottom here (the search expansion) is far and away the biggest win. We no longer create another |
||
} | ||
} | ||
|
||
|
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.
Not sure if we should just be consistent and use
__unsafe_unretained
in a framework.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.
Pros:
Cons: