Skip to content

Commit

Permalink
Avoid setting frame on the backing store while holding a node's lock (T…
Browse files Browse the repository at this point in the history
…extureGroup#1048)

Doing so may trigger `-layer:didChangeBoundsWithOldValue:newValue:` on the layer's delegate (i.e `ASCALayerExtendedDelegate`) which then runs other operations that require the lock to be free.
  • Loading branch information
nguyenhuy authored and mikezucc committed Oct 2, 2018
1 parent b8c7e18 commit 146368e
Showing 1 changed file with 64 additions and 56 deletions.
120 changes: 64 additions & 56 deletions Source/Private/ASDisplayNode+UIViewBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -296,70 +296,78 @@ - (CGRect)frame

- (void)setFrame:(CGRect)rect
{
_bridge_prologue_write;
BOOL setToView = NO;
BOOL setToLayer = NO;
CGRect newBounds = CGRectZero;
CGPoint newPosition = CGPointZero;
BOOL nodeLoaded = NO;
BOOL isMainThread = ASDisplayNodeThreadIsMain();
{
_bridge_prologue_write;

// For classes like ASTableNode, ASCollectionNode, ASScrollNode and similar - make sure UIView gets setFrame:
struct ASDisplayNodeFlags flags = _flags;
BOOL specialPropertiesHandling = ASDisplayNodeNeedsSpecialPropertiesHandling(checkFlag(Synchronous), flags.layerBacked);
// For classes like ASTableNode, ASCollectionNode, ASScrollNode and similar - make sure UIView gets setFrame:
struct ASDisplayNodeFlags flags = _flags;
BOOL specialPropertiesHandling = ASDisplayNodeNeedsSpecialPropertiesHandling(checkFlag(Synchronous), flags.layerBacked);

nodeLoaded = _loaded(self);
if (!specialPropertiesHandling) {
BOOL canReadProperties = isMainThread || !nodeLoaded;
if (canReadProperties) {
// We don't have to set frame directly, and we can read current properties.
// Compute a new bounds and position and set them on self.
CALayer *layer = _layer;
CGPoint origin = (nodeLoaded ? layer.bounds.origin : self.bounds.origin);
CGPoint anchorPoint = (nodeLoaded ? layer.anchorPoint : self.anchorPoint);

ASBoundsAndPositionForFrame(rect, origin, anchorPoint, &newBounds, &newPosition);

if (ASIsCGRectValidForLayout(newBounds) == NO || ASIsCGPositionValidForLayout(newPosition) == NO) {
ASDisplayNodeAssertNonFatal(NO, @"-[ASDisplayNode setFrame:] - The new frame (%@) is invalid and unsafe to be set.", NSStringFromCGRect(rect));
return;
}

BOOL nodeLoaded = _loaded(self);
BOOL isMainThread = ASDisplayNodeThreadIsMain();
if (!specialPropertiesHandling) {
BOOL canReadProperties = isMainThread || !nodeLoaded;
if (canReadProperties) {
// We don't have to set frame directly, and we can read current properties.
// Compute a new bounds and position and set them on self.
CALayer *layer = _layer;
BOOL useLayer = (layer != nil);
CGPoint origin = (useLayer ? layer.bounds.origin : self.bounds.origin);
CGPoint anchorPoint = (useLayer ? layer.anchorPoint : self.anchorPoint);

CGRect newBounds = CGRectZero;
CGPoint newPosition = CGPointZero;
ASBoundsAndPositionForFrame(rect, origin, anchorPoint, &newBounds, &newPosition);

if (ASIsCGRectValidForLayout(newBounds) == NO || ASIsCGPositionValidForLayout(newPosition) == NO) {
ASDisplayNodeAssertNonFatal(NO, @"-[ASDisplayNode setFrame:] - The new frame (%@) is invalid and unsafe to be set.", NSStringFromCGRect(rect));
return;
}

if (useLayer) {
layer.bounds = newBounds;
layer.position = newPosition;
if (nodeLoaded) {
setToLayer = YES;
} else {
self.bounds = newBounds;
self.position = newPosition;
}
} else {
self.bounds = newBounds;
self.position = newPosition;
}
} else {
// We don't have to set frame directly, but we can't read properties.
// Store the frame in our pending state, and it'll get decomposed into
// bounds and position when the pending state is applied.
_ASPendingState *pendingState = ASDisplayNodeGetPendingState(self);
if (nodeLoaded && !pendingState.hasChanges) {
[[ASPendingStateController sharedInstance] registerNode:self];
// We don't have to set frame directly, but we can't read properties.
// Store the frame in our pending state, and it'll get decomposed into
// bounds and position when the pending state is applied.
_ASPendingState *pendingState = ASDisplayNodeGetPendingState(self);
if (nodeLoaded && !pendingState.hasChanges) {
[[ASPendingStateController sharedInstance] registerNode:self];
}
pendingState.frame = rect;
}
pendingState.frame = rect;
}
} else {
if (nodeLoaded && isMainThread) {
// We do have to set frame directly, and we're on main thread with a loaded node.
// Just set the frame on the view.
// NOTE: Frame is only defined when transform is identity because we explicitly diverge from CALayer behavior and define frame without transform.
//#if DEBUG
// // Checking if the transform is identity is expensive, so disable when unnecessary. We have assertions on in Release, so DEBUG is the only way I know of.
// ASDisplayNodeAssert(CATransform3DIsIdentity(self.transform), @"-[ASDisplayNode setFrame:] - self.transform must be identity in order to set the frame property. (From Apple's UIView documentation: If the transform property is not the identity transform, the value of this property is undefined and therefore should be ignored.)");
//#endif
_view.frame = rect;
} else {
// We do have to set frame directly, but either the node isn't loaded or we're on a non-main thread.
// Set the frame on the pending state, and it'll call setFrame: when applied.
_ASPendingState *pendingState = ASDisplayNodeGetPendingState(self);
if (nodeLoaded && !pendingState.hasChanges) {
[[ASPendingStateController sharedInstance] registerNode:self];
if (nodeLoaded && isMainThread) {
// We do have to set frame directly, and we're on main thread with a loaded node.
// Just set the frame on the view.
// NOTE: Frame is only defined when transform is identity because we explicitly diverge from CALayer behavior and define frame without transform.
setToView = YES;
} else {
// We do have to set frame directly, but either the node isn't loaded or we're on a non-main thread.
// Set the frame on the pending state, and it'll call setFrame: when applied.
_ASPendingState *pendingState = ASDisplayNodeGetPendingState(self);
if (nodeLoaded && !pendingState.hasChanges) {
[[ASPendingStateController sharedInstance] registerNode:self];
}
pendingState.frame = rect;
}
pendingState.frame = rect;
}
}

if (setToView) {
ASDisplayNodeAssertTrue(nodeLoaded && isMainThread);
_view.frame = rect;
} else if (setToLayer) {
ASDisplayNodeAssertTrue(nodeLoaded && isMainThread);
_layer.bounds = newBounds;
_layer.position = newPosition;
}
}

- (void)setNeedsDisplay
Expand Down

0 comments on commit 146368e

Please sign in to comment.