Skip to content

Commit b1e8729

Browse files
rubennortefacebook-github-bot
rubennorte
authored and
facebook-github-bot
committed
Fix incorrect view flattening when using a specific not fully transparent color (#51379)
Summary: Pull Request resolved: #51379 Changelog: [General][Fixed] Fix incorrect flattening / non-rendering of views with backgroundColor set to `rgba(255, 255, 255, 127/256)` Fixes #51378. ## Context When testing some unrelated things with Fantom is realized that the color for some text that I wasn't explicitly defining was being set to `rgba(255, 255, 255, 127)`, like here: https://github.com/facebook/react-native/blob/249a24ac756275eadbe3b4df1ff9c974af1671d2/packages/react-native-fantom/src/__tests__/Fantom-itest.js#L540-L542 When digging a bit more about why, I realized that was actually the value for `UndefinedColor`. When looking a bit deeper, I saw that the value for that constant was being set like this: ``` using Color = int32_t; namespace HostPlatformColor { static const facebook::react::Color UndefinedColor = std::numeric_limits<facebook::react::Color>::max(); } ``` I'm not sure what the logic could've been here: - Defining it as a value out of bounds for all valid colors? In this case, it's a 32 bit value so all the range of values are actually valid RGBA colors. - Defining it as a fully opaque white? Seems dangerous for a default because you wouldn't be able to distinguish a explicitly set white color from a non-set color, relevant if you're seeing a white background color in a view on top of another view with any other background color. The result of this existing logic was actually setting `UndefinedColor` to `rgba(255, 255, 255, 127)` because the alpha channel is defined in the first bits of the value, and `Color` being a signed int with 32 bits, the largest value is `01111....1`, so extracting the first 8 bits, you get 127. ## Changes This changes the value set for the `UndefinedColor` constant (which is used, among other things, to determine if a view sets a background color, or otherwise could potentially be flattened). The new value, instead of white with a 127/256 opacity, is black with 0% opacity (or simply the number 0 in `int32_t`). Reviewed By: javache Differential Revision: D74869311 fbshipit-source-id: 5582b4803b0b5c72cb3c1b33720c4542c5e3f1de
1 parent 6581c69 commit b1e8729

5 files changed

Lines changed: 136 additions & 114 deletions

File tree

‎packages/react-native-fantom/src/__tests__/Fantom-itest.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ describe('Fantom', () => {
538538
},
539539
],
540540
props: {
541-
foregroundColor: 'rgba(255, 255, 255, 127)',
541+
foregroundColor: 'rgba(0, 0, 0, 0)',
542542
},
543543
type: 'Paragraph',
544544
});

