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

Jump to bottom

Readability improvements in ASDataController #trivial #1067

Merged
merged 1 commit into from
Aug 6, 2018
Merged

Readability improvements in ASDataController #trivial #1067

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
59 changes: 24 additions & 35 deletions Source/Details/ASDataController.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,13 @@ - (void)setLayoutDelegate:(id<ASDataControllerLayoutDelegate>)layoutDelegate

#pragma mark - Cell Layout

- (void)_allocateNodesFromElements:(NSArray<ASCollectionElement *> *)elements completion:(ASDataControllerCompletionBlock)completionHandler
- (void)_allocateNodesFromElements:(NSArray<ASCollectionElement *> *)elements
{
ASSERT_ON_EDITING_QUEUE;

NSUInteger nodeCount = elements.count;
__weak id<ASDataControllerSource> weakDataSource = _dataSource;
if (nodeCount == 0 || weakDataSource == nil) {
completionHandler();
return;
}

Expand All @@ -165,30 +164,23 @@ - (void)_allocateNodesFromElements:(NSArray<ASCollectionElement *> *)elements co
{
as_activity_create_for_scope("Data controller batch");

dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
// TODO: Should we use USER_INITIATED here since the user is probably waiting?
dispatch_queue_t queue = dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0);
ASDispatchApply(nodeCount, queue, 0, ^(size_t i) {
__strong id<ASDataControllerSource> strongDataSource = weakDataSource;
if (strongDataSource == nil) {
if (!weakDataSource) {
return;
}

// Allocate the node.
ASCollectionElement *context = elements[i];
ASCellNode *node = context.node;
if (node == nil) {
ASDisplayNodeAssertNotNil(node, @"Node block created nil node; %@, %@", self, strongDataSource);
node = [[ASCellNode alloc] init]; // Fallback to avoid crash for production apps.
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 know this is scary, but the exact same assertion and side-path lives inside ASCollectionElement. That class never returns nil from node.

}

unowned ASCollectionElement *element = elements[i];
unowned ASCellNode *node = element.node;
// Layout the node if the size range is valid.
ASSizeRange sizeRange = context.constrainedSize;
ASSizeRange sizeRange = element.constrainedSize;
if (ASSizeRangeHasSignificantArea(sizeRange)) {
[self _layoutNode:node withConstrainedSize:sizeRange];
}
});
}

completionHandler();
ASSignpostEndCustom(ASSignpostDataControllerBatch, self, 0, (weakDataSource != nil ? ASSignpostColorDefault : ASSignpostColorRed));
}

Expand Down Expand Up @@ -648,28 +640,9 @@ - (void)updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet
__block __unused os_activity_scope_state_s preparationScope = {}; // unused if deployment target < iOS10
as_activity_scope_enter(as_activity_create("Prepare nodes for collection update", AS_ACTIVITY_CURRENT, OS_ACTIVITY_FLAG_DEFAULT), &preparationScope);

dispatch_block_t completion = ^() {
[_mainSerialQueue performBlockOnMainThread:^{
as_activity_scope_leave(&preparationScope);
// Step 4: Inform the delegate
[_delegate dataController:self updateWithChangeSet:changeSet updates:^{
// Step 5: Deploy the new data as "completed"
//
// Note that since the backing collection view might be busy responding to user events (e.g scrolling),
// it will not consume the batch update blocks immediately.
// As a result, in a short intermidate time, the view will still be relying on the old data source state.
// Thus, we can't just swap the new map immediately before step 4, but until this update block is executed.
// (https://github.com/TextureGroup/Texture/issues/378)
self.visibleMap = newMap;
}];
}];
--_editingTransactionGroupCount;
};

// Step 3: Call the layout delegate if possible. Otherwise, allocate and layout all elements
if (canDelegate) {
[layoutDelegateClass calculateLayoutWithContext:layoutContext];
completion();
} else {
let elementsToProcess = [[NSMutableArray<ASCollectionElement *> alloc] init];
for (ASCollectionElement *element in newMap) {
Expand All @@ -682,8 +655,24 @@ - (void)updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet
[elementsToProcess addObject:element];
}
}
[self _allocateNodesFromElements:elementsToProcess completion:completion];
[self _allocateNodesFromElements:elementsToProcess];
}

// Step 4: Inform the delegate on main thread
[_mainSerialQueue performBlockOnMainThread:^{
as_activity_scope_leave(&preparationScope);
[_delegate dataController:self updateWithChangeSet:changeSet updates:^{
// Step 5: Deploy the new data as "completed"
//
// Note that since the backing collection view might be busy responding to user events (e.g scrolling),
// it will not consume the batch update blocks immediately.
// As a result, in a short intermidate time, the view will still be relying on the old data source state.
// Thus, we can't just swap the new map immediately before step 4, but until this update block is executed.
// (https://github.com/TextureGroup/Texture/issues/378)
self.visibleMap = newMap;
}];
}];
--_editingTransactionGroupCount;
});

if (_usesSynchronousDataLoading) {
Expand Down