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

[ASDisplayNode] Trigger a layout pass whenever a node enters preload state #3263

Merged
27 changes: 21 additions & 6 deletions Source/ASCollectionNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -193,18 +193,27 @@ - (void)clearContents
[self.rangeController clearContents];
}

- (void)didExitPreloadState
{
[super didExitPreloadState];
[self.rangeController clearPreloadedData];
}

- (void)interfaceStateDidChange:(ASInterfaceState)newState fromState:(ASInterfaceState)oldState
{
[super interfaceStateDidChange:newState fromState:oldState];
[ASRangeController layoutDebugOverlayIfNeeded];
}

- (void)didEnterPreloadState
{
// Intentionally allocate the collection view here so that super will trigger a layout pass on it which in turn will loads its intial data.
// We can get rid of this later when ASDataController, ASRangeController and the collection layout object can operate without the view.
[self view];
[super didEnterPreloadState];
}

- (void)didEnterDisplayState
{
[super didEnterDisplayState];
// Since an initial data load was triggered when this node enter preloads state, wait for it to finish
[self waitUntilAllUpdatesAreCommitted];
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the scariest part of the change, @Adlai-Holler can you double check that this makes sense to you as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually we don't need to do this so I removed it!

}

#if ASRangeControllerLoggingEnabled
- (void)didEnterVisibleState
{
Expand All @@ -219,6 +228,12 @@ - (void)didExitVisibleState
}
#endif

- (void)didExitPreloadState
{
[super didExitPreloadState];
[self.rangeController clearPreloadedData];
}

#pragma mark Setter / Getter

// TODO: Implement this without the view. Then revisit ASLayoutElementCollectionTableSetTraitCollection
Expand Down
5 changes: 5 additions & 0 deletions Source/ASDisplayNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,11 @@ extern NSInteger const ASDefaultDrawingPriority;
*/
- (void)setNeedsLayout;

/**
* Performs a layout pass on the node. Convenience for use whether the view / layer is loaded or not. Safe to call from a background thread.
*/
- (void)layoutIfNeeded;

@property (nonatomic, strong, nullable) id contents; // default=nil
@property (nonatomic, assign) BOOL clipsToBounds; // default==NO
@property (nonatomic, getter=isOpaque) BOOL opaque; // default==YES
Expand Down
19 changes: 13 additions & 6 deletions Source/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,7 @@ - (void)invalidateCalculatedLayout

- (void)__layout
{
ASDisplayNodeAssertMainThread();
ASDisplayNodeAssertThreadAffinity(self);
ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__);

{
Expand Down Expand Up @@ -1014,8 +1014,12 @@ - (void)__layout
[self _locked_layoutPlaceholderIfNecessary];
}

[self layout];
[self layoutDidFinish];
[self _layoutSublayouts];

ASPerformBlockOnMainThread(^{
[self layout];
[self layoutDidFinish];
});
}

/// Needs to be called with lock held
Expand Down Expand Up @@ -1054,7 +1058,7 @@ - (void)_locked_measureNodeWithBoundsIfNecessary:(CGRect)bounds
std::shared_ptr<ASDisplayNodeLayout> nextLayout = _pendingDisplayNodeLayout;
#define layoutSizeDifferentFromBounds !CGSizeEqualToSize(nextLayout->layout.size, boundsSizeForLayout)

// nextLayout was likely created by a call to layoutThatFits:, check if is valid and can be applied.
// nextLayout was likely created by a call to layoutThatFits:, check if it is valid and can be applied.
// If our bounds size is different than it, or invalid, recalculate. Use #define to avoid nullptr->
if (nextLayout == nullptr || nextLayout->isDirty() == YES || layoutSizeDifferentFromBounds) {
// Use the last known constrainedSize passed from a parent during layout (if never, use bounds).
Expand Down Expand Up @@ -1405,12 +1409,13 @@ - (void)displayNodeDidInvalidateSizeNewSize:(CGSize)size

- (void)layout
{
[self _layoutSublayouts];
ASDisplayNodeAssertMainThread();
// Subclass hook
}

- (void)_layoutSublayouts
{
ASDisplayNodeAssertMainThread();
ASDisplayNodeAssertThreadAffinity(self);
ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__);

ASLayout *layout;
Expand Down Expand Up @@ -3720,6 +3725,8 @@ - (void)didEnterPreloadState
ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__);
[_interfaceStateDelegate didEnterPreloadState];

