From 197950f39b53796adb51d0680f978485d651ed6d Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Mon, 23 May 2016 16:11:43 -0700 Subject: [PATCH 1/4] Add basic infrastructure for layout validation --- AsyncDisplayKit.xcodeproj/project.pbxproj | 12 ++ AsyncDisplayKit/ASDisplayNode.mm | 22 ++- AsyncDisplayKit/Layout/ASLayoutValidation.h | 73 +++++++ AsyncDisplayKit/Layout/ASLayoutValidation.mm | 197 +++++++++++++++++++ 4 files changed, 299 insertions(+), 5 deletions(-) create mode 100644 AsyncDisplayKit/Layout/ASLayoutValidation.h create mode 100644 AsyncDisplayKit/Layout/ASLayoutValidation.mm diff --git a/AsyncDisplayKit.xcodeproj/project.pbxproj b/AsyncDisplayKit.xcodeproj/project.pbxproj index 73889b0026..8ed10ccad0 100644 --- a/AsyncDisplayKit.xcodeproj/project.pbxproj +++ b/AsyncDisplayKit.xcodeproj/project.pbxproj @@ -285,6 +285,10 @@ 68FC85EB1CE29C7D00EDD713 /* ASVisibilityProtocols.m in Sources */ = {isa = PBXBuildFile; fileRef = 68FC85E81CE29C7D00EDD713 /* ASVisibilityProtocols.m */; }; 68FC85EC1CE29C7D00EDD713 /* ASVisibilityProtocols.m in Sources */ = {isa = PBXBuildFile; fileRef = 68FC85E81CE29C7D00EDD713 /* ASVisibilityProtocols.m */; }; 697B315A1CFE4B410049936F /* ASEditableTextNodeTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 697B31591CFE4B410049936F /* ASEditableTextNodeTests.m */; }; + 697C0DE31CF38F28001DE0D4 /* ASLayoutValidation.h in Headers */ = {isa = PBXBuildFile; fileRef = 697C0DE11CF38F28001DE0D4 /* ASLayoutValidation.h */; }; + 697C0DE41CF38F28001DE0D4 /* ASLayoutValidation.h in Headers */ = {isa = PBXBuildFile; fileRef = 697C0DE11CF38F28001DE0D4 /* ASLayoutValidation.h */; }; + 697C0DE51CF38F28001DE0D4 /* ASLayoutValidation.mm in Sources */ = {isa = PBXBuildFile; fileRef = 697C0DE21CF38F28001DE0D4 /* ASLayoutValidation.mm */; }; + 697C0DE61CF38F28001DE0D4 /* ASLayoutValidation.mm in Sources */ = {isa = PBXBuildFile; fileRef = 697C0DE21CF38F28001DE0D4 /* ASLayoutValidation.mm */; }; 698548631CA9E025008A345F /* ASEnvironment.h in Headers */ = {isa = PBXBuildFile; fileRef = 698548611CA9E025008A345F /* ASEnvironment.h */; settings = {ATTRIBUTES = (Public, ); }; }; 698548641CA9E025008A345F /* ASEnvironment.h in Headers */ = {isa = PBXBuildFile; fileRef = 698548611CA9E025008A345F /* ASEnvironment.h */; settings = {ATTRIBUTES = (Public, ); }; }; 698C8B611CAB49FC0052DC3F /* ASLayoutableExtensibility.h in Headers */ = {isa = PBXBuildFile; fileRef = 698C8B601CAB49FC0052DC3F /* ASLayoutableExtensibility.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -815,6 +819,8 @@ 68FC85E71CE29C7D00EDD713 /* ASVisibilityProtocols.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASVisibilityProtocols.h; sourceTree = ""; }; 68FC85E81CE29C7D00EDD713 /* ASVisibilityProtocols.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASVisibilityProtocols.m; sourceTree = ""; }; 697B31591CFE4B410049936F /* ASEditableTextNodeTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASEditableTextNodeTests.m; sourceTree = ""; }; + 697C0DE11CF38F28001DE0D4 /* ASLayoutValidation.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ASLayoutValidation.h; path = AsyncDisplayKit/Layout/ASLayoutValidation.h; sourceTree = ""; }; + 697C0DE21CF38F28001DE0D4 /* ASLayoutValidation.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = ASLayoutValidation.mm; path = AsyncDisplayKit/Layout/ASLayoutValidation.mm; sourceTree = ""; }; 698548611CA9E025008A345F /* ASEnvironment.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASEnvironment.h; sourceTree = ""; }; 698C8B601CAB49FC0052DC3F /* ASLayoutableExtensibility.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ASLayoutableExtensibility.h; path = AsyncDisplayKit/Layout/ASLayoutableExtensibility.h; sourceTree = ""; }; 69CB62A91CB8165900024920 /* _ASDisplayViewAccessiblity.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = _ASDisplayViewAccessiblity.h; sourceTree = ""; }; @@ -1454,6 +1460,8 @@ 9CDC18CB1B910E12004965E2 /* ASLayoutablePrivate.h */, ACF6ED0D1B17843500DA7C62 /* ASLayoutSpec.h */, ACF6ED0E1B17843500DA7C62 /* ASLayoutSpec.mm */, + 697C0DE11CF38F28001DE0D4 /* ASLayoutValidation.h */, + 697C0DE21CF38F28001DE0D4 /* ASLayoutValidation.mm */, ACF6ED121B17843500DA7C62 /* ASOverlayLayoutSpec.h */, ACF6ED131B17843500DA7C62 /* ASOverlayLayoutSpec.mm */, ACF6ED141B17843500DA7C62 /* ASRatioLayoutSpec.h */, @@ -1572,6 +1580,7 @@ AC7A2C171BDE11DF0093FE1A /* ASTableViewInternal.h in Headers */, 058D0A4D195D05CB00B7D73C /* ASDisplayNodeExtras.h in Headers */, 92074A671CC8BADA00918F75 /* ASControlNode+tvOS.h in Headers */, + 697C0DE31CF38F28001DE0D4 /* ASLayoutValidation.h in Headers */, 68355B3B1CB57A5A001D4E68 /* ASImageContainerProtocolCategories.h in Headers */, 68B0277A1C1A79CC0041016B /* ASDisplayNode+Beta.h in Headers */, 767E7F8D1C9019130066C000 /* AsyncDisplayKit+Debug.h in Headers */, @@ -1758,6 +1767,7 @@ 9CDC18CD1B910E12004965E2 /* ASLayoutablePrivate.h in Headers */, 68B8A4E21CBDB958007E4543 /* ASWeakProxy.h in Headers */, B35062201B010EFD0018CF92 /* ASLayoutController.h in Headers */, + 697C0DE41CF38F28001DE0D4 /* ASLayoutValidation.h in Headers */, B35062211B010EFD0018CF92 /* ASLayoutRangeType.h in Headers */, 34EFC76A1B701CE600AD841F /* ASLayoutSpec.h in Headers */, 34EFC7791B701D3600AD841F /* ASLayoutSpecUtilities.h in Headers */, @@ -2121,6 +2131,7 @@ ACF6ED301B17843500DA7C62 /* ASStackLayoutSpec.mm in Sources */, 257754BE1BEE458E00737CA5 /* ASTextKitComponents.m in Sources */, 257754A91BEE44CD00737CA5 /* ASTextKitContext.mm in Sources */, + 697C0DE51CF38F28001DE0D4 /* ASLayoutValidation.mm in Sources */, ACF6ED501B17847A00DA7C62 /* ASStackPositionedLayout.mm in Sources */, ACF6ED521B17847A00DA7C62 /* ASStackUnpositionedLayout.mm in Sources */, 257754A61BEE44CD00737CA5 /* ASTextKitAttributes.mm in Sources */, @@ -2285,6 +2296,7 @@ 34EFC7721B701D0300AD841F /* ASStackLayoutSpec.mm in Sources */, 34EFC7761B701D2A00AD841F /* ASStackPositionedLayout.mm in Sources */, 7AB338661C55B3420055FDE8 /* ASRelativeLayoutSpec.mm in Sources */, + 697C0DE61CF38F28001DE0D4 /* ASLayoutValidation.mm in Sources */, 9C70F2051CDA4F06007D6C76 /* ASTraitCollection.m in Sources */, 34EFC7781B701D3100AD841F /* ASStackUnpositionedLayout.mm in Sources */, DE84918E1C8FFF9F003D89E9 /* ASRunLoopQueue.mm in Sources */, diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index da1c3465e5..a5bea4a6c3 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -26,6 +26,7 @@ #import "ASEqualityHelpers.h" #import "ASRunLoopQueue.h" #import "ASEnvironmentInternal.h" +#import "ASLayoutValidation.h" #import "ASInternalHelpers.h" #import "ASLayout.h" @@ -854,6 +855,16 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) _pendingLayoutTransition = nil; } +#pragma mark - Layout Validation + +- (void)validateLayout:(ASLayout *)layout +{ + ASLayoutableValidation *validation = [[ASLayoutableValidation alloc] init]; + [validation registerValidator:[[ASLayoutableStaticValidator alloc] init]]; + [validation registerValidator:[[ASLayoutableStackValidator alloc] init]]; + [validation validateLayout:layout]; +} + #pragma mark - _ASTransitionContextCompletionDelegate @@ -1905,12 +1916,13 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock) layoutSpec.isMutable = NO; ASLayout *layout = [layoutSpec measureWithSizeRange:constrainedSize]; // Make sure layoutableObject of the root layout is `self`, so that the flattened layout will be structurally correct. - if (layout.layoutableObject != self) { + BOOL isRootLayout = (layout.layoutableObject != self); + if (isRootLayout) { layout.position = CGPointZero; - layout = [ASLayout layoutWithLayoutableObject:self - constrainedSizeRange:constrainedSize - size:layout.size - sublayouts:@[layout]]; + layout = [ASLayout layoutWithLayoutableObject:self constrainedSizeRange:constrainedSize size:layout.size sublayouts:@[layout]]; +#if DEBUG + [self validateLayout:layout]; +#endif } return [layout flattenedLayoutUsingPredicateBlock:^BOOL(ASLayout *evaluatedLayout) { if (self.usesImplicitHierarchyManagement) { diff --git a/AsyncDisplayKit/Layout/ASLayoutValidation.h b/AsyncDisplayKit/Layout/ASLayoutValidation.h new file mode 100644 index 0000000000..a5c6fd73b0 --- /dev/null +++ b/AsyncDisplayKit/Layout/ASLayoutValidation.h @@ -0,0 +1,73 @@ +/* + * Copyright (c) 2014-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + */ + +#import + +NS_ASSUME_NONNULL_BEGIN + +@class ASLayout; + +#pragma mark - ASLayoutableValidator + +@protocol ASLayoutableValidator +- (void)validateLayout:(ASLayout *)layout; +@end + +typedef void (^ASLayoutableBlockValidatorBlock)(id layout); + +@interface ASLayoutableBlockValidator : NSObject +@property (nonatomic, copy) ASLayoutableBlockValidatorBlock block; +- (instancetype)initWithBlock:(ASLayoutableBlockValidatorBlock)block NS_DESIGNATED_INITIALIZER; +- (instancetype)init NS_UNAVAILABLE; +@end + +/* + * ASLayoutables that have sizeRange or layoutPosition set needs to be wrapped into a ASStaticLayoutSpec. This + * validator checks if sublayouts has sizeRange or layoutPosition set and is wrapped in a ASStaticLayoutSpec + */ +@interface ASLayoutableStaticValidator : NSObject + +@end + +/* + * ASLayoutables that have spacingBefore, spacingAfter, flexGrow, flexShrink, flexBasis, alignSelf, ascender or descender + * set needs to be wrapped into a ASStackLayout. This validator checks if sublayouts has set one of this properties and + * asserts if it's not wrapped in a ASStackLayout if so. + */ +@interface ASLayoutableStackValidator : NSObject + +@end + +@interface ASLayoutablePreferredSizeValidator : NSObject + +@end + + +#pragma mark - ASLayoutableValidation + +@interface ASLayoutableValidation : NSObject + +/// Currently registered validators +@property (copy, nonatomic, readonly) NSMutableArray> *validators; + +/// Start from given layout and validates each layout in the layout tree with registered validators +- (void)validateLayout:(ASLayout *)layout; + +/// Register a layout validator +- (void)registerValidator:(id)validator; + +/// Register a layout validator with a block. Method returns the registered ASLayoutableValidator object that can be used to store somewhere and unregister +- (id)registerValidatorWithBlock:(ASLayoutableBlockValidatorBlock)block; + +/// Unregister a validtor +- (void)unregisterValidator:(id)validator; +@end + +NS_ASSUME_NONNULL_END diff --git a/AsyncDisplayKit/Layout/ASLayoutValidation.mm b/AsyncDisplayKit/Layout/ASLayoutValidation.mm new file mode 100644 index 0000000000..e8bb7f0d8e --- /dev/null +++ b/AsyncDisplayKit/Layout/ASLayoutValidation.mm @@ -0,0 +1,197 @@ +/* + * Copyright (c) 2014-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + */ + +#import "ASLayoutValidation.h" +#import "ASLayout.h" +#import "ASDisplayNode.h" + +#import "ASStaticLayoutSpec.h" +#import "ASStackLayoutSpec.h" + +#import + +#pragma mark ASLayoutableBlockValidator + +@implementation ASLayoutableBlockValidator + +#pragma mark Lifecycle + +- (instancetype)initWithBlock:(ASLayoutableBlockValidatorBlock)block +{ + self = [super init]; + if (self) { + _block = [block copy]; + } + return self; +} + +#pragma mark + +- (void)validateLayout:(ASLayout *)layout +{ + if (self.block) { + self.block(layout); + } +} + +@end + +#pragma mark ASLayoutableStaticValidator + +@implementation ASLayoutableStaticValidator + +- (void)validateLayout:(ASLayout *)layout +{ + id layoutable = layout.layoutableObject; + for (ASLayout * sublayout in layout.sublayouts) { + id sublayoutLayoutable = sublayout.layoutableObject; + + // Check for default sizeRange and layoutPosition + ASRelativeSizeRange sizeRange = sublayoutLayoutable.sizeRange; + ASRelativeSizeRange zeroSizeRange = ASRelativeSizeRangeMake(ASRelativeSizeMakeWithCGSize(CGSizeZero), + ASRelativeSizeMakeWithCGSize(CGSizeZero)); + + // Currently setting the preferredFrameSize also updates the sizeRange. Create a size range based on the + // preferredFrameSize and check it if it's the same as the current sizeRange to be sure it was not changed manually + CGSize preferredFrameSize = CGSizeZero; + if ([sublayoutLayoutable respondsToSelector:@selector(preferredFrameSize)]) { + preferredFrameSize = [((ASDisplayNode *)sublayoutLayoutable) preferredFrameSize]; + } + ASRelativeSizeRange preferredFrameSizeRange = ASRelativeSizeRangeMakeWithExactCGSize(preferredFrameSize); + + if ((ASRelativeSizeRangeEqualToRelativeSizeRange(sizeRange, zeroSizeRange) || + ASRelativeSizeRangeEqualToRelativeSizeRange(sizeRange, preferredFrameSizeRange)) + && CGPointEqualToPoint(sublayoutLayoutable.layoutPosition, CGPointZero)) { + continue; + } + + // Sublayout layoutable needs to be wrapped in a ASStaticLayoutSpec + if ([layoutable isKindOfClass:[ASStaticLayoutSpec class]]) { + continue; + } + + ASDisplayNodeCAssert(NO, @"A property was set that requires the ASLayoutable to be wrapped in a ASStaticLayoutSpec"); + } +} + +@end + +#pragma mark ASLayoutableStackValidator + +@implementation ASLayoutableStackValidator + +#pragma mark + +- (void)validateLayout:(ASLayout *)layout +{ + id layoutable = layout.layoutableObject; + for (ASLayout *sublayout in layout.sublayouts) { + id sublayoutLayoutable = sublayout.layoutableObject; + + // Check if default values related to ASStackLayoutSpec have changed + if (sublayoutLayoutable.spacingBefore == 0 && + sublayoutLayoutable.spacingAfter == 0 && + !sublayoutLayoutable.flexGrow && + !sublayoutLayoutable.flexShrink && + ASRelativeDimensionEqualToRelativeDimension([sublayoutLayoutable flexBasis], ASRelativeDimensionUnconstrained) && + sublayoutLayoutable.alignSelf == ASStackLayoutAlignSelfAuto) + { + continue; + } + + // Sublayout layoutable needs to be wrapped in a ASStackLayoutSpec + if ([layoutable isKindOfClass:[ASStackLayoutSpec class]]) { + continue; + } + + ASDisplayNodeCAssert(NO, @"A property was set that requires the ASLayoutable to be wrapped in a ASStackLayoutSpec"); + } +} + +@end + +#pragma mark ASLayoutablePreferredSizeValidator + +@implementation ASLayoutablePreferredSizeValidator + +#pragma mark + +- (void)validateLayout:(ASLayout *)layout +{ + // TODO: Implement validation that certain node classes need to have a preferredSize set e.g. ASVideoNode +} + +@end + + +#pragma mark - ASLayoutableValidation + +@interface ASLayoutableValidation () +@property (copy, nonatomic) NSMutableArray> *validators; +@end + +@implementation ASLayoutableValidation + +#pragma mark Lifecycle + +- (instancetype)init +{ + self = [super init]; + if (self) { + _validators = [NSMutableArray array]; + } + return self; +} + +#pragma mark Validator Management + +- (void)registerValidator:(id)validator +{ + [self.validators addObject:validator]; +} + +- (id)registerValidatorWithBlock:(ASLayoutableBlockValidatorBlock)block +{ + ASLayoutableBlockValidator *blockValidator = [[ASLayoutableBlockValidator alloc] initWithBlock:block]; + [self.validators addObject:blockValidator]; + return blockValidator; +} + +- (void)unregisterValidator:(id)validator +{ + [self.validators removeObject:validator]; +} + +#pragma mark Validation Process + +- (void)validateLayout:(ASLayout *)layout +{ + // Queue used to keep track of sublayouts while traversing this layout in a BFS fashion. + std::queue queue; + queue.push(layout); + + while (!queue.empty()) { + layout = queue.front(); + queue.pop(); + + // Validate layout with all registered validators + for (id validator in self.validators) { + [validator validateLayout:layout]; + } + + // Push sublayouts to queue for validation + for (id sublayout in [layout sublayouts]) { + queue.push(sublayout); + } + + } +} + +@end From f9e13545bf38235437dbcb2c4ee87cd5e573d075 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Wed, 8 Jun 2016 16:39:03 -0700 Subject: [PATCH 2/4] Improve layout validation - Asserts if layout is invalid - Add better help messages if layout is invalid --- AsyncDisplayKit/Layout/ASLayoutValidation.mm | 75 ++++++++++++-------- 1 file changed, 46 insertions(+), 29 deletions(-) diff --git a/AsyncDisplayKit/Layout/ASLayoutValidation.mm b/AsyncDisplayKit/Layout/ASLayoutValidation.mm index e8bb7f0d8e..4d567d52d3 100644 --- a/AsyncDisplayKit/Layout/ASLayoutValidation.mm +++ b/AsyncDisplayKit/Layout/ASLayoutValidation.mm @@ -17,7 +17,13 @@ #import -#pragma mark ASLayoutableBlockValidator +#pragma mark - Helpers + +static NSString *ASLayoutValidationWrappingAssertMessage(SEL selector, id obj, Class cl) { + return [NSString stringWithFormat:@"%@ was set on %@. It is either unecessary or the node needs to be wrapped in a %@", NSStringFromSelector(selector), obj, NSStringFromClass(cl)]; +} + +#pragma mark - ASLayoutableBlockValidator @implementation ASLayoutableBlockValidator @@ -43,20 +49,22 @@ @end -#pragma mark ASLayoutableStaticValidator +#pragma mark - ASLayoutableStaticValidator @implementation ASLayoutableStaticValidator - (void)validateLayout:(ASLayout *)layout { - id layoutable = layout.layoutableObject; - for (ASLayout * sublayout in layout.sublayouts) { + for (ASLayout *sublayout in layout.sublayouts) { + id layoutable = layout.layoutableObject; id sublayoutLayoutable = sublayout.layoutableObject; + NSString *assertMessage = nil; + Class stackContainerClass = [ASStaticLayoutSpec class]; + // Check for default sizeRange and layoutPosition ASRelativeSizeRange sizeRange = sublayoutLayoutable.sizeRange; - ASRelativeSizeRange zeroSizeRange = ASRelativeSizeRangeMake(ASRelativeSizeMakeWithCGSize(CGSizeZero), - ASRelativeSizeMakeWithCGSize(CGSizeZero)); + ASRelativeSizeRange zeroSizeRange = ASRelativeSizeRangeMakeWithExactCGSize(CGSizeZero); // Currently setting the preferredFrameSize also updates the sizeRange. Create a size range based on the // preferredFrameSize and check it if it's the same as the current sizeRange to be sure it was not changed manually @@ -66,24 +74,26 @@ } ASRelativeSizeRange preferredFrameSizeRange = ASRelativeSizeRangeMakeWithExactCGSize(preferredFrameSize); - if ((ASRelativeSizeRangeEqualToRelativeSizeRange(sizeRange, zeroSizeRange) || - ASRelativeSizeRangeEqualToRelativeSizeRange(sizeRange, preferredFrameSizeRange)) - && CGPointEqualToPoint(sublayoutLayoutable.layoutPosition, CGPointZero)) { + if (ASRelativeSizeRangeEqualToRelativeSizeRange(sizeRange, zeroSizeRange) == NO && + ASRelativeSizeRangeEqualToRelativeSizeRange(sizeRange, preferredFrameSizeRange) == NO) { + assertMessage = ASLayoutValidationWrappingAssertMessage(@selector(sizeRange), sublayoutLayoutable, stackContainerClass); + } else if (!CGPointEqualToPoint(sublayoutLayoutable.layoutPosition, CGPointZero)) { + assertMessage = ASLayoutValidationWrappingAssertMessage(@selector(layoutPosition), sublayoutLayoutable, stackContainerClass); + } + + // Sublayout layoutable should be wrapped in a ASStaticLayoutSpec + if (assertMessage == nil || [layoutable isKindOfClass:stackContainerClass]) { continue; } - // Sublayout layoutable needs to be wrapped in a ASStaticLayoutSpec - if ([layoutable isKindOfClass:[ASStaticLayoutSpec class]]) { - continue; - } - - ASDisplayNodeCAssert(NO, @"A property was set that requires the ASLayoutable to be wrapped in a ASStaticLayoutSpec"); + ASDisplayNodeCAssert(NO, assertMessage); } } @end -#pragma mark ASLayoutableStackValidator + +#pragma mark - ASLayoutableStackValidator @implementation ASLayoutableStackValidator @@ -95,23 +105,30 @@ for (ASLayout *sublayout in layout.sublayouts) { id sublayoutLayoutable = sublayout.layoutableObject; + NSString *assertMessage = nil; + Class stackContainerClass = [ASStackLayoutSpec class]; + // Check if default values related to ASStackLayoutSpec have changed - if (sublayoutLayoutable.spacingBefore == 0 && - sublayoutLayoutable.spacingAfter == 0 && - !sublayoutLayoutable.flexGrow && - !sublayoutLayoutable.flexShrink && - ASRelativeDimensionEqualToRelativeDimension([sublayoutLayoutable flexBasis], ASRelativeDimensionUnconstrained) && - sublayoutLayoutable.alignSelf == ASStackLayoutAlignSelfAuto) - { + if (sublayoutLayoutable.spacingBefore != 0) { + assertMessage = ASLayoutValidationWrappingAssertMessage(@selector(spacingBefore), sublayoutLayoutable, stackContainerClass); + } else if (sublayoutLayoutable.spacingAfter != 0) { + assertMessage = ASLayoutValidationWrappingAssertMessage(@selector(spacingAfter), sublayoutLayoutable, stackContainerClass); + } else if (sublayoutLayoutable.flexGrow == YES) { + assertMessage = ASLayoutValidationWrappingAssertMessage(@selector(flexGrow), sublayoutLayoutable, stackContainerClass); + } else if (sublayoutLayoutable.flexShrink == YES) { + assertMessage = ASLayoutValidationWrappingAssertMessage(@selector(flexShrink), sublayoutLayoutable, stackContainerClass); + } else if (!ASRelativeDimensionEqualToRelativeDimension(sublayoutLayoutable.flexBasis, ASRelativeDimensionUnconstrained) ) { + assertMessage = ASLayoutValidationWrappingAssertMessage(@selector(flexBasis), sublayoutLayoutable, stackContainerClass); + } else if (sublayoutLayoutable.alignSelf != ASStackLayoutAlignSelfAuto) { + assertMessage = ASLayoutValidationWrappingAssertMessage(@selector(alignSelf), sublayoutLayoutable, stackContainerClass); + } + + // Sublayout layoutable should be wrapped in a ASStackLayoutSpec + if (assertMessage == nil || [layoutable isKindOfClass:stackContainerClass]) { continue; } - // Sublayout layoutable needs to be wrapped in a ASStackLayoutSpec - if ([layoutable isKindOfClass:[ASStackLayoutSpec class]]) { - continue; - } - - ASDisplayNodeCAssert(NO, @"A property was set that requires the ASLayoutable to be wrapped in a ASStackLayoutSpec"); + ASDisplayNodeCAssert(NO, assertMessage); } } From 9ff7223b77537a5f2a4a66d3e62f0849d9cffe78 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Wed, 8 Jun 2016 16:59:46 -0700 Subject: [PATCH 3/4] Move validation code to ASLayoutValidation and add validation flag --- AsyncDisplayKit/ASDisplayNode.mm | 17 +++-------------- AsyncDisplayKit/Layout/ASLayoutValidation.h | 9 +++++++++ AsyncDisplayKit/Layout/ASLayoutValidation.mm | 9 +++++++++ 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index a5bea4a6c3..cb046190fe 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -26,11 +26,11 @@ #import "ASEqualityHelpers.h" #import "ASRunLoopQueue.h" #import "ASEnvironmentInternal.h" -#import "ASLayoutValidation.h" #import "ASInternalHelpers.h" #import "ASLayout.h" #import "ASLayoutSpec.h" +#import "ASLayoutValidation.h" #import "ASCellNode.h" NSInteger const ASDefaultDrawingPriority = ASDefaultTransactionPriority; @@ -855,17 +855,6 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) _pendingLayoutTransition = nil; } -#pragma mark - Layout Validation - -- (void)validateLayout:(ASLayout *)layout -{ - ASLayoutableValidation *validation = [[ASLayoutableValidation alloc] init]; - [validation registerValidator:[[ASLayoutableStaticValidator alloc] init]]; - [validation registerValidator:[[ASLayoutableStackValidator alloc] init]]; - [validation validateLayout:layout]; -} - - #pragma mark - _ASTransitionContextCompletionDelegate - (void)transitionContext:(_ASTransitionContext *)context didComplete:(BOOL)didComplete @@ -1920,8 +1909,8 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock) if (isRootLayout) { layout.position = CGPointZero; layout = [ASLayout layoutWithLayoutableObject:self constrainedSizeRange:constrainedSize size:layout.size sublayouts:@[layout]]; -#if DEBUG - [self validateLayout:layout]; +#if LAYOUT_VALIDATION + ASLayoutableValidateLayout(layout); #endif } return [layout flattenedLayoutUsingPredicateBlock:^BOOL(ASLayout *evaluatedLayout) { diff --git a/AsyncDisplayKit/Layout/ASLayoutValidation.h b/AsyncDisplayKit/Layout/ASLayoutValidation.h index a5c6fd73b0..7efde56c4f 100644 --- a/AsyncDisplayKit/Layout/ASLayoutValidation.h +++ b/AsyncDisplayKit/Layout/ASLayoutValidation.h @@ -9,11 +9,17 @@ */ #import +#import NS_ASSUME_NONNULL_BEGIN @class ASLayout; +// Enable or disable automatic layout validation +#define LAYOUT_VALIDATION 0 + +extern void ASLayoutableValidateLayout(ASLayout *layout); + #pragma mark - ASLayoutableValidator @protocol ASLayoutableValidator @@ -45,6 +51,9 @@ typedef void (^ASLayoutableBlockValidatorBlock)(id layout); @end +/* + * Not in use at the moment + */ @interface ASLayoutablePreferredSizeValidator : NSObject @end diff --git a/AsyncDisplayKit/Layout/ASLayoutValidation.mm b/AsyncDisplayKit/Layout/ASLayoutValidation.mm index 4d567d52d3..8010ae3698 100644 --- a/AsyncDisplayKit/Layout/ASLayoutValidation.mm +++ b/AsyncDisplayKit/Layout/ASLayoutValidation.mm @@ -17,6 +17,15 @@ #import +#pragma mark - Layout Validation + +void ASLayoutableValidateLayout(ASLayout *layout) { + ASLayoutableValidation *validation = [[ASLayoutableValidation alloc] init]; + [validation registerValidator:[[ASLayoutableStaticValidator alloc] init]]; + [validation registerValidator:[[ASLayoutableStackValidator alloc] init]]; + [validation validateLayout:layout]; +} + #pragma mark - Helpers static NSString *ASLayoutValidationWrappingAssertMessage(SEL selector, id obj, Class cl) { From 434102f98822607bacef56954f4382846687834c Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Wed, 8 Jun 2016 18:42:32 -0700 Subject: [PATCH 4/4] Address comments from review - Remove public NSMutableArray header for validators - Add ASDISPLAYNODE_EXTERN_C_BEGIN/END - Rename isRootLayout to isFinalLayoutable --- AsyncDisplayKit/ASDisplayNode.mm | 4 ++-- AsyncDisplayKit/Layout/ASLayoutValidation.h | 6 +++++- AsyncDisplayKit/Layout/ASLayoutValidation.mm | 16 +++++++++++----- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index cb046190fe..9236215c81 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -1905,8 +1905,8 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock) layoutSpec.isMutable = NO; ASLayout *layout = [layoutSpec measureWithSizeRange:constrainedSize]; // Make sure layoutableObject of the root layout is `self`, so that the flattened layout will be structurally correct. - BOOL isRootLayout = (layout.layoutableObject != self); - if (isRootLayout) { + BOOL isFinalLayoutable = (layout.layoutableObject != self); + if (isFinalLayoutable) { layout.position = CGPointZero; layout = [ASLayout layoutWithLayoutableObject:self constrainedSizeRange:constrainedSize size:layout.size sublayouts:@[layout]]; #if LAYOUT_VALIDATION diff --git a/AsyncDisplayKit/Layout/ASLayoutValidation.h b/AsyncDisplayKit/Layout/ASLayoutValidation.h index 7efde56c4f..b97985023f 100644 --- a/AsyncDisplayKit/Layout/ASLayoutValidation.h +++ b/AsyncDisplayKit/Layout/ASLayoutValidation.h @@ -18,8 +18,12 @@ NS_ASSUME_NONNULL_BEGIN // Enable or disable automatic layout validation #define LAYOUT_VALIDATION 0 +ASDISPLAYNODE_EXTERN_C_BEGIN + extern void ASLayoutableValidateLayout(ASLayout *layout); +ASDISPLAYNODE_EXTERN_C_END + #pragma mark - ASLayoutableValidator @protocol ASLayoutableValidator @@ -64,7 +68,7 @@ typedef void (^ASLayoutableBlockValidatorBlock)(id layout); @interface ASLayoutableValidation : NSObject /// Currently registered validators -@property (copy, nonatomic, readonly) NSMutableArray> *validators; +@property (copy, nonatomic, readonly) NSArray> *validators; /// Start from given layout and validates each layout in the layout tree with registered validators - (void)validateLayout:(ASLayout *)layout; diff --git a/AsyncDisplayKit/Layout/ASLayoutValidation.mm b/AsyncDisplayKit/Layout/ASLayoutValidation.mm index 8010ae3698..0cb574358b 100644 --- a/AsyncDisplayKit/Layout/ASLayoutValidation.mm +++ b/AsyncDisplayKit/Layout/ASLayoutValidation.mm @@ -160,10 +160,11 @@ static NSString *ASLayoutValidationWrappingAssertMessage(SEL selector, id obj, C #pragma mark - ASLayoutableValidation @interface ASLayoutableValidation () -@property (copy, nonatomic) NSMutableArray> *validators; @end -@implementation ASLayoutableValidation +@implementation ASLayoutableValidation { + NSMutableArray *_validators; +} #pragma mark Lifecycle @@ -178,21 +179,26 @@ static NSString *ASLayoutValidationWrappingAssertMessage(SEL selector, id obj, C #pragma mark Validator Management +- (NSArray> *)validators +{ + return [_validators copy]; +} + - (void)registerValidator:(id)validator { - [self.validators addObject:validator]; + [_validators addObject:validator]; } - (id)registerValidatorWithBlock:(ASLayoutableBlockValidatorBlock)block { ASLayoutableBlockValidator *blockValidator = [[ASLayoutableBlockValidator alloc] initWithBlock:block]; - [self.validators addObject:blockValidator]; + [_validators addObject:blockValidator]; return blockValidator; } - (void)unregisterValidator:(id)validator { - [self.validators removeObject:validator]; + [_validators removeObject:validator]; } #pragma mark Validation Process