From 0d6d58656afbedc88beeda2644cf572fd77f44a4 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Tue, 10 Mar 2020 16:57:20 -0700 Subject: [PATCH] Fabric: EmptyLayoutMetrics does not contain invalid values anymore Summary: Before this change, fields of EmptyLayoutMetrics have some "invalid" values to allow us to compare equal them individually and get `false`. Turned out that having invalid values there might break some serialization layers, which is no good. This change fixes that and adds explicit check for EmptyLayoutMetrics before running a comparison of individual fields. Changelog: [Internal] Fabric-specific internal change. Reviewed By: mdvacca Differential Revision: D20324881 fbshipit-source-id: ab8e2a402f6bdfb393fc9b6789decb526fa94dfa --- .../Mounting/UIView+ComponentViewProtocol.mm | 8 +++--- .../fabric/core/layout/LayoutMetrics.h | 25 +++---------------- 2 files changed, 8 insertions(+), 25 deletions(-) diff --git a/React/Fabric/Mounting/UIView+ComponentViewProtocol.mm b/React/Fabric/Mounting/UIView+ComponentViewProtocol.mm index 04c201d773d7be..c9dc5f98d7ac90 100644 --- a/React/Fabric/Mounting/UIView+ComponentViewProtocol.mm +++ b/React/Fabric/Mounting/UIView+ComponentViewProtocol.mm @@ -63,7 +63,9 @@ - (void)handleCommand:(NSString *)commandName args:(NSArray *)args - (void)updateLayoutMetrics:(LayoutMetrics const &)layoutMetrics oldLayoutMetrics:(LayoutMetrics const &)oldLayoutMetrics { - if (layoutMetrics.frame != oldLayoutMetrics.frame) { + bool forceUpdate = oldLayoutMetrics == EmptyLayoutMetrics; + + if (forceUpdate || (layoutMetrics.frame != oldLayoutMetrics.frame)) { CGRect frame = RCTCGRectFromRect(layoutMetrics.frame); if (!std::isfinite(frame.origin.x) || !std::isfinite(frame.origin.y) || !std::isfinite(frame.size.width) || @@ -86,13 +88,13 @@ - (void)updateLayoutMetrics:(LayoutMetrics const &)layoutMetrics self.bounds = CGRect{CGPointZero, frame.size}; } - if (layoutMetrics.layoutDirection != oldLayoutMetrics.layoutDirection) { + if (forceUpdate || (layoutMetrics.layoutDirection != oldLayoutMetrics.layoutDirection)) { self.semanticContentAttribute = layoutMetrics.layoutDirection == LayoutDirection::RightToLeft ? UISemanticContentAttributeForceRightToLeft : UISemanticContentAttributeForceLeftToRight; } - if (layoutMetrics.displayType != oldLayoutMetrics.displayType) { + if (forceUpdate || (layoutMetrics.displayType != oldLayoutMetrics.displayType)) { self.hidden = layoutMetrics.displayType == DisplayType::None; } } diff --git a/ReactCommon/fabric/core/layout/LayoutMetrics.h b/ReactCommon/fabric/core/layout/LayoutMetrics.h index 10fd949f63ee78..12980f65f257d3 100644 --- a/ReactCommon/fabric/core/layout/LayoutMetrics.h +++ b/ReactCommon/fabric/core/layout/LayoutMetrics.h @@ -57,29 +57,10 @@ struct LayoutMetrics { * Represents some undefined, not-yet-computed or meaningless value of * `LayoutMetrics` type. * The value is comparable by equality with any other `LayoutMetrics` value. - * All individual sub-properties of `EmptyLayoutMetrics` have the most possible - * "invalid" values; this is useful when we compare them with some valid values. */ -static const LayoutMetrics EmptyLayoutMetrics = { - /* .frame = */ { - /* .origin = */ {std::numeric_limits::min(), - std::numeric_limits::min()}, - /* .size = */ - {std::numeric_limits::min(), std::numeric_limits::min()}, - }, - /* .contentInsets = */ - {std::numeric_limits::min(), - std::numeric_limits::min(), - std::numeric_limits::min(), - std::numeric_limits::min()}, - /* .borderWidth = */ - {std::numeric_limits::min(), - std::numeric_limits::min(), - std::numeric_limits::min(), - std::numeric_limits::min()}, - /* .displayType = */ (DisplayType)-1, - /* .layoutDirection = */ (LayoutDirection)-1, - /* .pointScaleFactor = */ std::numeric_limits::min()}; +static LayoutMetrics const EmptyLayoutMetrics = { + /* .frame = */ {{0, 0}, {-1.0, -1.0}} +}; } // namespace react } // namespace facebook