Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rotate transform not working well on View with borderRadius #18266

Open
brunolemos opened this issue Mar 8, 2018 · 41 comments
Open

Rotate transform not working well on View with borderRadius #18266

brunolemos opened this issue Mar 8, 2018 · 41 comments
Labels
Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Platform: Android Android applications. Ran Commands One of our bots successfully processed a command. Resolution: Fixed A PR that fixes this issue has been merged.

Comments

@brunolemos
Copy link
Contributor

brunolemos commented Mar 8, 2018

When rotating a component, its children also rotate as expected, except for the Views with borderRadius > 0, which their rounded border does not rotate.

Environment

Environment:
OS: macOS High Sierra 10.13.3
Node: 8.9.0
Yarn: 1.5.1
npm: 5.7.1
Watchman: 4.9.0
Xcode: Xcode 9.2 Build version 9C40b
Android Studio: 3.0 AI-171.4443003

Packages: (wanted => installed)
react: ^16.3.0-alpha.1 => 16.3.0-alpha.1
react-native: ^0.54.0 => 0.54.0

Expected Behavior

Borders should rotate like everything else.

Actual Behavior

SCREENSHOT GIF

^ The one from the right has overflow: hidden on the main photo container, so things got worse. We actually can notice it's not just the border, but the actual view mask / clip bounds that's not rotating.

Steps to Reproduce

[ANDROID] https://snack.expo.io/@brunolemos/react-native---border-radius-rotate-bug

Full Code
import React, { Component } from 'react';
import { Image, Text, View, StyleSheet } from 'react-native';
import { Constants } from 'expo';

export default class App extends Component {
  render() {
    return (
      <View style={styles.container}>
        <View style={styles.block}>
          <View style={styles.textContainer}>
            <Text>SQUARE</Text>
            <Image
              source={require('./assets/logo_facebook_square.png')}
              style={styles.image}
            />
          </View>
          <View style={[styles.textContainer, { borderRadius: 10 }]}>
            <Text>ROUND</Text>
            <Image
              source={require('./assets/logo_facebook_square.png')}
              style={[styles.image, { borderRadius: 10 }]}
            />
          </View>
        </View>

        <View style={[styles.block, { transform: [{ rotate: '-10deg' }] }]}>
          <View style={styles.textContainer}>
            <Text>SQUARE</Text>
            <Image
              source={require('./assets/logo_facebook_square.png')}
              style={styles.image}
            />
          </View>
          <View style={[styles.textContainer, { borderRadius: 10 }]}>
            <Text>ROUND</Text>
            <Image
              source={require('./assets/logo_facebook_square.png')}
              style={[styles.image, { borderRadius: 10 }]}
            />
          </View>
        </View>
      </View>
    );
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    alignItems: 'center',
    justifyContent: 'center',
    paddingTop: Constants.statusBarHeight,
    backgroundColor: '#ecf0f1',
  },
  block: {
    justifyContent: 'center',
    alignItems: 'center',
    marginBottom: 60,
    width: 200,
    height: 200,
    backgroundColor: 'white',
  },
  textContainer: {
    flexDirection: 'row',
    justifyContent: 'center',
    alignItems: 'center',
    margin: 10,
    padding: 10,
    borderWidth: 2,
    borderColor: 'red',
  },
  image: {
    width: 40,
    height: 40,
    borderWidth: 4,
    borderColor: 'green',
  },
});

Investigating + Related

After more investigation this is what I found:

  • Everything works fine on react-native 0.49.5;
  • rotate + borderRadius > 0 + overflow:hidden bug introduced on 0.50.0; (30044fd?)
  • rotate + borderRadius > 0 + borderWidth > 0 bug introduced on 0.51.0; (4994d6a?)
  • Bug still present on 0.54.0.

Possibly related:

@react-native-bot

This comment has been minimized.

@react-native-bot react-native-bot added Ran Commands One of our bots successfully processed a command. Old Version labels Mar 8, 2018
brunolemos referenced this issue Mar 8, 2018
Reviewed By: shergin

Differential Revision: D5917111

fbshipit-source-id: e3d97f26b6aada199f700ec6659ace0d7dffd4c5
brunolemos referenced this issue Mar 8, 2018
Reviewed By: achen1

Differential Revision: D5982241

fbshipit-source-id: 2f694daca7e1b16b5ff65f07c7d15dd558a4b7e8
@adeelraza

This comment has been minimized.

@zachkirsch

This comment has been minimized.

1 similar comment
@LauR3y

This comment has been minimized.

@zachkirsch
Copy link

I was able to solve this from removing overflow: “hidden” from the Views’ style (on Android). I had been using it to clip the corners of my container View so that the border radius applied to the entire component, but instead I now just have the border radius style on every View in my component.