[self layoutIfNeeded];

if (_methodOverrides & ASDisplayNodeMethodOverrideFetchData) {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
Expand Down
1 change: 1 addition & 0 deletions Source/Details/UIView+ASConvenience.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ NS_ASSUME_NONNULL_BEGIN

- (void)setNeedsDisplay;
- (void)setNeedsLayout;
- (void)layoutIfNeeded;

@end

Expand Down
1 change: 1 addition & 0 deletions Source/Details/_ASCollectionViewCell.m
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ - (void)prepareForReuse
*/
- (void)applyLayoutAttributes:(UICollectionViewLayoutAttributes *)layoutAttributes
{
[super applyLayoutAttributes:layoutAttributes];
self.layoutAttributes = layoutAttributes;
}

Expand Down
77 changes: 58 additions & 19 deletions Source/Private/ASDisplayNode+UIViewBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
#if DISPLAYNODE_USE_LOCKS
#define _bridge_prologue_read ASDN::MutexLocker l(__instanceLock__); ASDisplayNodeAssertThreadAffinity(self)
#define _bridge_prologue_write ASDN::MutexLocker l(__instanceLock__)
#define _bridge_prologue_write_unlock ASDN::MutexUnlocker u(__instanceLock__)
#else
#define _bridge_prologue_read ASDisplayNodeAssertThreadAffinity(self)
#define _bridge_prologue_write
Expand Down Expand Up @@ -79,8 +78,6 @@ ASDISPLAYNODE_INLINE BOOL ASDisplayNodeShouldApplyBridgedWriteToView(ASDisplayNo
#define _setToLayer(layerProperty, layerValueExpr) BOOL shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self); \
if (shouldApply) { _layer.layerProperty = (layerValueExpr); } else { ASDisplayNodeGetPendingState(self).layerProperty = (layerValueExpr); }

#define _messageToViewOrLayer(viewAndLayerSelector) (_view ? [_view viewAndLayerSelector] : [_layer viewAndLayerSelector])

/**
* This category implements certain frequently-used properties and methods of UIView and CALayer so that ASDisplayNode clients can just call the view/layer methods on the node,
* with minimal loss in performance. Unlike UIView and CALayer methods, these can be called from a non-main thread until the view or layer is created.
Expand Down Expand Up @@ -301,8 +298,17 @@ - (void)setFrame:(CGRect)rect

- (void)setNeedsDisplay
{
_bridge_prologue_write;
if (_hierarchyState & ASHierarchyStateRasterized) {
BOOL isRasterized = NO;
BOOL shouldApply = NO;
id viewOrLayer = nil;
{
_bridge_prologue_write;
isRasterized = _hierarchyState & ASHierarchyStateRasterized;
shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self);
viewOrLayer = _view ?: _layer;
}

if (isRasterized) {
ASPerformBlockOnMainThread(^{
// The below operation must be performed on the main thread to ensure against an extremely rare deadlock, where a parent node
// begins materializing the view / layer hierarchy (locking itself or a descendant) while this node walks up
Expand All @@ -319,13 +325,14 @@ - (void)setNeedsDisplay
[rasterizedContainerNode setNeedsDisplay];
});
} else {
BOOL shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self);
if (shouldApply) {
// If not rasterized, and the node is loaded (meaning we certainly have a view or layer), send a
// message to the view/layer first. This is because __setNeedsDisplay calls as scheduleNodeForDisplay,
// which may call -displayIfNeeded. We want to ensure the needsDisplay flag is set now, and then cleared.
_messageToViewOrLayer(setNeedsDisplay);
[viewOrLayer setNeedsDisplay];
} else {
// Need to grab the lock here because _ASPendingState itself is not locked at all
_bridge_prologue_write;
[ASDisplayNodeGetPendingState(self) setNeedsDisplay];
}
[self __setNeedsDisplay];
Expand All @@ -334,29 +341,61 @@ - (void)setNeedsDisplay

- (void)setNeedsLayout
{
_bridge_prologue_write;
BOOL shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self);
BOOL shouldApply = NO;
BOOL loaded = NO;
id viewOrLayer = nil;
{
_bridge_prologue_write;
shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self);
loaded = __loaded(self);
viewOrLayer = _view ?: _layer;
}

if (shouldApply) {
// The node is loaded and we're on main.
// Quite the opposite of setNeedsDisplay, we must call __setNeedsLayout before messaging
// the view or layer to ensure that measurement and implicitly added subnodes have been handled.

// Calling __setNeedsLayout while holding the property lock can cause deadlocks
_bridge_prologue_write_unlock;
[self __setNeedsLayout];
_bridge_prologue_write;
_messageToViewOrLayer(setNeedsLayout);
} else if (__loaded(self)) {
[viewOrLayer setNeedsLayout];
} else if (loaded) {
// The node is loaded but we're not on main.
// We will call [self __setNeedsLayout] when we apply
// the pending state. We need to call it on main if the node is loaded
// to support automatic subnode management.
// We will call [self __setNeedsLayout] when we apply the pending state.
// We need to call it on main if the node is loaded to support automatic subnode management.

_bridge_prologue_write;
[ASDisplayNodeGetPendingState(self) setNeedsLayout];
} else {
// The node is not loaded and we're not on main.
_bridge_prologue_write_unlock;
[self __setNeedsLayout];
}
}

- (void)layoutIfNeeded
{
BOOL shouldApply = NO;
BOOL loaded = NO;
id viewOrLayer = nil;
{
_bridge_prologue_write;
shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self);
loaded = __loaded(self);
viewOrLayer = _view ?: _layer;
}

if (shouldApply) {
// The node is loaded and we're on main.
// Message the view or layer which in turn will call __layout on us (see -[_ASDisplayLayer layoutSublayers]).
[viewOrLayer layoutIfNeeded];
} else if (loaded) {
// The node is loaded but we're not on main.
// We will call layoutIfNeeded on the view or layer when we apply the pending state. __layout will in turn be called on us (see -[_ASDisplayLayer layoutSublayers]).
// We need to call it on main if the node is loaded to support automatic subnode management.

_bridge_prologue_write;
[ASDisplayNodeGetPendingState(self) layoutIfNeeded];
} else {
// The node is not loaded and we're not on main.
[self __layout];
}
}

Expand Down
26 changes: 19 additions & 7 deletions Source/Private/_ASPendingState.mm
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
// Properties
int needsDisplay:1;
int needsLayout:1;

int layoutIfNeeded:1;

// Flags indicating that a given property should be applied to the view at creation
int setClipsToBounds:1;
int setOpaque:1;
Expand Down Expand Up @@ -272,6 +273,11 @@ - (void)setNeedsLayout
_flags.needsLayout = YES;
}

- (void)layoutIfNeeded
{
_flags.layoutIfNeeded = YES;
}

- (void)setClipsToBounds:(BOOL)flag
{
clipsToBounds = flag;
Expand Down Expand Up @@ -761,16 +767,19 @@ - (void)applyToLayer:(CALayer *)layer
if (flags.setEdgeAntialiasingMask)
layer.edgeAntialiasingMask = edgeAntialiasingMask;

if (flags.needsLayout)
[layer setNeedsLayout];

if (flags.setAsyncTransactionContainer)
layer.asyncdisplaykit_asyncTransactionContainer = asyncTransactionContainer;

if (flags.setOpaque)
ASDisplayNodeAssert(layer.opaque == opaque, @"Didn't set opaque as desired");

ASPendingStateApplyMetricsToLayer(self, layer);

if (flags.needsLayout)
[layer setNeedsLayout];

if (flags.layoutIfNeeded)
[layer layoutIfNeeded];
}

- (void)applyToView:(UIView *)view withSpecialPropertiesHandling:(BOOL)specialPropertiesHandling
Expand Down Expand Up @@ -889,9 +898,6 @@ - (void)applyToView:(UIView *)view withSpecialPropertiesHandling:(BOOL)specialPr
if (flags.setEdgeAntialiasingMask)
layer.edgeAntialiasingMask = edgeAntialiasingMask;

if (flags.needsLayout)
[view setNeedsLayout];

if (flags.setAsyncTransactionContainer)
view.asyncdisplaykit_asyncTransactionContainer = asyncTransactionContainer;

Expand Down Expand Up @@ -955,6 +961,12 @@ - (void)applyToView:(UIView *)view withSpecialPropertiesHandling:(BOOL)specialPr
} else {
ASPendingStateApplyMetricsToLayer(self, layer);
}

if (flags.needsLayout)
[view setNeedsLayout];

if (flags.layoutIfNeeded)
[view layoutIfNeeded];
}

// FIXME: Make this more efficient by tracking which properties are set rather than reading everything.
Expand Down