Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.

[ASDisplayNode] Placeholder Image bug fix #1392

Closed
hannahmbanana wants to merge 5 commits into from
Closed

[ASDisplayNode] Placeholder Image bug fix #1392

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e4348c0
Merge remote-tracking branch 'facebook/master'
hannahmbanana Mar 17, 2016
2d1a8ff
Merge remote-tracking branch 'facebook/master'
hannahmbanana Mar 17, 2016
19f0d59
Merge remote-tracking branch 'facebook/master'
hannahmbanana Mar 17, 2016
4756a55
[ASDisplayNode] Placeholder Image bug fix
hannahmbanana Mar 17, 2016
3f50112
Another [ASDisplayNode] bug fix
hannahmbanana Mar 17, 2016
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
27 changes: 18 additions & 9 deletions AsyncDisplayKit/ASDisplayNode.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1676,7 +1676,7 @@ - (void)_pendingNodeDidDisplay:(ASDisplayNode *)node
[_pendingDisplayNodes removeObject:node];

// only trampoline if there is a placeholder and nodes are done displaying
if ([self _pendingDisplayNodesHaveFinished] && _placeholderLayer.superlayer) {
if (self.layer.contents && [self _pendingDisplayNodesHaveFinished] && _placeholderLayer.superlayer) {
void (^cleanupBlock)() = ^{
[self _tearDownPlaceholderLayer];
};
Expand Down Expand Up @@ -2260,14 +2260,23 @@ - (void)displayWillStart
[self _pendingNodeWillDisplay:self];

[_supernode subnodeDisplayWillStart:self];

if (_placeholderImage && _placeholderLayer && self.layer.contents == nil) {
[CATransaction begin];
[CATransaction setDisableActions:YES];
[self _setupPlaceholderLayerContents];
_placeholderLayer.opacity = 1.0;
[CATransaction commit];
[self.layer addSublayer:_placeholderLayer];

if (_placeholderEnabled && self.layer.contents == nil && [self __implementsDisplay]) {
if (!_placeholderImage) {
Copy link
Contributor

@appleguy appleguy Mar 20, 2016

Choose a reason for hiding this comment

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

This code is almost exactly correct, although doing it at this point Will mean that the sun elements which never become fully visible on screen may do work to re-create their placeholders as they are starting to display because of entering the off screen display range.

Because placeholders are synchronous, it is important to minimize the amount of work that it Is done to support them. Perhaps more importantly, in reviewing this I noticed a few other bugs, most notably that the network image node will remove its placeholder for the image has finished downloading because a display pass is allowed to complete on it even though it has no contents to draw.

_placeholderImage = [self placeholderImage];
}
if (!_placeholderLayer) {
[self _setupPlaceholderLayerContents];
}
// from __completeLayoutCalculation
if (_placeholderImage && _placeholderLayer) {
[CATransaction begin];
[CATransaction setDisableActions:YES];
[self _setupPlaceholderLayerContents];
_placeholderLayer.opacity = 1.0;
[CATransaction commit];
[self.layer addSublayer:_placeholderLayer];
}
}
}

Expand Down