feat: linear gradient ios #45434

intergalacticspacehighway · 2024-07-15T10:22:12Z

Summary:

Adds background prop that supports CSS's linear gradient. Later this can be extended to support various other gradients and possibly CSS's background image (less motivation as better solutions exists for image)
Uses CAGradientlayer to draw Linear Gradient layers. So it is GPU optimised under the hood.
Style supports JS object to specify LinearGradient, so it can support Animated libraries.

Changelog:

[IOS] [ADDED] - linear gradient

Test Plan:

Check out processBackground-test.js for supported syntax testcases.
Checkout example added in ViewExample.js

Although the PR is tested well but open to any changes/feedback on the approach taken.

Android PR - #45433. Separated the PRs to keep it easier to review. Both PRs can be reviewed individually.

analysis-bot · 2024-07-15T10:51:53Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	21,385,873	+18,392
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	24,581,455	+17,234
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: 2eb7bcb
Branch: main

joevilches

There are a lot of shared files with this impl and the android one in #45433. I did not review those files since @NickGerleman added some suggestions over there. In general I think we should ship the Android one first since it has all of that discussion for those files, and update this PR to reflect those changes (or just make it so we do not have 2 separate things trying to add the same logic)

I did review the iOS specifics and left a few questions. Thanks for doing this btw, its awesome to see!

Gonna request changes just for the reminder to update this PR with the shared code situation mentioned above

joevilches · 2024-07-24T18:49:48Z

packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm

@@ -228,6 +229,7 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
    self.backgroundColor = RCTUIColorFromSharedColor(newViewProps.backgroundColor);
    needsInvalidateLayer = YES;
  }
+


nit: remove extra space

joevilches · 2024-07-24T18:51:04Z

packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm

@@ -413,6 +420,7 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
  _props = std::static_pointer_cast<const ViewProps>(props);
 }

+


same, remove extra space

joevilches · 2024-07-24T18:51:10Z

packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm

@@ -761,7 +769,7 @@ - (void)invalidateLayer
        self.layer.opacity *= primitive.amount;
      }
    }
-
+  


joevilches · 2024-07-24T20:22:45Z

packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm

+    for (const auto& colorStop : gradientPrimitive.colorStops) {
+      if (colorStop.position.has_value()) {
+        auto location = @(colorStop.position.value());
+        UIColor* color = RCTUIColorFromSharedColor(colorStop.color);
+        [colors addObject:(id) color.CGColor];
+        [locations addObject:location];
+      }
+    }


will this just not add some colors if they do not have a position? in what cases would that happen? i.e. what cases do we have a color with an empty position

what cases do we have a color with an empty position

That should not happen. In view config processors we make sure that position is always set. We set default position also here if user doesn't pass a position.

joevilches · 2024-07-24T20:23:02Z

packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm

+
+    // So that border layer is always above gradient layer
+    gradientLayer.zPosition = _borderLayer.zPosition;
+    [self.layer insertSublayer:gradientLayer atIndex:0];


is there a reason this is at index 0? The z position should take care of that dimension for the most part. If you want gradients defined first in the array to be layered on top of the latter ones, maybe consider iterating over the gradients in reverse order so that you can just append to the end here.

Updated this to below. Tried to explain the approach here in a little bit longer answer

// So that border layer is always above gradient layer gradientLayer.zPosition = _borderLayer.zPosition - 1; [self.layer addSublayer:gradientLayer];

joevilches · 2024-07-24T20:23:15Z

packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm

+    if (useCoreAnimationBorderRendering) {
+      gradientLayer.borderWidth = (CGFloat)borderMetrics.borderWidths.left;
+      CGColorRef borderColor = RCTCreateCGColorRefFromSharedColor(borderMetrics.borderColors.left);
+      gradientLayer.borderColor = borderColor;
+      CGColorRelease(borderColor);
+      gradientLayer.cornerRadius = (CGFloat)borderMetrics.borderRadii.topLeft;
+      gradientLayer.cornerCurve = CornerCurveFromBorderCurve(borderMetrics.borderCurves.topLeft);
+      gradientLayer.shouldRasterize = YES;
+      gradientLayer.drawsAsynchronously = YES;
+    } else {
+      CAShapeLayer* maskLayer = [CAShapeLayer layer];
+      CGPathRef path = RCTPathCreateWithRoundedRect(self.bounds, RCTGetCornerInsets(
+        RCTCornerRadiiFromBorderRadii(borderMetrics.borderRadii), 
+        UIEdgeInsetsZero), nil);
+      maskLayer.path = path;
+      CGPathRelease(path);
+      gradientLayer.mask = maskLayer;
+    }
+    [_gradientLayers addObject:gradientLayer];


I am a bit confused why this layer has a border. Borders will go over the background, so as long as this layer is under the border layer it should be fine. Is this more about how the color is distributed when there is a border that pushes the content inward?

intergalacticspacehighway · 2024-08-02T11:00:27Z

Hi @joevilches thanks for review. I have rebased this against the latest main that has android changes merged. Will try to clarify the above questions here. Let me know if there is a better way to achieve this.

Based on how current border style works there are two cases:

if useCoreAnimationBorderRendering is true no new border layer is appended and border styles are set to view's root layer itself using .cornerRadius, .borderWidth etc
or else, we append a border layer to root layer (for non uniform, different style borders) and set border styles using _borderLayer.contents (it is an image). Background color and borders are drawn on borderLayer. e.g. here

Now when we append gradient layers to root view layer, it should support above two cases for borders to work correctly with them.

