Permalink
Browse files

Removing `inherited background color` optimization from RCTText

Summary:
This is a long story. Awhile ago awesome Nick Lockwood (Hey Nick!) introduced a special optimization for ReactNative rendering layer called "inherited background color".
He described this idea in D2811031:
>>>
Blending semitransparent pixels against their background is fairly a fairly expensive operation on mobile GPUs. To reduce blending, React Native has a system called "background color propagation", where the background color of parent views is automatically inherited by child views unless explicitly overridden. This means that translucent pixels can be blended directly against a known background color, avoiding the need to do this dynamically on the GPU.
In practice, this is only useful for views that do their own drawing, which is basically just <Image/> and <Text/> components, and for image components it only really matters when the image has an alpha component.
The automatic background propagation is a bit of a hack, and often does the wrong thing - for example if a view overflows its bounds, or if it overlaps a sibling, the background color will often be incorrect and need to be manually disabled. Because the only place that it provides a significant performance benefit is for text, this diff disables the behavior for everything except <Text/> nodes. It might still be useful for <Image/> nodes too, but looking through the examples in UIExplorer, the number of places where it does the wrong thing for images outnumbers the cases where it provides significant reduction in blending.

However. I think it is time to remove it. Why? There are several reasons:

* It drastically complicates rendering layer. DRASTICALLY. In many many unrelated places (try search for "backgroundColor"!);
* This mechanism is totally non-conceptual to RN and it prevents us to implement some new possible render optimization that we plan to do;
* This adopted only by two components now: Text and ART;
* This is not a significant performance drain anymore; from iOS 6 even UILabel has clear background color by default.
* I doubt that it even works now because `drawRect:` in Text component does not call super method.

So, this diff just turns this feature off for Text. If all performance metrics are neutral, I will delete this mechanism.
Peace.

Reviewed By: sahrens

Differential Revision: D6564199

fbshipit-source-id: 70524fdd955ca32bbf86d2d1ff5e73316b791219
  • Loading branch information...
shergin authored and facebook-github-bot committed Dec 15, 2017
1 parent d326c86 commit 8c8944c10fb7dc30ea99657225f25ea438cf6e14
Showing with 0 additions and 5 deletions.
  1. +0 −5 Libraries/Text/RCTText.m
  2. BIN ...NTesterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testARTExample_1-iOS10@2x.png
  3. BIN ...NTesterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testARTExample_1-iOS11@2x.png
  4. BIN ...ster/RNTesterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testARTExample_1@2x.png
  5. BIN ...sterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testLayoutExample_1-iOS10@2x.png
  6. BIN ...sterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testLayoutExample_1-iOS11@2x.png
  7. BIN ...r/RNTesterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testLayoutExample_1@2x.png
  8. BIN ...IntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testScrollViewExample_1-iOS10@2x.png
  9. BIN ...IntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testScrollViewExample_1-iOS11@2x.png
  10. BIN ...TesterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testScrollViewExample_1@2x.png
  11. BIN ...sterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testSliderExample_1-iOS10@2x.png
  12. BIN ...sterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testSliderExample_1-iOS11@2x.png
  13. BIN ...r/RNTesterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testSliderExample_1@2x.png
  14. BIN ...sterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testSwitchExample_1-iOS10@2x.png
  15. BIN ...sterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testSwitchExample_1-iOS11@2x.png
  16. BIN ...r/RNTesterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testSwitchExample_1@2x.png
  17. BIN ...sterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testTabBarExample_1-iOS10@2x.png
  18. BIN ...sterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testTabBarExample_1-iOS11@2x.png
  19. BIN ...r/RNTesterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testTabBarExample_1@2x.png
  20. BIN ...TesterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testTextExample_1-iOS10@2x.png
  21. BIN ...TesterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testTextExample_1-iOS11@2x.png
  22. BIN ...ter/RNTesterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testTextExample_1@2x.png
  23. BIN ...TesterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testViewExample_1-iOS10@2x.png
  24. BIN ...TesterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testViewExample_1-iOS11@2x.png
  25. BIN ...ter/RNTesterIntegrationTests/ReferenceImages/RNTester-js-RNTesterApp.ios/testViewExample_1@2x.png
@@ -80,11 +80,6 @@ - (void)reactSetFrame:(CGRect)frame
}];
}
- (void)reactSetInheritedBackgroundColor:(UIColor *)inheritedBackgroundColor
{
self.backgroundColor = inheritedBackgroundColor;
}
- (void)didUpdateReactSubviews
{
// Do nothing, as subviews are managed by `setTextStorage:` method

3 comments on commit 8c8944c

@janicduplessis

This comment has been minimized.

Collaborator

janicduplessis replied Dec 15, 2017

🙏 As much as I understand the perf optimisation this was definitely annoying especially when developing on Android then coming back on iOS with a bunch of broken text with a random background.

Posted about this a while back #7964

@janicduplessis

This comment has been minimized.

Collaborator

janicduplessis replied Dec 15, 2017

@shergin Curious about what type of rendering optimisation you are looking at. View flattening / custom drawing à la litho?

@shergin

This comment has been minimized.

Contributor

shergin replied Dec 18, 2017

@janicduplessis We are thinking about many types of conceptual changes in RN rendering model and about how UIManager communicates with React. But, before we even can think about it concretely, I have to remove a lot of... technical debt... from RN rendering infra. My goal is to have really simple, logically clean rendering layer. Even this work itself should bring significant performance benefits. I hope.

Please sign in to comment.