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.

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

[ASDisplayNode] Provide safeAreaInsets and layoutMargins bridge #685

Merged
merged 13 commits into from
Mar 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## master
* Add your own contributions to the next release on the line below this with your name.
- [ASDisplayNode] Add safeAreaInsets, layoutMargins and related properties to ASDisplayNode, with full support for older OS versions [Yevgen Pogribnyi](https://github.com/ypogribnyi) [#685](https://github.com/TextureGroup/Texture/pull/685)
- [ASPINRemoteImageDownloader] Allow cache to provide animated image. [Max Wang](https://github.com/wsdwsd0829) [#850](https://github.com/TextureGroup/Texture/pull/850)
- [tvOS] Fixes errors when building against tvOS SDK [Alex Hill](https://github.com/alexhillc) [#728](https://github.com/TextureGroup/Texture/pull/728)
- [ASDisplayNode] Add unit tests for layout z-order changes (with an open issue to fix).
Expand Down
41 changes: 41 additions & 0 deletions Source/ASDisplayNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,20 @@ extern NSInteger const ASDefaultDrawingPriority;
*/
@property (nonatomic, readonly) BOOL supportsLayerBacking;

/**
* Whether or not the node layout should be automatically updated when it receives safeAreaInsetsDidChange.
*
* Defaults to NO.
*/
@property (nonatomic, assign) BOOL automaticallyRelayoutOnSafeAreaChanges;

/**
* Whether or not the node layout should be automatically updated when it receives layoutMarginsDidChange.
*
* Defaults to NO.
*/
@property (nonatomic, assign) BOOL automaticallyRelayoutOnLayoutMarginsChanges;

@end

/**
Expand Down Expand Up @@ -725,6 +739,33 @@ extern NSInteger const ASDefaultDrawingPriority;
@property (nonatomic, assign) BOOL autoresizesSubviews; // default==YES (undefined for layer-backed nodes)
@property (nonatomic, assign) UIViewAutoresizing autoresizingMask; // default==UIViewAutoresizingNone (undefined for layer-backed nodes)

/**
* @abstract Content margins
*
* @discussion This property is bridged to its UIView counterpart.
*
* If your layout depends on this property, you should probably enable automaticallyRelayoutOnLayoutMarginsChanges to ensure
* that the layout gets automatically updated when the value of this property changes. Or you can override layoutMarginsDidChange
* and make all the necessary updates manually.
*/
@property (nonatomic, assign) UIEdgeInsets layoutMargins;
@property (nonatomic, assign) BOOL preservesSuperviewLayoutMargins; // default is NO - set to enable pass-through or cascading behavior of margins from this view’s parent to its children
- (void)layoutMarginsDidChange;

/**
* @abstract Safe area insets
*
* @discussion This property is bridged to its UIVIew counterpart.
*
* If your layout depends on this property, you should probably enable automaticallyRelayoutOnSafeAreaChanges to ensure
* that the layout gets automatically updated when the value of this property changes. Or you can override safeAreaInsetsDidChange
* and make all the necessary updates manually.
*/
@property (nonatomic, readonly) UIEdgeInsets safeAreaInsets;
@property (nonatomic, assign) BOOL insetsLayoutMarginsFromSafeArea; // Default: YES
- (void)safeAreaInsetsDidChange;


// UIResponder methods
// By default these fall through to the underlying view, but can be overridden.
- (BOOL)canBecomeFirstResponder; // default==NO
Expand Down
109 changes: 108 additions & 1 deletion Source/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,14 @@ - (void)_initializeInstance

_flags.canClearContentsOfLayer = YES;
_flags.canCallSetNeedsDisplayOfLayer = YES;

_fallbackSafeAreaInsets = UIEdgeInsetsZero;
_fallbackInsetsLayoutMarginsFromSafeArea = YES;
_isViewControllerRoot = NO;

_automaticallyRelayoutOnSafeAreaChanges = NO;
_automaticallyRelayoutOnLayoutMarginsChanges = NO;

ASDisplayNodeLogEvent(self, @"init");
}

Expand Down Expand Up @@ -851,7 +859,104 @@ - (void)nodeViewDidAddGestureRecognizer
_flags.viewEverHadAGestureRecognizerAttached = YES;
}

#pragma mark UIResponder
- (UIEdgeInsets)fallbackSafeAreaInsets
{
ASDN::MutexLocker l(__instanceLock__);
return _fallbackSafeAreaInsets;
}

- (void)setFallbackSafeAreaInsets:(UIEdgeInsets)insets
{
BOOL needsManualUpdate;
BOOL updatesLayoutMargins;

{
ASDN::MutexLocker l(__instanceLock__);
ASDisplayNodeAssertThreadAffinity(self);

if (UIEdgeInsetsEqualToEdgeInsets(insets, _fallbackSafeAreaInsets)) {
return;
}

_fallbackSafeAreaInsets = insets;
needsManualUpdate = !AS_AT_LEAST_IOS11 || _flags.layerBacked;
updatesLayoutMargins = needsManualUpdate && [self _locked_insetsLayoutMarginsFromSafeArea];
}

if (needsManualUpdate) {
[self safeAreaInsetsDidChange];
}

if (updatesLayoutMargins) {
[self layoutMarginsDidChange];
}
}

- (void)_fallbackUpdateSafeAreaOnChildren
Copy link
Member

Choose a reason for hiding this comment

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

This method is called by __layout which means it runs on every frame and must be treated as a hot path. I haven't profiled to see how fast/slow it is, but I think it's worthy to try to add some straightforward optimizations right from the beginning.

One optimization I can think of is that, since fallback safe area insets are only used for backward compatibility, we can bail early/skip a child whenever possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a shortcut that skips a child if it is a view-backed node and we run on iOS 11. And -[ASDisplayNode setFallbackSafeAreaInsets:] returns early when the insets don't change.

Although we could try to skip calculations for the children that use fallback safe area insets but don't really need to have them updated (e.g. neither the child's frame, the parent's frame nor the parent's safe area wasn't updated), I doubt that it would save more CPU time than would spend running the checks.

The other thing I'm thinking about is that on iOS 11 we can disable fallback logic completely if we avoid supporting the safe area for the layer-backed nodes. But I'm not sure if we should do that.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the note. I think this implementation and your trade-offs are reasonable. Let's roll with this.

{
ASDisplayNodeAssertThreadAffinity(self);

UIEdgeInsets insets = self.safeAreaInsets;
CGRect bounds = self.bounds;

for (ASDisplayNode *child in self.subnodes) {
if (AS_AT_LEAST_IOS11 && !child.layerBacked) {
// In iOS 11 view-backed nodes already know what their safe area is.
continue;
}

if (child.viewControllerRoot) {
// Its safe area is controlled by a view controller. Don't override it.
continue;
}

CGRect childFrame = child.frame;
UIEdgeInsets childInsets = UIEdgeInsetsMake(MAX(insets.top - (CGRectGetMinY(childFrame) - CGRectGetMinY(bounds)), 0),
MAX(insets.left - (CGRectGetMinX(childFrame) - CGRectGetMinX(bounds)), 0),
MAX(insets.bottom - (CGRectGetMaxY(bounds) - CGRectGetMaxY(childFrame)), 0),
MAX(insets.right - (CGRectGetMaxX(bounds) - CGRectGetMaxX(childFrame)), 0));

child.fallbackSafeAreaInsets = childInsets;
}
}

- (BOOL)isViewControllerRoot
{
ASDN::MutexLocker l(__instanceLock__);
return _isViewControllerRoot;
}

- (void)setViewControllerRoot:(BOOL)flag
{
ASDN::MutexLocker l(__instanceLock__);
_isViewControllerRoot = flag;
}

- (BOOL)automaticallyRelayoutOnSafeAreaChanges
{
ASDN::MutexLocker l(__instanceLock__);
return _automaticallyRelayoutOnSafeAreaChanges;
}

- (void)setAutomaticallyRelayoutOnSafeAreaChanges:(BOOL)flag
{
ASDN::MutexLocker l(__instanceLock__);
_automaticallyRelayoutOnSafeAreaChanges = flag;
}

- (BOOL)automaticallyRelayoutOnLayoutMarginsChanges
{
ASDN::MutexLocker l(__instanceLock__);
return _automaticallyRelayoutOnLayoutMarginsChanges;
}

- (void)setAutomaticallyRelayoutOnLayoutMarginsChanges:(BOOL)flag
{
ASDN::MutexLocker l(__instanceLock__);
_automaticallyRelayoutOnLayoutMarginsChanges = flag;
}

#pragma mark - UIResponder

#define HANDLE_NODE_RESPONDER_METHOD(__sel) \
/* All responder methods should be called on the main thread */ \
Expand Down Expand Up @@ -1042,6 +1147,8 @@ - (void)__layout
[self layoutDidFinish];
});
}

