Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.
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
2 changes: 2 additions & 0 deletions AsyncDisplayKit/ASDisplayNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#import <AsyncDisplayKit/ASAsciiArtBoxCreator.h>
#import <AsyncDisplayKit/ASLayoutable.h>

#define ASDisplayNodeLoggingEnabled 0

@class ASDisplayNode;

/**
Expand Down
66 changes: 43 additions & 23 deletions AsyncDisplayKit/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,11 @@ - (void)_staticInitialize;

@end

//#define LOG(...) NSLog(__VA_ARGS__)
#define LOG(...)
#if ASDisplayNodeLoggingEnabled
#define LOG(...) NSLog(__VA_ARGS__)
#else
#define LOG(...)
#endif

// Conditionally time these scopes to our debug ivars (only exist in debug/profile builds)
#if TIME_DISPLAYNODE_OPS
Expand Down Expand Up @@ -880,7 +883,7 @@ - (BOOL)_shouldAbortTransitionWithID:(int32_t)transitionID

- (void)animateLayoutTransition:(id<ASContextTransitioning>)context
{
[self __layoutSublayouts];
[self __layoutSubnodes];
[context completeTransition:YES];
}

Expand Down Expand Up @@ -1114,26 +1117,61 @@ - (void)__layout
ASDisplayNodeAssertMainThread();
ASDN::MutexLocker l(_propertyLock);
CGRect bounds = self.bounds;

[self measureNodeWithBoundsIfNecessary:bounds];

if (CGRectEqualToRect(bounds, CGRectZero)) {
// Performing layout on a zero-bounds view often results in frame calculations
// with negative sizes after applying margins, which will cause
// measureWithSizeRange: on subnodes to assert.
return;
}

// Handle placeholder layer creation in case the size of the node changed after the initial placeholder layer
// was created
if ([self _shouldHavePlaceholderLayer]) {
[self _setupPlaceholderLayerIfNeeded];
}
_placeholderLayer.frame = bounds;

[self layout];
[self layoutDidFinish];
}

- (void)measureNodeWithBoundsIfNecessary:(CGRect)bounds
{
// Normally measure will be called before layout occurs. If this doesn't happen, nothing is going to call it at all.
// We simply call measureWithSizeRange: using a size range equal to whatever bounds were provided to that element
if (self.supernode == nil && !self.supportsRangeManagedInterfaceState && [self _hasDirtyLayout]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@maicki what about ASViewController? I think there is a way to override the constrainedSize, but the .node has no supernode, is not range managed, etc. This may be handled by _hasDirtyLayout, not sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

We probably want to grab the lock here, at least while checking these three properties. If we don't, the value of one or more could change as the condition is being evaluated... There is also locking overhead in each method as it is re-acquired.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@appleguy In ASViewController we pass in the overwritten constrained size in measureWithSizeRange: so this will calling through to the measure method and dirty layout is false

Will add acquiring a lock for supportsRangeManagedInterfaceState and check for has dirty layout. In [self supernode] we already grab a lock

if (CGRectEqualToRect(bounds, CGRectZero)) {
LOG(@"Warning: No size given for node before node was trying to layout itself: %@. Please provide a frame for the node.", self);
} else {
[self measureWithSizeRange:ASSizeRangeMake(bounds.size, bounds.size)];
}
}
}

- (void)layout
{
ASDisplayNodeAssertMainThread();

if ([self _hasDirtyLayout]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't read correctly; is it correct? Shouldn't it be == NO? If the layout is dirty, then we should probably layout our subnodes.

Maybe this means _isLayoutCalculated, or _isMeasured

Copy link
Contributor Author

@maicki maicki Jul 15, 2016

Choose a reason for hiding this comment

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

I just moved this method, before the _hasDirtyLayout change it was !_flags.isMeasured so the logic did not change. I have to talk with @nguyenhuy about this but this could be the same reason as in here: #1902

return;
}

[self __layoutSubnodes];
}

- (void)__layoutSubnodes
{
for (ASLayout *subnodeLayout in _layout.sublayouts) {
((ASDisplayNode *)subnodeLayout.layoutableObject).frame = [subnodeLayout frame];
}
}

- (void)layoutDidFinish
{
// Hook for subclasses
}

- (CATransform3D)_transformToAncestor:(ASDisplayNode *)ancestor
Expand Down Expand Up @@ -2463,24 +2501,6 @@ - (void)_applyLayout:(ASLayout *)layout layoutTransition:(ASLayoutTransition *)l
[layoutTransition startTransition];
}

- (void)layout
{
ASDisplayNodeAssertMainThread();

if ([self _hasDirtyLayout]) {
return;
}

[self __layoutSublayouts];
}

- (void)__layoutSublayouts
{
for (ASLayout *subnodeLayout in _layout.sublayouts) {
((ASDisplayNode *)subnodeLayout.layoutableObject).frame = [subnodeLayout frame];
}
}

- (void)displayWillStart
{
ASDisplayNodeAssertMainThread();
Expand Down
3 changes: 1 addition & 2 deletions examples/Videos/Sample/ViewController.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@
// ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
// CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
//
#import <AsyncDisplayKit/AsyncDisplayKit.h>
#import <AsyncDisplayKit/ASVideoNode.h>
#import <UIKit/UIKit.h>

@interface ViewController : UIViewController
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. Technically this should be an ASViewController, but of course as-is it makes for a useful test case for this...although a bit of a "secret" purpose / functionality!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, but we need that for testing purposes.


Expand Down
20 changes: 5 additions & 15 deletions examples/Videos/Sample/ViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@
//

#import "ViewController.h"
#import "ASLayoutSpec.h"
#import "ASStaticLayoutSpec.h"
#import <AsyncDisplayKit/AsyncDisplayKit.h>

@interface ViewController()<ASVideoNodeDelegate>
@property (nonatomic, strong) ASDisplayNode *rootNode;
Expand All @@ -28,12 +27,13 @@ @implementation ViewController

#pragma mark - UIViewController

- (void)viewWillAppear:(BOOL)animated
- (void)viewDidLoad
{
[super viewWillAppear:animated];

[super viewDidLoad];
// Root node for the view controller
_rootNode = [ASDisplayNode new];
_rootNode.frame = self.view.bounds;
_rootNode.autoresizingMask = UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight;

ASVideoNode *guitarVideoNode = self.guitarVideoNode;
Expand Down Expand Up @@ -68,16 +68,6 @@ - (void)viewWillAppear:(BOOL)animated
[self.view addSubnode:_rootNode];
}

- (void)viewDidLayoutSubviews
{
[super viewDidLayoutSubviews];

// After all subviews are layed out we have to measure it and move the root node to the right place
CGSize viewSize = self.view.bounds.size;
[self.rootNode measureWithSizeRange:ASSizeRangeMake(viewSize, viewSize)];
[self.rootNode setNeedsLayout];
}

#pragma mark - Getter / Setter

- (ASVideoNode *)guitarVideoNode;
Expand Down