If useCoreAnimationBorderRendering is true, we append gradient layer to the root layer but gradient layer won't inherit root layer's border styles. So we need to set .cornerRadius, .borderWidth, .borderColor etc to gradient layers, similar to how we set for root layer.
if a border layer is added, it should appear above all the gradient layers so we set gradient layers zPosition = borderLayer.zPosition - 1. Gradient layers won't clip to border layer's image path even if border layer is on top, so we need to mask gradient layers to clip to border layer's path.

Added border with linear gradient examples here.

Aside

Another approach would be to draw gradients with Core graphics in border layer's image instead of using CA gradient layers for non uniform border styles somewhere here. But that would be CPU intensive instead of GPU. That would be cleaner but might not be a good tradeoff.

joevilches · 2024-08-05T21:12:10Z

packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm

@@ -831,6 +838,56 @@ - (void)invalidateLayer
  }

  [_boxShadowLayer removeFromSuperlayer];


this might be result of a merge that I committed recently, but can you put your code above this line so box shadow logic is together

my bad i missed that, fixed.

joevilches · 2024-08-05T21:44:07Z

@intergalacticspacehighway thanks! this looks good. One thing I just thought of is that I think this breaks with non uniform borders and a background color set. Since the image that we set on the border layer just fills with the background color, and that covers this gradient layers, we run into and issue where we no longer see the gradient. That is honestly a problem with how the border layer is drawn - it should not fill with background color, it should just leave that empty I think, but there seems to be some reasoning why. I do not think the burden is on you to fix that and for users the fix is just don't set background color. But just calling that out

facebook-github-bot · 2024-08-05T21:48:42Z

@joevilches has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

intergalacticspacehighway · 2024-08-05T21:49:19Z

@joevilches yeah I imagined this issue, i honestly thought about it late at night and forgot to add in description sorry 😅. But I agree that the solution involves separating border and background layer drawings. Users can simply choose not to add background color for gradients, not sure if there would be such a usecase anyway.

facebook-github-bot · 2024-08-06T21:04:33Z

@joevilches merged this pull request in b99675d.

react-native-bot · 2024-08-06T21:04:38Z

This pull request was successfully merged by @intergalacticspacehighway in b99675d

^{When will my fix make it into a release? | How to file a pick request?}

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jul 15, 2024

joevilches requested changes Jul 24, 2024

View reviewed changes

intergalacticspacehighway mentioned this pull request Aug 2, 2024

feat: linear gradient android #45433

Closed

ios backgroundImage changes

11fdf1b

intergalacticspacehighway force-pushed the feat/linear-gradient-ios branch from 5fbec53 to 11fdf1b Compare August 2, 2024 09:21

set gradient layer's zposition = borderLayer's zposition - 1

0030c35

intergalacticspacehighway requested a review from joevilches August 2, 2024 11:08

intergalacticspacehighway added 4 commits August 2, 2024 23:24

gradient example with border style

22aa3dd

remove early optimisation and use border value from layer

8df2d61

nit: improve comment

d1024e8

nit cleanup

54dd08f

joevilches reviewed Aug 5, 2024

View reviewed changes

fix box shadow cleanup loc

72eb65a

joevilches approved these changes Aug 5, 2024

View reviewed changes

facebook-github-bot closed this in b99675d Aug 6, 2024

facebook-github-bot added the Merged This PR has been merged. label Aug 6, 2024

intergalacticspacehighway deleted the feat/linear-gradient-ios branch August 9, 2024 22:42

vonovak mentioned this pull request Aug 14, 2024

[expo-linear-gradient] Require at least two colors in typescript expo/expo#27820

Merged

3 tasks

intergalacticspacehighway mentioned this pull request Oct 15, 2024

fix: start and end points linear gradient algorithm #47003

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: linear gradient ios #45434

feat: linear gradient ios #45434

intergalacticspacehighway commented Jul 15, 2024 •

edited

Loading

analysis-bot commented Jul 15, 2024 •

edited

Loading

joevilches left a comment

joevilches Jul 24, 2024

joevilches Jul 24, 2024

joevilches Jul 24, 2024

joevilches Jul 24, 2024

intergalacticspacehighway Aug 2, 2024 •

edited

Loading

joevilches Jul 24, 2024

intergalacticspacehighway Aug 2, 2024

joevilches Jul 24, 2024

intergalacticspacehighway commented Aug 2, 2024 •

edited

Loading

joevilches Aug 5, 2024

intergalacticspacehighway Aug 5, 2024

joevilches commented Aug 5, 2024

facebook-github-bot commented Aug 5, 2024

intergalacticspacehighway commented Aug 5, 2024

facebook-github-bot commented Aug 6, 2024

react-native-bot commented Aug 6, 2024

		@@ -413,6 +420,7 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
		_props = std::static_pointer_cast<const ViewProps>(props);
		}

		@@ -831,6 +838,56 @@ - (void)invalidateLayer
		}

		[_boxShadowLayer removeFromSuperlayer];

feat: linear gradient ios #45434

feat: linear gradient ios #45434

Conversation

intergalacticspacehighway commented Jul 15, 2024 • edited Loading

Summary:

Changelog:

Test Plan:

analysis-bot commented Jul 15, 2024 • edited Loading

joevilches left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

intergalacticspacehighway Aug 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

intergalacticspacehighway commented Aug 2, 2024 • edited Loading

Aside

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joevilches commented Aug 5, 2024

facebook-github-bot commented Aug 5, 2024

intergalacticspacehighway commented Aug 5, 2024

facebook-github-bot commented Aug 6, 2024

react-native-bot commented Aug 6, 2024

intergalacticspacehighway commented Jul 15, 2024 •

edited

Loading

analysis-bot commented Jul 15, 2024 •

edited

Loading

intergalacticspacehighway Aug 2, 2024 •

edited

Loading

intergalacticspacehighway commented Aug 2, 2024 •

edited

Loading