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.

Sign up for GitHub

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

Jump to bottom

[0.60.0] Regression in border radius rendering #25591

Closed
mjmasn opened this issue Jul 11, 2019 · 2 comments
Closed

[0.60.0] Regression in border radius rendering #25591

mjmasn opened this issue Jul 11, 2019 · 2 comments
Labels
Bug Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: Linux Building on Linux. Resolution: Locked This issue was locked by the bot.

Comments

@mjmasn
Copy link
Contributor

mjmasn commented Jul 11, 2019 •
edited
Loading

Edit: This is on Android, I haven't tested iOS.

Border radius rendering is broken in 0.60.0 when the border radius is less than or equal to the border width.

See screenshots:

0.59.10 0.60.0
Screenshot_20190711-113401_BorderRadius59 Screenshot_20190711-113316_BorderRadius60

React Native version:

System:
    OS: Linux 4.15 Ubuntu 16.04.6 LTS (Xenial Xerus)
    CPU: (8) x64 Intel(R) Core(TM) i7-7700 CPU @ 3.60GHz
    Memory: 674.33 MB / 15.54 GB
    Shell: 4.3.48 - /bin/bash
  Binaries:
    Node: 8.11.3 - ~/.nvm/versions/node/v8.11.3/bin/node
    Yarn: 1.16.0 - /usr/bin/yarn
    npm: 5.6.0 - ~/.nvm/versions/node/v8.11.3/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  SDKs:
    Android SDK:
      API Levels: 22, 24, 26, 27, 28, 29
      Build Tools: 23.0.1, 26.0.1, 26.0.2, 26.0.3, 27.0.0, 27.0.3, 28.0.2, 28.0.3, 29.0.0
      System Images: android-22 | Google APIs Intel x86 Atom_64, android-24 | Google APIs ARM 64 v8a, android-24 | Google APIs Intel x86 Atom, android-24 | Google APIs Intel x86 Atom_64, android-24 | Google Play Intel x86 Atom, android-25 | Google Play Intel x86 Atom, android-26 | Google APIs Intel x86 Atom, android-27 | Google APIs Intel x86 Atom, android-29 | Google Play Intel x86 Atom, android-29 | Google Play Intel x86 Atom_64
      Android NDK: 17.1.4828580
  IDEs:
    Android Studio: 3.4 AI-183.6156.11.34.5522156
  npmPackages:
    react: 16.8.6 => 16.8.6 
    react-native: 0.60.0 => 0.60.0 
  npmGlobalPackages:
    create-react-native-app: 2.0.2
    react-native-cli: 2.0.1
    react-native-create-library: 3.1.2
    react-native-git-upgrade: 0.2.7

Steps To Reproduce

  1. Create a react-native 0.60.0 app
  2. Create a view with a border width of x and a border radius <= x
  3. Rounded corners are not rendered 😭

Describe what you expected to happen:
Rendering to be the same as in 0.59.10, where rounded corners are rendered correctly.

Snack, code example, or link to a repository:
https://github.com/mjmasn/BorderRadius59 (working)
https://github.com/mjmasn/BorderRadius60 (regression)
@mjmasn mjmasn added the Bug label Jul 11, 2019
@react-native-bot react-native-bot added Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: Linux Building on Linux. labels Jul 11, 2019
@JonnyBurger
Copy link
Contributor

@ecreeth Expo SDK33 is still on React Native 0.59.0.

@cabelitos cabelitos mentioned this issue Jul 13, 2019
Closed
@cabelitos
Copy link
Contributor

Hello @JonnyBurger and @mjmasn I submitted a fix for this problem. Comments and suggestions are appreciated.

#25626

thank you very much.