[self _fallbackUpdateSafeAreaOnChildren];
}

- (void)layoutDidFinish
Expand Down
5 changes: 5 additions & 0 deletions Source/ASViewController.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ NS_ASSUME_NONNULL_BEGIN
// Refer to examples/SynchronousConcurrency, AsyncViewController.m
@property (nonatomic, assign) BOOL neverShowPlaceholders;

/* Custom container UIViewController subclasses can use this property to add to the overlay
that UIViewController calculates for the safeAreaInsets for contained view controllers.
*/
@property(nonatomic) UIEdgeInsets additionalSafeAreaInsets;

@end

@interface ASViewController (ASRangeControllerUpdateRangeProtocol)
Expand Down
38 changes: 38 additions & 0 deletions Source/ASViewController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ @implementation ASViewController
NSInteger _visibilityDepth;
BOOL _selfConformsToRangeModeProtocol;
BOOL _nodeConformsToRangeModeProtocol;
UIEdgeInsets _fallbackAdditionalSafeAreaInsets;
}

- (instancetype)initWithNibName:(NSString *)nibNameOrNil bundle:(NSBundle *)nibBundleOrNil
Expand Down Expand Up @@ -73,10 +74,14 @@ - (void)_initializeInstance
if (_node == nil) {
return;
}