@Symyon
Copy link

Symyon commented Apr 6, 2018

I have a similar issue, latest RN (0.54) and no overflow: “hidden” used.
Was hopping the new release will cure that, but it seems it's not the case.

@jacksmith3888
Copy link

0.55.2 the same
works fine on ios

@kelset kelset added the Platform: Android Android applications. label Apr 11, 2018
@lufinkey
Copy link

lufinkey commented Apr 15, 2018

I was able to get overflow: 'hidden' to work by also adding the style zIndex: 1 to the same element. It seems #3198 was closed.

@naoey
Copy link

naoey commented Apr 19, 2018

Apart from child Views with borders not rotating, the rotated View's own border is misbehaving too.

Before rotation:
image

After a 45deg rotation:
image

This is on 55.3

@brunolemos

This comment has been minimized.

@RSNara
Copy link
Contributor

RSNara commented Jun 7, 2018

Hey @brunolemos!

Your problem reproduces on React Native ~0.55.2. As evidence, here's a screenshot:

image

But I also ran your code against React Native master and saw that the rounded border rendering has been fixed. Screenshot:

image

However, it still looks like clipping is broken (with rounded borders).

Code: https://snack.expo.io/Hyc6HMPxX

Edit: I did some digging around and narrowed it down to the canvas.clipPath call inside ReactViewGroup.drawdispatchOverflowDraw. Clipping works perfectly fine if I zero all the radii and use canvas.clipPath. However, if I leave the radii non-zero, it fails. An interesting consequence is that when borderWidth > borderRadius, clipping works correctly (because the clipped rectangle won't be rounded at that point).

I'll look into this a bit further.

@RSNara
Copy link
Contributor

RSNara commented Jun 10, 2018

Yup, so this wasn't a React Native issue; it was an AOSP bug. I just tested clipping in API 26, API 27, and API 28, and verified that it was broken in API 26, and 27, but was working in API 28.

Code: https://snack.expo.io/Skj_iWjxX

React Native v0.55.0 running on my S8 (Android 8.0, API 26):
image

React Native master, running on API 28 emulator:
image

So, rounded border rendering works as expected under transformations. Also, clipping works as expected for sufficiently new devices. @hramos, can we close this issue?

@brunolemos
Copy link
Contributor Author

brunolemos commented Jun 10, 2018

@RSNara can we close this issue?

Everything worked fine on react-native@0.49.5 and got broke on 0.51, so clearly there is some fix to be made here. Unless the fix is released on npm I don't see why this issue should be closed.

@RSNara
Copy link
Contributor

RSNara commented Jun 10, 2018

Why would you close if everything worked fine on react-native@0.49.5 and got broke on 0.50+? Unless the fix is released on npm I don't see why a bug should be closed.

From my understanding, there were two problems here.

  1. Border rendering was re-implemented sometime around v0.51.0, which may likely have broken rotated rounded borders. This problem has since been fixed in React Native. Since the fix is in master, it'll eventually be shipped to npm.
  2. Sometime around v0.51.0 a clipping algorithm was also implemented to support overflow: hidden for views with non-rounded borders. I'm not completely sure about the history of borders and overflow: hidden in React Native, but I just tried v0.49.5 and overflow: hidden doesn't work at all. Child views render on top of their parent's borders, but are clipped to the outer edge of their parent regardless of the overflow property.
    image
    This clipping algorithm worked perfectly fine for rotated views. However, when it was extended to account for rounded borders, it broke due to what I think was an AOSP bug. Whatever the probem was, it was resolved in the latest release of Android.

So really, overflow: hidden is the only thing that's presently broken. Specifically, I think it's broken whenever the inner border edge has a curved corner, which happens when borderRadius > borderWidth. This problem also only occurs on Android API < 28.

From where I stand, I see two options:

  1. Do nothing and wait, and expect app developers to work around this problem. Apps running the latest edition of React Native (what's currently on master and what will eventually be released on npm) and running on the latest edition of Android do not experience this problem. Plus, there's an easy way to circumvent this problem, which is to ensure that borderWidth > borderRadius or to not use borders. This is a bit of a bummer, I know.
  2. Hack together a solution for overflow:hidden on android devices running API < 28. Maybe this involves adding a nested view so the contents of the border are clipped to the outer edge of the inner view. Or maybe, there's---there probably is---a better solution. I'm really not sure. (What I do know is that we have to be careful about view hierarchy depth on older Android devices, because they can easily run into stack overflow errors. So, maybe the multi-layer solution isn't the best idea?)

Previously, I was leaning more toward the first option. That's why I suggested that we should close this issue. But now that I think about it a bit more, I'm not sure what the best approach is. Any thoughts, @shergin?

Edit 1: I'll do some more thorough testing and see if things actually are working.
Edit 2: Hmm.... so it looks like API 28 works perfectly for all cases I've tried, but with borders whose width is specified using border(Top|Left|Bottom|Right)Width, devices running API 27 and below don't render rotated borders correctly. Sigh...

@developdeez

This comment has been minimized.

@hramos hramos added Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. labels Jun 13, 2018
@hramos
Copy link
Contributor

hramos commented Jun 13, 2018

Tagged as a regression so the bot does not close this due to staleness. Thanks @RSNara for the writeup. 0.56.0-rc should have the fixes you mentioned.

@ghost

This comment has been minimized.

@tominou
Copy link

tominou commented Apr 13, 2019

I also still have this issue on some devices.
Working fine on Android 6.0.1 but not on 8.1.0.
Using RN 0.59.4

@swdenglian
Copy link

swdenglian commented May 6, 2019

I also still have this issue on Android 8.0.0(honer EMUI 8.0.0) when i use "overflow:'hidden'" and " transform: [{ rotate: "45deg" }]".
It's normal when i use transform-rotate alone.

React And ReactNative Version:
"react": "16.8.1",
"react-native": "0.59.5",

@ovy9086
Copy link

ovy9086 commented May 16, 2019

so why is this closed ? as this clearly still happens. I assume the RN implementation uses clipChildren=true when a borderRadius is specified in the Style, and this causes the above mentioned issue.

@tominou
Copy link

tominou commented May 16, 2019

Im not sure whether it is an Android or React Native issue, but here is what is working using RN 0.59.8:
Android 21 - Fine
Android 22 - Fine
Android 23 - Fine
Android 24 - Not working
Android 25 - Not working
Android 26 - Not working
Android 27 - Not working
Android 28 - Not working
Android Q - Fine

@matthargett matthargett reopened this May 17, 2019
@matthargett
Copy link
Contributor

Based on @tominou ‘s table above, this is an OS bug? If that’s the case, is the best option to document this properly with a PR that includes the above table?

@Tencryn
Copy link

Tencryn commented Jun 18, 2019

Is there any workaround for this? Currently facing it myself on Android 28 and haven't found any success in my searches for a fix.

@Fawxy
Copy link

Fawxy commented Aug 7, 2019

We are also running into this issue. Tried removing individual border radius widths and sizes, however that breaks specifying different border radius colours for each side on the affect platforms.

Changing this:

const AStyledView = styled.View`
  border-top-width: 8;
  border-bottom-width: 8;
  border-right-width: 8;
  border-left-width: 8;
  border-top-right-radius: 50;
  border-top-left-radius: 50;
  border-bottom-left-radius: 50;
  border-bottom-right-radius: 50;
  border-right-color: yellow;
  border-top-color: yellow;
  border-left-color: transparent;
  border-bottom-color: transparent;
  transform: rotateZ(45deg);
`;

To this:

const AStyledView = styled.View`
  border-width: 8;
  border-radius: 50;
  border-right-color: yellow;
  border-top-color: yellow;
  border-left-color: transparent;
  border-bottom-color: transparent;
  transform: rotateZ(45deg);
`;

Breaks the separate border colours, they are all black on android. Works on iOS.

facebook-github-bot pushed a commit that referenced this issue Mar 16, 2020
Summary:
This sync includes the following changes:
- **[b5c6dd2de](facebook/react@b5c6dd2de )**: Don't use Spread in DevTools Injection (#18277) //<Sebastian Markbåge>//
- **[a463fef31](facebook/react@a463fef31 )**: Revert "[React Native] Add getInspectorDataForViewAtPoint (#18233)" //<Sebastian Markbage>//
- **[dc7eedae3](facebook/react@dc7eedae3 )**: Encode server rendered host components as array tuples (#18273) //<Sebastian Markbåge>//
- **[bf351089a](facebook/react@bf351089a )**: [React Native] Add getInspectorDataForViewAtPoint (#18233) //<Ricky>//
- **[99d737186](facebook/react@99d737186 )**: [Flight] Split Streaming from Relay Implemenation (#18260) //<Sebastian Markbåge>//
- **[160505b0c](facebook/react@160505b0c )**: ReactDOM.useEvent: Add more scaffolding for useEvent hook (#18271) //<Dominic Gannaway>//
- **[526c12f49](facebook/react@526c12f49 )**: Enable enableProfilerCommitHooks flag for FB (#18230) //<Brian Vaughn>//
- **[29534252a](facebook/react@29534252a )**: ReactDOM.useEvent add flag and entry point (#18267) //<Dominic Gannaway>//
- **[704c8b011](facebook/react@704c8b011 )**: Fix Flow type for AnyNativeEvent (#18266) //<Dominic Gannaway>//
- **[bdc5cc463](facebook/react@bdc5cc463 )**: Add Relay Flight Build (#18242) //<Sebastian Markbåge>//
- **[7a1691cdf](facebook/react@7a1691cdf )**: Refactor Host Config Infra (getting rid of .inline*.js) (#18240) //<Sebastian Markbåge>//
- **[238b57f0f](facebook/react@238b57f0f )**: [Blocks] Make it possible to have lazy initialized and lazy loaded Blocks (#18220) //<Sebastian Markbåge>//
- **[235a6c4af](facebook/react@235a6c4af )**: Bugfix: Dropped effects in Legacy Mode Suspense (#18238) //<Andrew Clark>//
- **[562cf013d](facebook/react@562cf013d )**: Add a flag to disable module pattern components (#18133) //<Dan Abramov>//
- **[115cd12d9](facebook/react@115cd12d9 )**: Add test run that uses www feature flags (#18234) //<Andrew Clark>//
- **[4027f2a3b](facebook/react@4027f2a3b )**: Break up require/import statements in strings (#18222) //<Christoph Nakazawa>//
- **[024a76431](facebook/react@024a76431 )**: Implemented Profiler onCommit() and onPostCommit() hooks (#17910) //<Brian Vaughn>//
- **[d35f8a581](facebook/react@d35f8a581 )**: feat: honor displayName of context types (#18224) //<Brian Vaughn>//
- **[3ee812e6b](facebook/react@3ee812e6b )**: Revert "feat: honor displayName of context types (#18035)" (#18223) //<Dominic Gannaway>//
- **[6a0efddd8](facebook/react@6a0efddd8 )**: Modern Event System: export internal FB flag for testing (#18221) //<Dominic Gannaway>//
- **[fa03206ee](facebook/react@fa03206ee )**: Remove _ctor field from Lazy components (#18217) //<Sebastian Markbåge>//
- **[2fe0fbb05](facebook/react@2fe0fbb05 )**: Use accumulateTwoPhaseDispatchesSingle directly (#18203) //<Dominic Gannaway>//
- **[503fd82b4](facebook/react@503fd82b4 )**: Modern Event System: Add support for internal FB Primer (#18210) //<Dominic Gannaway>//
- **[45c172d94](facebook/react@45c172d94 )**: feat: honor displayName of context types (#18035) //<Brian Vaughn>//
- **[ec652f4da](facebook/react@ec652f4da )**: Bugfix: Expired partial tree infinite loops (#17949) //<Andrew Clark>//
- **[d2158d6cc](facebook/react@d2158d6cc )**: Fix flow types (#18204) //<Brian Vaughn>//
- **[7e83af17c](facebook/react@7e83af17c )**: Put React.jsx and React.jsxDEV behind experimental build (#18023) //<Luna Ruan>//
- **[8cb2fb21e](facebook/react@8cb2fb21e )**: Refine isFiberSuspenseAndTimedOut (#18184) //<Dominic Gannaway>//
- **[62861bbcc](facebook/react@62861bbcc )**: More event system cleanup and scaffolding (#18179) //<Dominic Gannaway>//
- **[8ccfce460](facebook/react@8ccfce460 )**: Only use Rollup's CommonJS plugin for "react-art" (#18186) //<Sebastian Markbåge>//
- **[c26506a7d](facebook/react@c26506a7d )**: Update react-shallow-renderer from 16.12.0 to 16.13.0 (#18185) //<Minh Nguyen>//
- **[26aa1987c](facebook/react@26aa1987c )**: [Native] Enable and remove targetAsInstance feature flag. (#18182) //<Eli White>//
- **[4469700bb](facebook/react@4469700bb )**: Change ReactVersion from CJS to ES module (#18181) //<Sebastian Markbåge>//
- **[58eedbb02](facebook/react@58eedbb02 )**: Check in a forked version of object-assign only for UMD builds (#18180) //<Sebastian Markbåge>//
- **[053347e6b](facebook/react@053347e6b )**: react-test-renderer: improve findByType() error message (#17439) //<Henry Q. Dineen>//
- **[4ee592e95](facebook/react@4ee592e95 )**: Add an early invariant to debug a mystery crash (#18159) //<Dan Abramov>//
- **[7ea4e4111](facebook/react@7ea4e4111 )**: Fix typo in warning text (#18103) //<Sophie Alpert>//
- **[79a25125b](facebook/react@79a25125b )**: feat: add recommended config eslint rule (#14762) //<Simen Bekkhus>//
- **[ae60caacf](facebook/react@ae60caacf )**: [Fabric] Fix targetAsInstance dispatchEvent "cannot read property of null" (#18156) //<Joshua Gross>//
- **[d72700ff5](facebook/react@d72700ff5 )**: Remove runtime dependency on prop-types (#18127) //<Dan Abramov>//
- **[549e41883](facebook/react@549e41883 )**: Move remaining things to named exports (#18165) //<Sebastian Markbåge>//
- **[739f20bed](facebook/react@739f20bed )**: Remove Node shallow builds (#18157) //<Sebastian Markbåge>//
- **[3e809bf5d](facebook/react@3e809bf5d )**: Convert React Native builds to named exports (#18136) //<Sebastian Markbåge>//
- **[869dbda72](facebook/react@869dbda72 )**: Don't build shallow renderer for FB (#18153) //<Dan Abramov>//
- **[293878e07](facebook/react@293878e07 )**: Replace ReactShallowRenderer with a dependency (#18144) //<Minh Nguyen>//
- **[b4e314891](facebook/react@b4e314891 )**: Remove unused flag (#18132) //<Dan Abramov>//
- **[849e8328b](facebook/react@849e8328b )**: Remove unnecessary warnings (#18135) //<Dan Abramov>//
- **[f9c0a4544](facebook/react@f9c0a4544 )**: Convert the rest of react-dom and react-test-renderer to Named Exports (#18145) //<Sebastian Markbåge>//
- **[c1c5499cc](facebook/react@c1c5499cc )**: update version numbers for 16.13 (#18143) //<Sunil Pai>//
- **[e1c7e651f](facebook/react@e1c7e651f )**: Update ReactDebugHooks to handle composite hooks (#18130) //<Brian Vaughn>//
- **[d28bd2994](facebook/react@d28bd2994 )**: remove OSS testing builds (#18138) //<Sunil Pai>//
- **[8e13e770e](facebook/react@8e13e770e )**: Remove /testing entry point from 'react' package (#18137) //<Sebastian Markbåge>//
- **[60016c448](facebook/react@60016c448 )**: Export React as Named Exports instead of CommonJS (#18106) //<Sebastian Markbåge>//
- **[8d7535e54](facebook/react@8d7535e54 )**: Add nolint to FB bundle headers (#18126) //<Dominic Gannaway>//
- **[bf13d3e3c](facebook/react@bf13d3e3c )**: [eslint-plugin-react-hooks] Fix cyclic caching for loops containing a… (#16853) //<Moji Izadmehr>//
- **[501a78881](facebook/react@501a78881 )**: runAllPassiveEffectDestroysBeforeCreates's feature flag description typo fixed (#18115) //<adasq>//
- **[09348798a](facebook/react@09348798a )**: Codemod to import * as React from "react"; (#18102) //<Sebastian Markbåge>//
- **[78e816032](facebook/react@78e816032 )**: Don't warn about unmounted updates if pending passive unmount (#18096) //<Brian Vaughn>//
- **[2c4221ce8](facebook/react@2c4221ce8 )**: Change string refs in function component message (#18031) //<Sebastian Markbåge>//
- **[65bbda7f1](facebook/react@65bbda7f1 )**: Rename Chunks API to Blocks (#18086) //<Sebastian Markbåge>//
- **[8b596e00a](facebook/react@8b596e00a )**: Remove unused arguments in the reconciler (#18092) //<Dan Abramov>//
- **[5de5b6150](facebook/react@5de5b6150 )**: Bugfix: `memo` drops lower pri updates on bail out (#18091) //<Andrew Clark>//
- **[abfbae02a](facebook/react@abfbae02a )**: Update Rollup version to 1.19.4 and fix breaking changes (#15037) //<Kunuk Nykjær>//
- **[b789060dc](facebook/react@b789060dc )**: Feature Flag for React.jsx` "spreading a key to jsx" warning (#18074) //<Sunil Pai>//
- **[3f85d53ca](facebook/react@3f85d53ca )**: Further pre-requisite changes to plugin event system (#18083) //<Dominic Gannaway>//
- **[ea6ed3dbb](facebook/react@ea6ed3dbb )**: Warn for update on different component in render (#17099) //<Andrew Clark>//
- **[085d02133](facebook/react@085d02133 )**: [Native] Migrate focus/blur to call TextInputState with the host component (#18068) //<Eli White>//
- **[1000f6135](facebook/react@1000f6135 )**: Add container to event listener signature (#18075) //<Dominic Gannaway>//
- **[a12dd52a4](facebook/react@a12dd52a4 )**: Don't build some packages for WWW (#18078) //<Dan Abramov>//
- **[2512c309e](facebook/react@2512c309e )**: Remove Flare bundles from build (#18077) //<Dominic Gannaway>//
- **[4912ba31e](facebook/react@4912ba31e )**: Add modern event system flag + rename legacy plugin module (#18073) //<Dominic Gannaway>//
- **[4d9f85006](facebook/react@4d9f85006 )**: Re-throw errors thrown by the renderer at the root in the complete phase (#18029) //<Andrew Clark>//
- **[14afeb103](facebook/react@14afeb103 )**: Added missing feature flag //<Brian Vaughn>//
- **[691096c95](facebook/react@691096c95 )**: Split recent passive effects changes into 2 flags (#18030) //<Brian Vaughn>//
- **[56d8a73af](facebook/react@56d8a73af )**: [www] Disable Scheduler `timeout` w/ dynamic flag (#18069) //<Andrew Clark>//
- **[d533229fb](facebook/react@d533229fb )**: Fix Prettier //<Dan Abramov>//
- **[56a8c3532](facebook/react@56a8c3532 )**: eslint-plugin-react-hooks@2.4.0 //<Dan Abramov>//
- **[93a229bab](facebook/react@93a229bab )**: Update eslint rule exhaustive deps to use new suggestions feature (#17385) //<Will Douglas>//
- **[9def56ec0](facebook/react@9def56ec0 )**: Refactor DOM plugin system to single module (#18025) //<Dominic Gannaway>//
- **[2d6be757d](facebook/react@2d6be757d )**: [Native] Delete NativeComponent and NativeMethodsMixin (#18036) //<Eli White>//
- **[d4f2b03](facebook/react@d4f2b0379 )**: Add Auto Import to Babel Plugin  (#16626) //<Luna Ruan>//
- **[8777b44e9](facebook/react@8777b44e9 )**: Add Modern WWW build (#18028) //<Dan Abramov>//
- **[a607ea4c4](facebook/react@a607ea4c4 )**: Remove getIsHydrating (#18019) //<Dan Abramov>//
- **[f7278034d](facebook/react@f7278034d )**: Flush all passive destroy fns before calling create fns (#17947) //<Brian Vaughn>//
- **[529e58ab0](facebook/react@529e58ab0 )**: Remove legacy www config from Rollup build (#18016) //<Dominic Gannaway>//
- **[42918f40a](facebook/react@42918f40a )**: Change build from babylon to babel (#18015) //<Dominic Gannaway>//
- **[df5faddcc](facebook/react@df5faddcc )**: Refactor commitPlacement to recursively insert nodes (#17996) //<Dominic Gannaway>//
- **[517de74b0](facebook/react@517de74b0 )**: Tweak comment wording (#18007) //<Dan Abramov>//
- **[b63cb6f6c](facebook/react@b63cb6f6c )**: Update ReactFiberExpirationTime.js (#17825) //<haseeb>//
- **[89c6042df](facebook/react@89c6042df )**: fix: typo in test (#18005) //<Jesse Katsumata>//
- **[4f71f25a3](facebook/react@4f71f25a3 )**: Re-enable shorthand CSS property collision warning (#18002) //<Sophie Alpert>//
- **[c55c34e46](facebook/react@c55c34e46 )**: Move React Map child check to behind flags or __DEV__ (#17995) //<Dominic Gannaway>//
- **[3f814e758](facebook/react@3f814e758 )**: Fix Flow type for React Native (#17992) //<Dan Abramov>//
- **[256d78d11](facebook/react@256d78d11 )**: Add feature flag for removing children Map support (#17990) //<Dominic Gannaway>//
- **[9dba218d9](facebook/react@9dba218d9 )**: [Mock Scheduler] Mimic browser's advanceTime (#17967) //<Andrew Clark>//
- **[d6e08fe0a](facebook/react@d6e08fe0a )**: Remove Suspense priority warning (#17971) //<Dan Abramov>//
- **[812277dab](facebook/react@812277dab )**: Fix onMouseEnter is fired on disabled buttons (#17675) //<Alfredo Granja>//
- **[3e9251d60](facebook/react@3e9251d60 )**: make testing builds for React/ReactDOM (#17915) //<Sunil Pai>//
- **[ace9e8134](facebook/react@ace9e8134 )**: Simplify Continuous Hydration Targets (#17952) //<Sebastian Markbåge>//
- **[7df32c4c8](facebook/react@7df32c4c8 )**: Flush `useEffect` clean up functions in the passive effects phase (#17925) //<Brian Vaughn>//
- **[434770c3b](facebook/react@434770c3b )**: Add beforeRemoveInstance method to ReactNoop (#17959) //<Dominic Gannaway>//
- **[6ae2c33a7](facebook/react@6ae2c33a7 )**: StrictMode should call sCU twice in DEV (#17942) //<Brian Vaughn>//
- **[9dbe1c54d](facebook/react@9dbe1c54d )**: Revert "Bugfix: Expiring a partially completed tree (#17926)" (#17941) //<Andrew Clark>//
- **[b2382a715](facebook/react@b2382a715 )**: Add ReactDOM.unstable_renderSubtreeIntoContainer warning flag (#17936) //<Dominic Gannaway>//
- **[01974a867](facebook/react@01974a867 )**: Bugfix: Expiring a partially completed tree (#17926) //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions 241c446...b5c6dd2

Reviewed By: gaearon

Differential Revision: D20347361

fbshipit-source-id: e9e6282474ab6471585e8e7fb6ea8518aa48390d
@jayasme
Copy link

jayasme commented Mar 20, 2020

Still seen at 0.61.5 and Android SDK 25, but not seen at Android SDK 29, seems using overflow: 'visible' for the parent view could solve the problem.

@fabOnReact

This comment has been minimized.

@fabOnReact

This comment has been minimized.

@fabOnReact
Copy link
Contributor

fabOnReact commented May 5, 2020

The issue is caused from the canvas cropping on a LAYER_TYPE_NONE on Android Sdk 24-28. Adding the below code to ReactViewGroup.java fixes the issue. I am preparing a pull request.

// Disable Hardware Accelleration on Views with property overflow hidden
// for SDK 24-28 https://github.com/facebook/react-native/issues/18266
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N 
  && Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) { 
    setLayerType(View.LAYER_TYPE_HARDWARE, null);
}

before canvas.clipPath(mPath)

canvas.clipPath(mPath);
hasClipPath = true;

Result in the example from @RSNara comment before and after the fix.

The issue was caused from clipping the view that had property overflow: 'hidden' and LAYER_TYPE_NONE, in specific drawing an area (mPath) to crop the canvas from the before mentioned view using a method called addRoundRect, which would not take in consideration the view rotation previously applied with view.setRotation()

mPath.addRoundRect(
new RectF(left, top, right, bottom),
new float[] {
Math.max(topLeftBorderRadius - borderWidth.left, 0),
Math.max(topLeftBorderRadius - borderWidth.top, 0),
Math.max(topRightBorderRadius - borderWidth.right, 0),
Math.max(topRightBorderRadius - borderWidth.top, 0),
Math.max(bottomRightBorderRadius - borderWidth.right, 0),
Math.max(bottomRightBorderRadius - borderWidth.bottom, 0),
Math.max(bottomLeftBorderRadius - borderWidth.left, 0),
Math.max(bottomLeftBorderRadius - borderWidth.bottom, 0),
},

The original expo snack posted from @brunolemos does not reproduce anymore on react-native master, but the issue in the above example from @RSNara still reproduces.

The issue and solution was tested on Physical Devices and Emulators running Android Sdk 24-29, my pull request will explicitly set the layerType for Views with property overflow: 'hidden' on devices running Android 24-28 and avoid this issue.

I am preparing the pull request. Thanks a lot for reading this message 😃

@hosseinmd
Copy link

hosseinmd commented May 10, 2020

This happened in react-native 0.61.0. When set rotate and border Radius and overflow hidden to the parent, the child works fine, but when I set background color to children, parent border radius does not work.

@everfire130
Copy link
Contributor

Same shit different day😭

@bardliu
Copy link

bardliu commented Sep 27, 2020

The same problem was found on Samsung S8.

@Stevemoretz
Copy link

So what the hell shall we do? :/
It's 2021 for God's sake

@fabOnReact
Copy link
Contributor

fabOnReact commented Aug 4, 2021

You could try fixing this by using prop rendertohardwaretextureandroid with value of true

https://reactnative.dev/docs/view#rendertohardwaretextureandroid

Whether this View should render itself (and all of its children) into a single hardware texture on the GPU.

On Android, this is useful for animations and interactions that only modify opacity, rotation, translation, and/or scale: in those cases, the view doesn't have to be redrawn and display lists don't need to be re-executed. The texture can be re-used and re-composited with different parameters. The downside is that this can use up limited video memory, so this prop should be set back to false at the end of the interaction/animation.

My pr would just enable hardware accelleration on that view after applying the border radius, but you could solve this issue by enabling hardware accelleration on the view in Javascript for Android API N till P

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N
&& Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) {
setLayerType(View.LAYER_TYPE_HARDWARE, null);
}

the prop allows you to call setLayerType(HARDWARE) on each React View.

public void setRenderToHardwareTexture(@NonNull T view, boolean useHWTexture) {
view.setLayerType(useHWTexture ? View.LAYER_TYPE_HARDWARE : View.LAYER_TYPE_NONE, null);
}

#29312 (comment)

@Stevemoretz
Copy link

Stevemoretz commented Aug 4, 2021

You could try fixing this by using prop rendertohardwaretextureandroid with value of true

https://reactnative.dev/docs/view#rendertohardwaretextureandroid

Whether this View should render itself (and all of its children) into a single hardware texture on the GPU.

On Android, this is useful for animations and interactions that only modify opacity, rotation, translation, and/or scale: in those cases, the view doesn't have to be redrawn and display lists don't need to be re-executed. The texture can be re-used and re-composited with different parameters. The downside is that this can use up limited video memory, so this prop should be set back to false at the end of the interaction/animation.

My pr would just enable hardware accelleration on that view after applying the border radius, but you could solve this issue by enabling hardware accelleration on the view in Javascript for Android API N till P

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N
&& Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) {
setLayerType(View.LAYER_TYPE_HARDWARE, null);
}

the prop allows you to call setLayerType(HARDWARE) on each React View.

public void setRenderToHardwareTexture(@NonNull T view, boolean useHWTexture) {
view.setLayerType(useHWTexture ? View.LAYER_TYPE_HARDWARE : View.LAYER_TYPE_NONE, null);
}

#29312 (comment)

Hi thanks for informing us about this, Looks like your pr isn't merged yet, also would you mind talking about the limitations? Docs are a little unclear. It says turn this off before animation because you can use a limited amount of video memory, we only need it when we have the animation otherwise what's the point?

@fabOnReact
Copy link
Contributor

fabOnReact commented Aug 4, 2021

I reply to @Stevemoretz comment on my post #18266 (comment)

this prop should be set back to false at the end of the interaction/animation.

You just need to enable hardware accelleration for the duration of the animation on the View, as tested in my pr #28881 it fixes this issue.

you only apply this change for a limited number of Android APIs (from n till p)

CLICK TO OPEN TESTS RESULTS

The solution uses setLayerType to change the layer to LAYER_TYPE_HARDWARE. setLayerType specifies the type of layer backing this view.

The change is applied only to Views with overflow hidden running on sdk 24-28.

The below screenshots are from RNTester transformation examples.

The screenshot on the left displays how the clipPath method crops incorrectly using a Path shape without the transform: { rotate: "30deg" } effect. The View is cropped as if no rotation was applied. The screenshot on the right displays the result after implementing the patch.

BEFORE AFTER

The below tests performed with RNTester Animated examples do not identify any issue when running animations. Gif image quality was reduced to respect github upload limits. Thanks a lot. 😃 Fabrizio Bertoglio

@Stevemoretz
Copy link

I reply to @Stevemoretz comment on my post #18266 (comment)

this prop should be set back to false at the end of the interaction/animation.

You just need to enable hardware accelleration for the duration of the animation on the View, as tested in my pr #28881 it fixes this issue.

you only apply this change for a limited number of Android APIs (from n till p)

CLICK TO OPEN TESTS RESULTS
The solution uses setLayerType to change the layer to LAYER_TYPE_HARDWARE. setLayerType specifies the type of layer backing this view.

The change is applied only to Views with overflow hidden running on sdk 24-28.

The below screenshots are from RNTester transformation examples.

The screenshot on the left displays how the clipPath method crops incorrectly using a Path shape without the transform: { rotate: "30deg" } effect. The View is cropped as if no rotation was applied. The screenshot on the right displays the result after implementing the patch.

BEFORE AFTER

The below tests performed with RNTester Animated examples do not identify any issue when running animations. Gif image quality was reduced to respect github upload limits. Thanks a lot. 😃 Fabrizio Bertoglio

It sounds great but a couple of questions what happens for sdks below 24 and it could be great if you provided a code example, I don't exactly get how to do this use a state and change it before and after animation starts and ends?

Also when do you think your pr get merged?

@github-actions
Copy link

github-actions bot commented Mar 2, 2023

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added Stale There has been a lack of activity on this issue and it may be closed soon. and removed Stale There has been a lack of activity on this issue and it may be closed soon. labels Mar 2, 2023
@alabsi91
Copy link

alabsi91 commented Dec 13, 2023

The issue still persists in React Native 0.73. The solution is to set the renderToHardwareTextureAndroid prop to true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Platform: Android Android applications. Ran Commands One of our bots successfully processed a command. Resolution: Fixed A PR that fixes this issue has been merged.
Projects
None yet