‎packages/react-native/Libraries/Components/__tests__/Button-itest.js‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,7 @@ describe('<Button>', () => {
3333
).toEqual(
3434
// Upper case on Android (also used by Fantom)
3535
<rn-view backgroundColor="rgba(33, 150, 243, 255)">
36-
<rn-paragraph
37-
backgroundColor="rgba(255, 255, 255, 127)"
38-
foregroundColor="rgba(255, 255, 255, 255)">
36+
<rn-paragraph foregroundColor="rgba(255, 255, 255, 255)">
3937
HELLO
4038
</rn-paragraph>
4139
</rn-view>,
@@ -57,9 +55,7 @@ describe('<Button>', () => {
5755
.toJSX(),
5856
).toEqual(
5957
<rn-view backgroundColor="rgba(0, 0, 255, 255)">
60-
<rn-paragraph
61-
backgroundColor="rgba(255, 255, 255, 127)"
62-
foregroundColor="rgba(255, 255, 255, 255)">
58+
<rn-paragraph foregroundColor="rgba(255, 255, 255, 255)">
6359
HELLO
6460
</rn-paragraph>
6561
</rn-view>,
@@ -105,9 +101,7 @@ describe('<Button>', () => {
105101
.toJSX(),
106102
).toEqual(
107103
<rn-view backgroundColor="rgba(223, 223, 223, 255)">
108-
<rn-paragraph
109-
backgroundColor="rgba(255, 255, 255, 127)"
110-
foregroundColor="rgba(161, 161, 161, 255)">
104+
<rn-paragraph foregroundColor="rgba(161, 161, 161, 255)">
111105
HELLO
112106
</rn-paragraph>
113107
</rn-view>,
@@ -189,9 +183,7 @@ describe('<Button>', () => {
189183
.toJSX(),
190184
).toEqual(
191185
<rn-view backgroundColor="rgba(33, 150, 243, 255)">
192-
<rn-paragraph
193-
backgroundColor="rgba(255, 255, 255, 127)"
194-
foregroundColor="rgba(255, 255, 255, 255)">
186+
<rn-paragraph foregroundColor="rgba(255, 255, 255, 255)">
195187
HELLO
196188
</rn-paragraph>
197189
</rn-view>,
@@ -233,9 +225,7 @@ describe('<Button>', () => {
233225
.toJSX(),
234226
).toEqual(
235227
<rn-view backgroundColor="rgba(223, 223, 223, 255)">
236-
<rn-paragraph
237-
backgroundColor="rgba(255, 255, 255, 127)"
238-
foregroundColor="rgba(161, 161, 161, 255)">
228+
<rn-paragraph foregroundColor="rgba(161, 161, 161, 255)">
239229
HELLO
240230
</rn-paragraph>
241231
</rn-view>,

‎packages/react-native/ReactCommon/react/renderer/graphics/platform/android/react/renderer/graphics/HostPlatformColor.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ namespace facebook::react {
1515
using Color = int32_t;
1616

1717
namespace HostPlatformColor {
18-
static const facebook::react::Color UndefinedColor =
19-
std::numeric_limits<facebook::react::Color>::max();
18+
constexpr facebook::react::Color UndefinedColor = 0;
2019
}
2120

2221
inline Color

‎packages/react-native/ReactCommon/react/renderer/graphics/platform/cxx/react/renderer/graphics/HostPlatformColor.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ namespace facebook::react {
1616
using Color = int32_t;
1717

1818
namespace HostPlatformColor {
19-
static const facebook::react::Color UndefinedColor =
20-
std::numeric_limits<facebook::react::Color>::max();
19+
constexpr facebook::react::Color UndefinedColor = 0;
2120
}
2221

2322
inline Color

‎packages/react-native/src/private/renderer/mounting/__tests__/Mounting-itest.js‎

Lines changed: 128 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -231,125 +231,159 @@ describe('ViewFlattening', () => {
231231
'Insert {type: "View", parentNativeID: "J", index: 1, nativeID: "B"}',
232232
]);
233233
});
234-
});
235234

236-
test('parent-child switching from unflattened-flattened to flattened-unflattened', () => {
237-
const root = Fantom.createRoot();
235+
test('parent-child switching from unflattened-flattened to flattened-unflattened', () => {
236+
const root = Fantom.createRoot();
238237

239-
Fantom.runTask(() => {
240-
root.render(
241-
<View
242-
style={{
243-
marginTop: 100,
244-
opacity: 0,
245-
}}>
238+
Fantom.runTask(() => {
239+
root.render(
246240
<View
247241
style={{
248-
marginTop: 50,
242+
marginTop: 100,
243+
opacity: 0,
249244
}}>
250245
<View
251-
nativeID={'child'}
252-
style={{height: 10, width: 10, backgroundColor: 'red'}}
253-
/>
254-
</View>
255-
</View>,
256-
);
257-
});
246+
style={{
247+
marginTop: 50,
248+
}}>
249+
<View
250+
nativeID={'child'}
251+
style={{height: 10, width: 10, backgroundColor: 'red'}}
252+
/>
253+
</View>
254+
</View>,
255+
);
256+
});
258257

259-
expect(root.takeMountingManagerLogs()).toEqual([
260-
'Update {type: "RootView", nativeID: (root)}',
261-
'Create {type: "View", nativeID: (N/A)}',
262-
'Create {type: "View", nativeID: "child"}',
263-
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
264-
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
265-
]);
266-
267-
// force view to be flattened.
268-
Fantom.runTask(() => {
269-
root.render(
270-
<View
271-
style={{
272-
marginTop: 100,
273-
}}>
258+
expect(root.takeMountingManagerLogs()).toEqual([
259+
'Update {type: "RootView", nativeID: (root)}',
260+
'Create {type: "View", nativeID: (N/A)}',
261+
'Create {type: "View", nativeID: "child"}',
262+
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
263+
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
264+
]);
265+
266+
// force view to be flattened.
267+
Fantom.runTask(() => {
268+
root.render(
274269
<View
275270
style={{
276-
marginTop: 50,
277-
opacity: 0,
271+
marginTop: 100,
278272
}}>
279273
<View
280-
nativeID={'child'}
281-
style={{height: 10, width: 10, backgroundColor: 'red'}}
282-
/>
283-
</View>
284-
</View>,
285-
);
274+
style={{
275+
marginTop: 50,
276+
opacity: 0,
277+
}}>
278+
<View
279+
nativeID={'child'}
280+
style={{height: 10, width: 10, backgroundColor: 'red'}}
281+
/>
282+
</View>
283+
</View>,
284+
);
285+
});
286+
287+
expect(root.takeMountingManagerLogs()).toEqual([
288+
'Update {type: "View", nativeID: "child"}',
289+
'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
290+
'Remove {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
291+
'Delete {type: "View", nativeID: (N/A)}',
292+
'Create {type: "View", nativeID: (N/A)}',
293+
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
294+
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
295+
]);
286296
});
287297

288-
expect(root.takeMountingManagerLogs()).toEqual([
289-
'Update {type: "View", nativeID: "child"}',
290-
'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
291-
'Remove {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
292-
'Delete {type: "View", nativeID: (N/A)}',
293-
'Create {type: "View", nativeID: (N/A)}',
294-
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
295-
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
296-
]);
297-
});
298+
test('parent-child switching from flattened-unflattened to unflattened-flattened', () => {
299+
const root = Fantom.createRoot();
298300

299-
test('parent-child switching from flattened-unflattened to unflattened-flattened', () => {
300-
const root = Fantom.createRoot();
301+
Fantom.runTask(() => {
302+
root.render(
303+
<View
304+
style={{
305+
marginTop: 100,
306+
}}>
307+
<View
308+
style={{
309+
marginTop: 50,
310+
opacity: 0,
311+
}}>
312+
<View nativeID={'child'} style={{height: 10, width: 10}} />
313+
</View>
314+
</View>,
315+
);
316+
});
301317

302-
Fantom.runTask(() => {
303-
root.render(
304-
<View
305-
style={{
306-
marginTop: 100,
307-
}}>
318+
expect(root.takeMountingManagerLogs()).toEqual([
319+
'Update {type: "RootView", nativeID: (root)}',
320+
'Create {type: "View", nativeID: (N/A)}',
321+
'Create {type: "View", nativeID: "child"}',
322+
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
323+
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
324+
]);
325+
326+
// force view to be flattened.
327+
Fantom.runTask(() => {
328+
root.render(
308329
<View
309330
style={{
310-
marginTop: 50,
331+
marginTop: 100,
311332
opacity: 0,
312333
}}>
313-
<View nativeID={'child'} style={{height: 10, width: 10}} />
314-
</View>
315-
</View>,
316-
);
334+
<View
335+
style={{
336+
marginTop: 50,
337+
}}>
338+
<View nativeID={'child'} style={{height: 10, width: 10}} />
339+
</View>
340+
</View>,
341+
);
342+
});
343+
expect(root.takeMountingManagerLogs()).toEqual([
344+
'Update {type: "View", nativeID: "child"}',
345+
'Remove {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
346+
'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
347+
'Delete {type: "View", nativeID: (N/A)}',
348+
'Create {type: "View", nativeID: (N/A)}',
349+
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
350+
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
351+
]);
317352
});
318353

319-
expect(root.takeMountingManagerLogs()).toEqual([
320-
'Update {type: "RootView", nativeID: (root)}',
321-
'Create {type: "View", nativeID: (N/A)}',
322-
'Create {type: "View", nativeID: "child"}',
323-
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
324-
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
325-
]);
326-
327-
// force view to be flattened.
328-
Fantom.runTask(() => {
329-
root.render(
330-
<View
331-
style={{
332-
marginTop: 100,
333-
opacity: 0,
334-
}}>
354+
test('#51378: view with rgba(255,255,255,127/256) background color is not flattened', () => {
355+
const root = Fantom.createRoot();
356+
357+
Fantom.runTask(() => {
358+
root.render(
335359
<View
336360
style={{
337-
marginTop: 50,
338-
}}>
339-
<View nativeID={'child'} style={{height: 10, width: 10}} />
340-
</View>
341-
</View>,
361+
width: 100,
362+
height: 100,
363+
backgroundColor: `rgba(255, 255, 255, ${127 / 256})`,
364+
}}
365+
/>,
366+
);
367+
});
368+
369+
expect(root.takeMountingManagerLogs()).toEqual([
370+
'Update {type: "RootView", nativeID: (root)}',
371+
'Create {type: "View", nativeID: (N/A)}',
372+
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
373+
]);
374+
375+
expect(
376+
root
377+
.getRenderedOutput({props: ['width', 'height', 'backgroundColor']})
378+
.toJSX(),
379+
).toEqual(
380+
<rn-view
381+
width="100.000000"
382+
height="100.000000"
383+
backgroundColor="rgba(255, 255, 255, 127)"
384+
/>,
342385
);
343386
});
344-
expect(root.takeMountingManagerLogs()).toEqual([
345-
'Update {type: "View", nativeID: "child"}',
346-
'Remove {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
347-
'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
348-
'Delete {type: "View", nativeID: (N/A)}',
349-
'Create {type: "View", nativeID: (N/A)}',
350-
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
351-
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
352-
]);
353387
});
354388

355389
describe('reconciliation of setNativeProps and React commit', () => {

0 commit comments

Comments
 (0)