_node.viewControllerRoot = YES;
Copy link
Member

Choose a reason for hiding this comment

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

This is fine for now, especially because it's thread-safe and faster than -[ASDisplayNode(Convenience) closestViewController].

One edge case I can think of is that a root-of-view-controller node is reused and added to a different node hierarchy. It is rare, but we should document is somehow, and maybe assert in debug mode (e.g if viewControllerRoot is true, assert that its close view controller indeed points to itself).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added an assertion to -[_ASDisplayView didMoveToSuperview] to check for that case. Would that be enough?

Copy link
Member

Choose a reason for hiding this comment

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

👍


_selfConformsToRangeModeProtocol = [self conformsToProtocol:@protocol(ASRangeControllerUpdateRangeProtocol)];
_nodeConformsToRangeModeProtocol = [_node conformsToProtocol:@protocol(ASRangeControllerUpdateRangeProtocol)];
_automaticallyAdjustRangeModeBasedOnViewEvents = _selfConformsToRangeModeProtocol || _nodeConformsToRangeModeProtocol;

_fallbackAdditionalSafeAreaInsets = UIEdgeInsetsZero;

// In case the node will get loaded
if (_node.nodeLoaded) {
Expand Down Expand Up @@ -159,6 +164,20 @@ - (void)viewDidLayoutSubviews
[_node recursivelyEnsureDisplaySynchronously:YES];
}
[super viewDidLayoutSubviews];

if (!AS_AT_LEAST_IOS11) {
[self _updateNodeFallbackSafeArea];
}
}

- (void)_updateNodeFallbackSafeArea
{
UIEdgeInsets safeArea = UIEdgeInsetsMake(self.topLayoutGuide.length, 0, self.bottomLayoutGuide.length, 0);
UIEdgeInsets additionalInsets = self.additionalSafeAreaInsets;

safeArea = ASConcatInsets(safeArea, additionalInsets);

_node.fallbackSafeAreaInsets = safeArea;
Copy link
Member

@nguyenhuy nguyenhuy Jan 16, 2018

Choose a reason for hiding this comment

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

  • This node is guaranteed to be view-backed and loaded. The only reason the fallback insets is needed here is that the runtime is iOS 10 or earlier. Let's take advantage of that and avoid doing unnecessary work if possible.

}

ASVisibilityDidMoveToParentViewController;
Expand Down Expand Up @@ -264,6 +283,25 @@ - (ASInterfaceState)interfaceState
return _node.interfaceState;
}

- (UIEdgeInsets)additionalSafeAreaInsets
{
if (AS_AVAILABLE_IOS(11.0)) {
return super.additionalSafeAreaInsets;
}

return _fallbackAdditionalSafeAreaInsets;
}

- (void)setAdditionalSafeAreaInsets:(UIEdgeInsets)additionalSafeAreaInsets
{
if (AS_AVAILABLE_IOS(11.0)) {
[super setAdditionalSafeAreaInsets:additionalSafeAreaInsets];
} else {
_fallbackAdditionalSafeAreaInsets = additionalSafeAreaInsets;
[self _updateNodeFallbackSafeArea];
}
}

#pragma mark - ASTraitEnvironment

