Skip to content

Commit 001d046

Browse files
rubennortefacebook-github-bot
rubennorte
authored and
facebook-github-bot
committed
Fix incorrect view flattening when using a specific not fully transparent color (facebook#51379)
Summary: Changelog: [General][Fixed] Fix incorrect flattening / non-rendering of views with backgroundColor set to `rgba(255, 255, 255, 127/256)` Fixes facebook#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 back with 0% opacity (or simply the number 0 in `int32_t`). Differential Revision: D74869311
1 parent 249a24a commit 001d046

File tree

3 files changed

+130
-97
lines changed

3 files changed

+130
-97
lines changed

‎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/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+
static const 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
@@ -232,125 +232,159 @@ describe('ViewFlattening', () => {
232232
'Insert {type: "View", parentNativeID: "J", index: 1, nativeID: "B"}',
233233
]);
234234
});
235-
});
236235

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

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

260-
expect(root.takeMountingManagerLogs()).toEqual([
261-
'Update {type: "RootView", nativeID: (root)}',
262-
'Create {type: "View", nativeID: (N/A)}',
263-
'Create {type: "View", nativeID: "child"}',
264-
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
265-
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
266-
]);
267-
268-
// force view to be flattened.
269-
Fantom.runTask(() => {
270-
root.render(
271-
<View
272-
style={{
273-
marginTop: 100,
274-
}}>
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(
275270
<View
276271
style={{
277-
marginTop: 50,
278-
opacity: 0,
272+
marginTop: 100,
279273
}}>
280274
<View
281-
nativeID={'child'}
282-
style={{height: 10, width: 10, backgroundColor: 'red'}}
283-
/>
284-
</View>
285-
</View>,
286-
);
275+
style={{
276+
marginTop: 50,
277+
opacity: 0,
278+
}}>
279+
<View
280+
nativeID={'child'}
281+
style={{height: 10, width: 10, backgroundColor: 'red'}}
282+
/>
283+
</View>
284+
</View>,
285+
);
286+
});
287+
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+
]);
287297
});
288298

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

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

303-
Fantom.runTask(() => {
304-
root.render(
305-
<View
306-
style={{
307-
marginTop: 100,
308-
}}>
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(
309330
<View
310331
style={{
311-
marginTop: 50,
332+
marginTop: 100,
312333
opacity: 0,
313334
}}>
314-
<View nativeID={'child'} style={{height: 10, width: 10}} />
315-
</View>
316-
</View>,
317-
);
335+
<View
336+
style={{
337+
marginTop: 50,
338+
}}>
339+
<View nativeID={'child'} style={{height: 10, width: 10}} />
340+
</View>
341+
</View>,
342+
);
343+
});
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+
]);
318353
});
319354

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

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

0 commit comments

Comments
 (0)