Repository navigation
Commit 089a101
Read only the drawn child's mix-blend-mode in Android drawChild (#59016)
Summary:
`ReactViewGroup.drawChild` and `ReactRootView.drawChild` call `BlendModeHelper.needsIsolatedLayer(this)` before they read the drawn child's `mix_blend_mode` tag. `needsIsolatedLayer` reads that tag on every child of the group, and `dispatchDraw` calls `drawChild` once per child, so recording the display list of a group with N children reads N² tags. When no child sets a blend mode — the common case — `any` never short-circuits, so the whole scan runs for every child of every group that records.
I found it profiling flings over a grid of a few hundred views on two release builds: sampling the main thread, `needsIsolatedLayer` under `drawChild` was 37% of display-list recording on a Snapdragon 845 (Android 10) and 46% on a Snapdragon 8 Gen 2 (Android 14).
The guard adds nothing to the decision `drawChild` makes. A child that carries the tag makes `needsIsolatedLayer` true by definition, and a child without it draws with no layer whatever the guard says. So both overrides now read the drawn child's own tag. `draw()` keeps its group-wide check, which decides whether the whole group renders offscreen and runs once per recording.
I searched the tracker for `needsIsolatedLayer`, `BlendModeHelper`, `mix_blend_mode` and `drawChild` and found no existing report or PR.
## Changelog:
[ANDROID] [FIXED] - Drawing a view group no longer reads every sibling's mix-blend-mode for each child it draws
Pull Request resolved: #59016
Test Plan:
- `./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests "com.facebook.react.views.view.ReactViewGroupTest" -Preact.internal.useHermesStable=true`: on `main` the new `drawing a child reads no sibling's blend mode` test fails with `expected:<[3]> but was:<[21]>` — each child of a 20-child group has its tag read 21 times, against 3 in a 2-child group; with the fix both groups read each child equally often, and all 19 tests in the class pass.
- The new `a child with a blend mode draws into a layer with that blend mode` test records `[null, MULTIPLY]` — the group's isolated layer, then the child's — before and after the change; with the child's layer skipped it fails with `Expecting actual: [null] to contain exactly (and in same order): [null, MULTIPLY]`.
- `./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest -PenableWarningsAsErrors=true -Preact.internal.useHermesStable=true`: 707 tests, 0 failures.
- `node ./scripts/format-kotlin.js --check` and `node ./scripts/format-java.js --check` pass.
- The same change on 0.86.3, in an app with a method trace over those flings: `needsIsolatedLayer` ran 1,608 times for 1,400 `drawChild` calls before and 182 times for 3,234 after, every one of them from `draw()`. A `mixBlendMode: 'multiply'` view in the same app still blends.
- Not run: the RNTester build and Android lint (`build_android`).
Reviewed By: andrewdacenko
Differential Revision: D124965507
Pulled By: javache
fbshipit-source-id: e0155f6de592f9d14e8cb17a6b092b5fda711d041 parent 9b7c956 commit 089a101
3 files changed
Lines changed: 52 additions & 3 deletions
File tree
- packages/react-native/ReactAndroid/src
- main/java/com/facebook/react
- views/view
- test/java/com/facebook/react/views/view
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
12 | 11 | | |
13 | 12 | | |
14 | 13 | | |
| |||
308 | 307 | | |
309 | 308 | | |
310 | 309 | | |
311 | | - | |
| 310 | + | |
312 | 311 | | |
313 | 312 | | |
314 | 313 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1003 | 1003 | | |
1004 | 1004 | | |
1005 | 1005 | | |
1006 | | - | |
| 1006 | + | |
1007 | 1007 | | |
1008 | 1008 | | |
1009 | 1009 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
12 | 15 | | |
13 | 16 | | |
14 | 17 | | |
| 18 | + | |
15 | 19 | | |
16 | 20 | | |
17 | 21 | | |
| |||
67 | 71 | | |
68 | 72 | | |
69 | 73 | | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
70 | 100 | | |
71 | 101 | | |
72 | 102 | | |
| |||
294 | 324 | | |
295 | 325 | | |
296 | 326 | | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
0 commit comments