- (ASPrimitiveTraitCollection)primitiveTraitCollectionForUITraitCollection:(UITraitCollection *)traitCollection
Expand Down
3 changes: 3 additions & 0 deletions Source/Details/UIView+ASConvenience.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ NS_ASSUME_NONNULL_BEGIN
@property (nonatomic, assign, getter=isUserInteractionEnabled) BOOL userInteractionEnabled;
@property (nonatomic, assign, getter=isExclusiveTouch) BOOL exclusiveTouch;
@property (nonatomic, assign, getter=asyncdisplaykit_isAsyncTransactionContainer, setter = asyncdisplaykit_setAsyncTransactionContainer:) BOOL asyncdisplaykit_asyncTransactionContainer;
@property (nonatomic, assign) UIEdgeInsets layoutMargins;
@property (nonatomic, assign) BOOL preservesSuperviewLayoutMargins;
@property (nonatomic, assign) BOOL insetsLayoutMarginsFromSafeArea;

/**
Following properties of the UIAccessibility informal protocol are supported as well.
Expand Down
30 changes: 29 additions & 1 deletion Source/Details/_ASDisplayView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@
#import <AsyncDisplayKit/_ASCoreAnimationExtras.h>
#import <AsyncDisplayKit/_ASDisplayLayer.h>
#import <AsyncDisplayKit/ASDisplayNodeInternal.h>
#import <AsyncDisplayKit/ASDisplayNode+Convenience.h>
#import <AsyncDisplayKit/ASDisplayNode+FrameworkPrivate.h>
#import <AsyncDisplayKit/ASDisplayNode+Subclasses.h>
#import <AsyncDisplayKit/ASInternalHelpers.h>
#import <AsyncDisplayKit/ASObjectDescriptionHelpers.h>
#import <AsyncDisplayKit/ASLayout.h>
#import <AsyncDisplayKit/ASObjectDescriptionHelpers.h>
#import <AsyncDisplayKit/ASViewController.h>

#pragma mark - _ASDisplayViewMethodOverrides

Expand Down Expand Up @@ -234,6 +236,16 @@ - (void)didMoveToSuperview
self.keepalive_node = nil;
}

#if DEBUG
// This is only to help detect issues when a root-of-view-controller node is reused separately from its view controller.
// Avoid overhead in release.
if (superview && node.viewControllerRoot) {
UIViewController *vc = [node closestViewController];

ASDisplayNodeAssert(vc != nil && [vc isKindOfClass:[ASViewController class]] && ((ASViewController*)vc).node == node, @"This node was once used as a view controller's node. You should not reuse it without its view controller.");
}
#endif

ASDisplayNode *supernode = node.supernode;
ASDisplayNodeAssert(!supernode.isLayerBacked, @"Shouldn't be possible for superview's node to be layer-backed.");

Expand Down Expand Up @@ -481,6 +493,22 @@ - (BOOL)canPerformAction:(SEL)action withSender:(id)sender
return ([super canPerformAction:action withSender:sender] || [node respondsToSelector:action]);
}

- (void)layoutMarginsDidChange
{
ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
[super layoutMarginsDidChange];

[node layoutMarginsDidChange];
}

- (void)safeAreaInsetsDidChange
{
ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
[super safeAreaInsetsDidChange];

[node safeAreaInsetsDidChange];
}

- (id)forwardingTargetForSelector:(SEL)aSelector
{
// Ideally, we would implement -targetForAction:withSender: and simply return the node where we don't respond personally.
Expand Down
19 changes: 19 additions & 0 deletions Source/Private/ASDisplayNode+FrameworkPrivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,25 @@ __unused static NSString * _Nonnull NSStringFromASHierarchyStateChange(ASHierarc
*/
- (BOOL)shouldScheduleDisplayWithNewInterfaceState:(ASInterfaceState)newInterfaceState;

/**
* @abstract safeAreaInsets will fallback to this value if the corresponding UIKit property is not available
* (due to an old iOS version).
*
* @discussion This should be set by the owning view controller based on it's layout guides.
* If this is not a view controllet's node the value will be calculated automatically by the parent node.
*/
@property (nonatomic, assign) UIEdgeInsets fallbackSafeAreaInsets;

/**
* @abstract Indicates if this node is a view controller's root node. Defaults to NO.
*
* @discussion Set to YES in -[ASViewController initWithNode:].
*
* YES here only means that this node is used as an ASViewController node. It doesn't mean that this node is a root of
* ASDisplayNode hierarchy, e.g. when its view controller is parented by another ASViewController.
*/
@property (nonatomic, assign, getter=isViewControllerRoot) BOOL viewControllerRoot;
Copy link
Member

Choose a reason for hiding this comment

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

Good call on making this property private :)


@end


Expand Down
Loading