grabbou pushed a commit that referenced this issue Aug 7, 2019
@cabelitos @grabbou
Properly set borderRadius on Android (#25626)
feb931f 
Summary:
In order to properly set the view's borderRadius the inner*RadiusX/inner*RadiusY should not
be used, since their values are obtained via the following formula:

int innerTopLeftRadiusX = Math.max(topLeftRadius - borderWidth.left, 0);

If topLeftRadius and borderWidth.left have the same value innerTopLeftRadiusX will
be zero and it will cause the top left radius to never be set since
"(innerTopLeftRadiusX > 0 ? extraRadiusForOutline : 0)" will evaluate to zero.
In order to prevent this issue the condition will only consider topLeftRadius, topRightRadius, and etc.

I took a closer look to see if this fix does not causes a regression in #22511

[Android] [FIX] - Correctly set the border radius on android
Pull Request resolved: #25626

Test Plan:
Using the following code and Android certify that the border radius is correctly applied.
```javascript
import React from "react";
import { ScrollView, StyleSheet, Text, View } from "react-native";

export default class App extends React.Component<Props> {
  render() {
    return (
      <ScrollView style={styles.container}>
        <View style={styles.box1}>
          <Text>borderWidth: 2</Text>
          <Text>borderRadius: 2</Text>
        </View>
        <View style={styles.box2}>
          <Text>borderWidth: 2</Text>
          <Text>borderRadius: 2.000001</Text>
        </View>
        <View style={styles.box3}>
          <Text>borderWidth: 5</Text>
          <Text>borderRadius: 5</Text>
        </View>
        <View style={styles.box4}>
          <Text>borderWidth: 5</Text>
          <Text>borderRadius: 5.000001</Text>
        </View>
        <View style={styles.box5}>
          <Text>borderWidth: 10</Text>
          <Text>borderRadius: 10</Text>
        </View>
        <View style={styles.box6}>
          <Text>borderWidth: 10</Text>
          <Text>borderRadius: 11</Text>
        </View>
        <View style={styles.box7}>
          <Text>borderWidth: 10</Text>
          <Text>borderRadius: 5</Text>
        </View>
        <Text>Testing if this does not cause a regression in https://github.com/facebook/react-native/issues/22511</Text>
        <View style={{
          margin: 10,
          backgroundColor: 'red',
          width: 100,
          height: 100,
          borderWidth: 5,
          borderBottomLeftRadius: 15,
        }} />
        <View style={{
          margin: 10,
          backgroundColor: 'green',
          borderWidth: 5,
          width: 100,
          height: 100,
        }} />
      </ScrollView>
    );
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1
  },
  box1: {
    margin: 10,
    height: 100,
    borderRadius: 2,
    borderWidth: 2,
    borderColor: "#000000"
  },
  box2: {
    margin: 10,
    height: 100,
    borderRadius: 2.000001,
    borderWidth: 2,
    borderColor: "#000000"
  },
  box3: {
    margin: 10,
    height: 100,
    borderRadius: 5,
    borderWidth: 5,
    borderColor: "#000000"
  },
  box4: {
    margin: 10,
    height: 100,
    borderRadius: 5.000001,
    borderWidth: 5,
    borderColor: "#000000"
  },
  box5: {
    margin: 10,
    height: 100,
    borderRadius: 10,
    borderWidth: 10,
    borderColor: "#000000"
  },
  box6: {
    margin: 10,
    height: 100,
    borderRadius: 11,
    borderWidth: 10,
    borderColor: "#000000"
  },
  box7: {
    margin: 10,
    height: 100,
    borderRadius: 5,
    borderWidth: 10,
    borderColor: "#000000"
  }
});

```

![not-working](https://user-images.githubusercontent.com/984610/61164801-e1dc6580-a4ee-11e9-91f3-6ca2fab7c461.gif)

![fixed](https://user-images.githubusercontent.com/984610/61164794-db4dee00-a4ee-11e9-88d7-367c29c785ec.gif)

Closes #25591

Reviewed By: osdnk

Differential Revision: D16243602

Pulled By: mdvacca

fbshipit-source-id: 1e16572fdf6936aa19c3d4c01ff6434028652942
@facebook facebook locked as resolved and limited conversation to collaborators Oct 9, 2021
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Bug Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: Linux Building on Linux. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Android][Fix]Properly set borderRadius on Android cabelitos/react-native
4 participants
@cabelitos @JonnyBurger @mjmasn @react-native-bot