From 471eefcb746f501a012492cef20565671fb53773 Mon Sep 17 00:00:00 2001 From: Emil Sjolander Date: Mon, 25 Jul 2016 03:48:50 -0700 Subject: [PATCH] Move dirty layout into css-layout Reviewed By: majak Differential Revision: D3610634 fbshipit-source-id: 1dc9017c0a34ced231b5bebe334591f3d0b89bf3 --- Libraries/Text/RCTShadowRawText.m | 2 -- Libraries/Text/RCTShadowText.m | 4 +-- React/CSSLayout/CSSLayout-internal.h | 3 +- React/CSSLayout/CSSLayout.c | 43 ++++++++++++++++++----- React/CSSLayout/CSSLayout.h | 7 ++-- React/Modules/RCTUIManager.m | 10 +----- React/Views/RCTShadowView.h | 7 ---- React/Views/RCTShadowView.m | 51 ++-------------------------- 8 files changed, 47 insertions(+), 80 deletions(-) diff --git a/Libraries/Text/RCTShadowRawText.m b/Libraries/Text/RCTShadowRawText.m index 6d1dd6d25..12db15382 100644 --- a/Libraries/Text/RCTShadowRawText.m +++ b/Libraries/Text/RCTShadowRawText.m @@ -31,7 +31,6 @@ - (void)contentSizeMultiplierDidChange:(NSNotification *)note { - [self dirtyLayout]; [self dirtyText]; } @@ -39,7 +38,6 @@ { if (_text != text && ![_text isEqualToString:text]) { _text = [text copy]; - [self dirtyLayout]; [self dirtyText]; } } diff --git a/Libraries/Text/RCTShadowText.m b/Libraries/Text/RCTShadowText.m index 4243dbff7..96b6ed20e 100644 --- a/Libraries/Text/RCTShadowText.m +++ b/Libraries/Text/RCTShadowText.m @@ -88,7 +88,7 @@ static CSSSize RCTMeasure(void *context, float width, CSSMeasureMode widthMode, - (void)contentSizeMultiplierDidChange:(NSNotification *)note { - [self dirtyLayout]; + CSSNodeMarkDirty(self.cssNode); [self dirtyText]; } @@ -330,7 +330,7 @@ static CSSSize RCTMeasure(void *context, float width, CSSMeasureMode widthMode, // create a non-mutable attributedString for use by the Text system which avoids copies down the line _cachedAttributedString = [[NSAttributedString alloc] initWithAttributedString:attributedString]; - [self dirtyLayout]; + CSSNodeMarkDirty(self.cssNode); return _cachedAttributedString; } diff --git a/React/CSSLayout/CSSLayout-internal.h b/React/CSSLayout/CSSLayout-internal.h index b3d12c34a..2c65026cd 100644 --- a/React/CSSLayout/CSSLayout-internal.h +++ b/React/CSSLayout/CSSLayout-internal.h @@ -89,12 +89,13 @@ typedef struct CSSNode { int lineIndex; bool shouldUpdate; bool isTextNode; + CSSNodeRef parent; CSSNodeListRef children; + bool isDirty; struct CSSNode* nextChild; CSSSize (*measure)(void *context, float width, CSSMeasureMode widthMode, float height, CSSMeasureMode heightMode); - bool (*isDirty)(void *context); void (*print)(void *context); void *context; } CSSNode; diff --git a/React/CSSLayout/CSSLayout.c b/React/CSSLayout/CSSLayout.c index a7b48ff61..49136970f 100644 --- a/React/CSSLayout/CSSLayout.c +++ b/React/CSSLayout/CSSLayout.c @@ -33,7 +33,6 @@ CSSNodeRef CSSNodeNew() { CSSNodeRef node = calloc(1, sizeof(CSSNode)); assert(node != NULL); - node->children = CSSNodeListNew(4); CSSNodeInit(node); return node; } @@ -44,6 +43,11 @@ void CSSNodeFree(CSSNodeRef node) { } void CSSNodeInit(CSSNodeRef node) { + node->parent = NULL; + node->children = CSSNodeListNew(4); + node->shouldUpdate = true; + node->isDirty = false; + node->style.alignItems = CSSAlignStretch; node->style.alignContent = CSSAlignFlexStart; @@ -79,7 +83,6 @@ void CSSNodeInit(CSSNodeRef node) { // Such that the comparison is always going to be false node->layout.lastParentDirection = (CSSDirection)-1; - node->shouldUpdate = true; node->layout.nextCachedMeasurementsIndex = 0; node->layout.measuredDimensions[CSSDimensionWidth] = CSSUndefined; @@ -88,12 +91,25 @@ void CSSNodeInit(CSSNodeRef node) { node->layout.cached_layout.heightMeasureMode = (CSSMeasureMode)-1; } +void _CSSNodeMarkDirty(CSSNodeRef node) { + if (!node->isDirty) { + node->isDirty = true; + if (node->parent) { + _CSSNodeMarkDirty(node->parent); + } + } +} + void CSSNodeInsertChild(CSSNodeRef node, CSSNodeRef child, unsigned int index) { CSSNodeListInsert(node->children, child, index); + child->parent = node; + _CSSNodeMarkDirty(node); } void CSSNodeRemoveChild(CSSNodeRef node, CSSNodeRef child) { CSSNodeListDelete(node->children, child); + child->parent = NULL; + _CSSNodeMarkDirty(node); } CSSNodeRef CSSNodeGetChild(CSSNodeRef node, unsigned int index) { @@ -104,32 +120,40 @@ unsigned int CSSNodeChildCount(CSSNodeRef node) { return CSSNodeListCount(node->children); } +void CSSNodeMarkDirty(CSSNodeRef node) { + // Nodes without custom measure functions should not manually mark themselves as dirty + assert(node->measure != NULL); + _CSSNodeMarkDirty(node); +} + #define CSS_NODE_PROPERTY_IMPL(type, name, paramName, instanceName) \ void CSSNodeSet##name(CSSNodeRef node, type paramName) { \ - node->instanceName = paramName;\ + node->instanceName = paramName; \ } \ \ type CSSNodeGet##name(CSSNodeRef node) { \ - return node->instanceName;\ + return node->instanceName; \ } \ #define CSS_NODE_STYLE_PROPERTY_IMPL(type, name, paramName, instanceName) \ void CSSNodeStyleSet##name(CSSNodeRef node, type paramName) { \ - node->style.instanceName = paramName;\ + if (node->style.instanceName != paramName) { \ + node->style.instanceName = paramName; \ + _CSSNodeMarkDirty(node); \ + } \ } \ \ type CSSNodeStyleGet##name(CSSNodeRef node) { \ - return node->style.instanceName;\ + return node->style.instanceName; \ } \ #define CSS_NODE_LAYOUT_PROPERTY_IMPL(type, name, instanceName) \ type CSSNodeLayoutGet##name(CSSNodeRef node) { \ - return node->layout.instanceName;\ + return node->layout.instanceName; \ } \ CSS_NODE_PROPERTY_IMPL(void*, Context, context, context); CSS_NODE_PROPERTY_IMPL(CSSMeasureFunc, MeasureFunc, measureFunc, measure); -CSS_NODE_PROPERTY_IMPL(CSSIsDirtyFunc, IsDirtyFunc, isDirtyFunc, isDirty); CSS_NODE_PROPERTY_IMPL(CSSPrintFunc, PrintFunc, printFunc, print); CSS_NODE_PROPERTY_IMPL(bool, IsTextnode, isTextNode, isTextNode); CSS_NODE_PROPERTY_IMPL(bool, ShouldUpdate, shouldUpdate, shouldUpdate); @@ -1735,7 +1759,7 @@ bool layoutNodeInternal(CSSNode* node, float availableWidth, float availableHeig gDepth++; - bool needToVisitNode = (node->isDirty(node->context) && layout->generationCount != gCurrentGenerationCount) || + bool needToVisitNode = (node->isDirty && layout->generationCount != gCurrentGenerationCount) || layout->lastParentDirection != parentDirection; if (needToVisitNode) { @@ -1867,6 +1891,7 @@ bool layoutNodeInternal(CSSNode* node, float availableWidth, float availableHeig node->layout.dimensions[CSSDimensionWidth] = node->layout.measuredDimensions[CSSDimensionWidth]; node->layout.dimensions[CSSDimensionHeight] = node->layout.measuredDimensions[CSSDimensionHeight]; node->shouldUpdate = true; + node->isDirty = false; } gDepth--; diff --git a/React/CSSLayout/CSSLayout.h b/React/CSSLayout/CSSLayout.h index af721d9ea..23d38372d 100644 --- a/React/CSSLayout/CSSLayout.h +++ b/React/CSSLayout/CSSLayout.h @@ -110,7 +110,6 @@ typedef struct CSSSize { typedef struct CSSNode * CSSNodeRef; typedef CSSSize (*CSSMeasureFunc)(void *context, float width, CSSMeasureMode widthMode, float height, CSSMeasureMode heightMode); -typedef bool (*CSSIsDirtyFunc)(void *context); typedef void (*CSSPrintFunc)(void *context); // CSSNode @@ -129,6 +128,11 @@ void CSSNodeCalculateLayout( float availableHeight, CSSDirection parentDirection); +// Mark a node as dirty. Only valid for nodes with a custom measure function set. +// CSSLayout knows when to mark all other nodes as dirty but because nodes with measure functions +// depends on information not known to CSSLayout they must perform this dirty marking manually. +void CSSNodeMarkDirty(CSSNodeRef node); + void CSSNodePrint(CSSNodeRef node, CSSPrintOptions options); bool isUndefined(float value); @@ -146,7 +150,6 @@ type CSSNodeLayoutGet##name(CSSNodeRef node); CSS_NODE_PROPERTY(void*, Context, context); CSS_NODE_PROPERTY(CSSMeasureFunc, MeasureFunc, measureFunc); -CSS_NODE_PROPERTY(CSSIsDirtyFunc, IsDirtyFunc, isDirtyFunc); CSS_NODE_PROPERTY(CSSPrintFunc, PrintFunc, printFunc); CSS_NODE_PROPERTY(bool, IsTextnode, isTextNode); CSS_NODE_PROPERTY(bool, ShouldUpdate, shouldUpdate); diff --git a/React/Modules/RCTUIManager.m b/React/Modules/RCTUIManager.m index bed6629b3..b55135742 100644 --- a/React/Modules/RCTUIManager.m +++ b/React/Modules/RCTUIManager.m @@ -424,11 +424,8 @@ dispatch_queue_t RCTGetUIManagerQueue(void) RCTShadowView *shadowView = self->_shadowViewRegistry[reactTag]; RCTAssert(shadowView != nil, @"Could not locate shadow view with tag #%@", reactTag); - BOOL dirtyLayout = NO; - if (!CGRectEqualToRect(frame, shadowView.frame)) { shadowView.frame = frame; - dirtyLayout = YES; } // Trigger re-layout when size flexibility changes, as the root view might grow or @@ -437,14 +434,9 @@ dispatch_queue_t RCTGetUIManagerQueue(void) RCTRootShadowView *rootShadowView = (RCTRootShadowView *)shadowView; if (rootShadowView.sizeFlexibility != sizeFlexibility) { rootShadowView.sizeFlexibility = sizeFlexibility; - dirtyLayout = YES; + [self batchDidComplete]; } } - - if (dirtyLayout) { - [shadowView dirtyLayout]; - [self batchDidComplete]; - } }); } diff --git a/React/Views/RCTShadowView.h b/React/Views/RCTShadowView.h index 8547c1ddc..58c9dbbf7 100644 --- a/React/Views/RCTShadowView.h +++ b/React/Views/RCTShadowView.h @@ -47,7 +47,6 @@ typedef void (^RCTApplierBlock)(NSDictionary *viewRegistry @property (nonatomic, assign, readonly) CSSNodeRef cssNode; @property (nonatomic, copy) NSString *viewName; @property (nonatomic, strong) UIColor *backgroundColor; // Used to propagate to children -@property (nonatomic, assign) RCTUpdateLifecycle layoutLifecycle; @property (nonatomic, copy) RCTDirectEventBlock onLayout; /** @@ -183,12 +182,6 @@ typedef void (^RCTApplierBlock)(NSDictionary *viewRegistry viewsWithNewFrame:(NSMutableSet *)viewsWithNewFrame absolutePosition:(CGPoint)absolutePosition; -/** - * The following are implementation details exposed to subclasses. Do not call them directly - */ -- (void)dirtyLayout NS_REQUIRES_SUPER; -- (BOOL)isLayoutDirty; - /** * Return whether or not this node acts as a leaf node in the eyes of CSSLayout. For example * RCTShadowText has children which it does not want CSSLayout to lay out so in the eyes of diff --git a/React/Views/RCTShadowView.m b/React/Views/RCTShadowView.m index 2bd6cf390..eafd667ae 100644 --- a/React/Views/RCTShadowView.m +++ b/React/Views/RCTShadowView.m @@ -56,12 +56,6 @@ static void RCTPrint(void *context) printf("%s(%zd), ", shadowView.viewName.UTF8String, shadowView.reactTag.integerValue); } -static bool RCTIsDirty(void *context) -{ - RCTShadowView *shadowView = (__bridge RCTShadowView *)context; - return [shadowView isLayoutDirty]; -} - // Enforces precedence rules, e.g. marginLeft > marginHorizontal > margin. #define DEFINE_PROCESS_META_PROPS(type) \ static void RCTProcessMetaProps##type(const float metaProps[META_PROP_COUNT], CSSNodeRef node) { \ @@ -147,7 +141,6 @@ DEFINE_PROCESS_META_PROPS(Border); return; } CSSNodeSetShouldUpdate(node, false); - _layoutLifecycle = RCTUpdateLifecycleComputed; CGPoint absoluteTopLeft = { absolutePosition.x + CSSNodeLayoutGetLeft(node), @@ -263,11 +256,6 @@ DEFINE_PROCESS_META_PROPS(Border); CSSNodeStyleSetHeight(_cssNode, frame.size.height); CSSNodeStyleSetPositionLeft(_cssNode, frame.origin.x); CSSNodeStyleSetPositionTop(_cssNode, frame.origin.y); - - // Our parent has asked us to change our cssNode->styles. Dirty the layout - // so that we can rerun layout on this node. The request came from our parent - // so there's no need to dirty our ancestors by calling dirtyLayout. - _layoutLifecycle = RCTUpdateLifecycleDirtied; } CSSNodeCalculateLayout(_cssNode, frame.size.width, frame.size.height, CSSDirectionInherit); @@ -304,7 +292,6 @@ DEFINE_PROCESS_META_PROPS(Border); } _newView = YES; - _layoutLifecycle = RCTUpdateLifecycleUninitialized; _propagationLifecycle = RCTUpdateLifecycleUninitialized; _textLifecycle = RCTUpdateLifecycleUninitialized; @@ -313,7 +300,6 @@ DEFINE_PROCESS_META_PROPS(Border); _cssNode = CSSNodeNew(); CSSNodeSetContext(_cssNode, (__bridge void *)self); CSSNodeSetPrintFunc(_cssNode, RCTPrint); - CSSNodeSetIsDirtyFunc(_cssNode, RCTIsDirty); } return self; } @@ -328,19 +314,6 @@ DEFINE_PROCESS_META_PROPS(Border); CSSNodeFree(_cssNode); } -- (void)dirtyLayout -{ - if (_layoutLifecycle != RCTUpdateLifecycleDirtied) { - _layoutLifecycle = RCTUpdateLifecycleDirtied; - [_superview dirtyLayout]; - } -} - -- (BOOL)isLayoutDirty -{ - return _layoutLifecycle != RCTUpdateLifecycleComputed; -} - - (BOOL)isCSSLeafNode { return NO; @@ -386,14 +359,12 @@ DEFINE_PROCESS_META_PROPS(Border); subview->_superview = self; _didUpdateSubviews = YES; [self dirtyText]; - [self dirtyLayout]; [self dirtyPropagation]; } - (void)removeReactSubview:(RCTShadowView *)subview { [subview dirtyText]; - [subview dirtyLayout]; [subview dirtyPropagation]; _didUpdateSubviews = YES; subview->_superview = nil; @@ -533,7 +504,6 @@ RCT_BORDER_PROPERTY(Right, RIGHT) - (void)set##setProp:(CGFloat)value \ { \ CSSNodeStyleSet##cssProp(_cssNode, value); \ - [self dirtyLayout]; \ [self dirtyText]; \ } \ - (CGFloat)getProp \ @@ -561,40 +531,31 @@ RCT_DIMENSION_PROPERTY(Left, left, PositionLeft) CSSNodeStyleSetPositionTop(_cssNode, CGRectGetMinY(frame)); CSSNodeStyleSetWidth(_cssNode, CGRectGetWidth(frame)); CSSNodeStyleSetHeight(_cssNode, CGRectGetHeight(frame)); - [self dirtyLayout]; } -static inline BOOL RCTAssignSuggestedDimension(CSSNodeRef cssNode, CSSDimension dimension, CGFloat amount) +static inline void RCTAssignSuggestedDimension(CSSNodeRef cssNode, CSSDimension dimension, CGFloat amount) { if (amount != UIViewNoIntrinsicMetric) { switch (dimension) { case CSSDimensionWidth: if (isnan(CSSNodeStyleGetWidth(cssNode))) { CSSNodeStyleSetWidth(cssNode, amount); - return YES; } break; case CSSDimensionHeight: if (isnan(CSSNodeStyleGetHeight(cssNode))) { CSSNodeStyleSetHeight(cssNode, amount); - return YES; } break; } } - - return NO; } - (void)setIntrinsicContentSize:(CGSize)size { if (CSSNodeStyleGetFlex(_cssNode) == 0) { - BOOL dirty = NO; - dirty |= RCTAssignSuggestedDimension(_cssNode, CSSDimensionHeight, size.height); - dirty |= RCTAssignSuggestedDimension(_cssNode, CSSDimensionWidth, size.width); - if (dirty) { - [self dirtyLayout]; - } + RCTAssignSuggestedDimension(_cssNode, CSSDimensionHeight, size.height); + RCTAssignSuggestedDimension(_cssNode, CSSDimensionWidth, size.width); } } @@ -602,14 +563,12 @@ static inline BOOL RCTAssignSuggestedDimension(CSSNodeRef cssNode, CSSDimension { CSSNodeStyleSetPositionLeft(_cssNode, topLeft.x); CSSNodeStyleSetPositionTop(_cssNode, topLeft.y); - [self dirtyLayout]; } - (void)setSize:(CGSize)size { CSSNodeStyleSetWidth(_cssNode, size.width); CSSNodeStyleSetHeight(_cssNode, size.height); - [self dirtyLayout]; } // Flex @@ -618,7 +577,6 @@ static inline BOOL RCTAssignSuggestedDimension(CSSNodeRef cssNode, CSSDimension - (void)set##setProp:(type)value \ { \ CSSNodeStyleSet##cssProp(_cssNode, value); \ - [self dirtyLayout]; \ } \ - (type)getProp \ { \ @@ -665,9 +623,6 @@ RCT_STYLE_PROPERTY(FlexWrap, flexWrap, FlexWrap, CSSWrapType) if (_recomputeBorder) { RCTProcessMetaPropsBorder(_borderMetaProps, _cssNode); } - if (_recomputePadding || _recomputeMargin || _recomputeBorder) { - [self dirtyLayout]; - } _recomputeMargin = NO; _recomputePadding = NO; _recomputeBorder = NO;