Skip to content

Fix 1-pixel height horizontal line disappear#30481

Open
laotian wants to merge 3 commits into
react:mainfrom
laotian:master
Open

Fix 1-pixel height horizontal line disappear#30481
laotian wants to merge 3 commits into
react:mainfrom
laotian:master

Conversation

@laotian

@laotian laotian commented Nov 26, 2020

Copy link
Copy Markdown

Summary

On Android devices with a non integer DPI (for example, 2.75), View layoutX/Y may be decimal (for example, 3.25). Using integer type in recursive calculation may cause 1 pixel margin or 1 pixel overlap between adjacent views. The dividing line of 1 pixel (StyleSheet.hairlineWidth) may disappear.

Fixes #22927

Changelog

[Android] [Fixed] - Fix 1-pixel height horizontal line disappear.

Test Plan

manual test
first repeats the problem according to the demo code, and verifies after repair

reproduce steps

  • APP don't use FabricUIManager
  • Dividing line, width/height set to 1 pixel(StyleSheet.hairlineWidth)
  • Adjacent view opaque
  • Android devices with a non integer DPI (for example, 2.75)

demo code

import React,{Component} from 'react';
import {StyleSheet, View, Text} from 'react-native';

export default class App extends Component{
  // some line disappear on decimal dpi Android device
  render() {
    return (
        <View>
          {['1','2','3','4','5','6'].map((row, index) => (
              <View>
                <View style={{height: 54, flexDirection: 'row', alignItems: 'center', backgroundColor: '#ffffff' }}>
                    {['1','2','3','4','5','6'].map(column=>{
                        return(
                            <View style={{flexDirection: 'row'}}>
                                <View style={{width: 61, backgroundColor: '#ffffff', justifyContent: 'center', alignItems: 'center'}}>
                                    <Text>{`R${row}C${column}`}</Text>
                                </View>
                                <View style={{width: StyleSheet.hairlineWidth,backgroundColor: '#ff0000' }} />
                            </View>)
                    })}
                </View>
                <View style={{height: StyleSheet.hairlineWidth,backgroundColor: '#ff0000'}} />
              </View>
          ))}
        </View>);
  }
};

Screenshot

Before Fix

device : Android MI8SE (dpi 2.75)

disappear

Fixed

fixed

Without FabricUIManager, On Android devices with a non integer DPI (for example, 2.75), View layoutX/Y may be  decimal (for example, 3.25). Using integer type in recursive calculation may cause 1 pixel margin or 1 pixel overlap between adjacent views. The horizontal dividing line of 1 pixel (height: StyleSheet.hairlineWidth) may disappear.
@facebook-github-bot

Copy link
Copy Markdown
Contributor

Hi @laotian!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot

Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 26, 2020
@react-native-bot react-native-bot added Bug Platform: Android Android applications. labels Nov 26, 2020
@facebook-github-bot

Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Comment on lines 342 to +346
// We use screenX/screenY (which round to integer pixels) at each node in the hierarchy to
// emulate what the layout would look like if it were actually built with native views which
// have to have integral top/left/bottom/right values
int x = node.getScreenX();
int y = node.getScreenY();
float x = node.getLayoutX();
float y = node.getLayoutY();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should update the comment as well

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have update the comment, thank you

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ecreeth I have update the comment and I submitted in the original branch. Do I need to launch a new PR?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ecreeth Can you review my commit again?

@ecreeth

ecreeth commented Nov 28, 2020

Copy link
Copy Markdown
Contributor

cc @safaiyeh

@laotian

laotian commented Nov 30, 2020

Copy link
Copy Markdown
Author

@safaiyeh Can you review my PR?

@kelset
kelset requested a review from mdvacca November 30, 2020 09:40
@kelset

kelset commented Nov 30, 2020

Copy link
Copy Markdown
Contributor

Based on file history, looks like @mdvacca is the most relevant FB dev to check/review this PR out.

cc @dulmandakh

@mdvacca

mdvacca commented Dec 1, 2020

Copy link
Copy Markdown
Contributor

Thanks for working on this @laotian, I'm importing to review internally.

@facebook-github-bot facebook-github-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdvacca has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@github-actions

Copy link
Copy Markdown

This PR 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 the Stale There has been a lack of activity on this issue and it may be closed soon. label Sep 21, 2023
@github-actions

Copy link
Copy Markdown

This PR was closed because it has been stalled for 7 days with no activity.

@github-actions github-actions Bot closed this Sep 28, 2023
@ddikodroid

Copy link
Copy Markdown
Contributor

Thanks for working on this @laotian, I'm importing to review internally.

any update on this pull request?

@jamonholmgren jamonholmgren reopened this Oct 9, 2024
@react-native-bot

Copy link
Copy Markdown
Collaborator
Fails
🚫

📋 Missing Changelog - Please add a Changelog to your PR description. See Changelog format

Generated by 🚫 dangerJS against 6289a2e

@react-native-bot react-native-bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Oct 10, 2024
@ddikodroid

Copy link
Copy Markdown
Contributor

@kelset could you take a look at this pr?

@react-native-bot

Copy link
Copy Markdown
Collaborator

This PR 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.

@react-native-bot react-native-bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Aug 13, 2025
@diegolmello

Copy link
Copy Markdown

This bug still exists

@react-native-bot react-native-bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Aug 14, 2025
@react-native-bot

Copy link
Copy Markdown
Collaborator

This PR 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.

@react-native-bot react-native-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 Feb 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Platform: Android Android applications.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

StyleSheet.hairlineWidth not visible on devices with decimal PixelRatio

10 participants