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

1 pixel line on iOS when using offset #36

Closed
walterholohan opened this issue Feb 8, 2022 · 7 comments
Closed

1 pixel line on iOS when using offset #36

walterholohan opened this issue Feb 8, 2022 · 7 comments

Comments

@walterholohan
Copy link

walterholohan commented Feb 8, 2022

Reopening this issue as I'm experiencing a 1-pixel line on the shadow when using offset. See below code snippet and screenshot.

<Shadow
				distance={SPACING.s12}
				viewStyle={{ alignSelf: 'stretch' }}
				radius={SPACING.s16}
				startColor={COLORS.primary}
				finalColor={COLORS.primary}
				offset={[5, 5]}
				safeRender
			>

image

Package Version:

"react-native": "0.66.4",
"react-native-svg": "^12.1.1",
"react-native-shadow-2": "^6.0.2",
@ftzi
Copy link
Owner

ftzi commented Feb 8, 2022

Thanks!

If you change the offset to offset={[-5, -5]}, does it also happens for the top and left sides?

image

Looks like it's a paintInside issue. The corners and sides contacts seems to be ok.

I think I found the cause: In the line 432 (in TS source), https://github.com/SrBrahma/react-native-shadow-2/blob/aaf5dee9f050ca40a98b7f471c3201410c20417a/src/index.tsx#L432, there is "width={width} height={height}".

In the transpiled .js file, it's on the line 240. You can give it a try and edit the node_modules/react-native-shadow-2/lib/index.js and replace what I quoted with width={widthWithAdditional} height={heightWithAdditional}. I think it will work, as by having a smaller width and height, the SVG gets shortened, specially on high dp screens, as your iPhone must be. What is your iPhone model?

If it fixes it, I will release it asap.

Edit: Hmmm... actually, it shouldn't be the cause of this issue. The wrapping svg already has the withAdditional. Maybe changing it would cause overlaps on smaller dps. Anyway, give it a try.

@ftzi
Copy link
Owner

ftzi commented Feb 9, 2022

Also, please, make a minimal reproducible example in Expo Snack (iOS), so if I can get this issue here I won't have to make you try different stuff.

@walterholohan
Copy link
Author

@SrBrahma thanks for the quick reply, I have tried editing line 240 above but that did not fix the issue.

Below is a screenshot of offset={[-5, -5]}, you can see that they are some 1 pixel lines visible
image

@walterholohan
Copy link
Author

Also, the issue is visible on your snack, just change the shadow color to a darker color - https://snack.expo.dev/@srbrahma/react-native-shadow-2-sandbox

I am using an iPhone Xs

@ftzi
Copy link
Owner

ftzi commented Feb 11, 2022

Hi, @walterholohan.

Big text ahead just for future references:

I managed to get the issue on my iPhone 8. As it doesn't have a high dp and has the same issue, this seems it's an issue caused by how react-native-svg handles masks on iOS, a svg tag that paintInside uses. It doesn't seem to be predictable on iOS. Sometimes an x/y positioning of like +1 would fix it, sometimes not. Changing the size or positioning would change its thickness somewhat randomly. In your image, you can see that some lines are thicker than others:

image

I've managed to get a solution: on 2nd render and beyond (or when size property is defined), we use a simple <Path/> SVG component to exactly paint the internal of the shadow.

<Path fill={startColor} d={`M0,${topLeft} v${height - bottomLeft - topLeft} h${bottomLeft} v${bottomLeft} h${width - bottomLeft - bottomRight} v${-bottomRight} h${bottomRight} v${-height + bottomRight + topRight} h${-topRight} v${-topRight} h${-width + topLeft + topRight} v${topLeft} Z`}/>

I can't do this on the first render as the Path doesn't accept percentage values, only numbers.

We could do this on the first render if we smartly placed rectangles inside the hole, but it's a quite hard task, as each corner may have different radii. You can see this if you imagine a square and at each corner you remove a square of a random size. What algorithm would you use to always fill the entire hole without overlaps, as overlaps would mess the internal color, as it may have transparency? Also, doing this wouldn't give a guarantee that it wouldn't also give gaps.

On the first render, the current mask solution is still used, so the gaps will briefly show up. But, on a 60fps screen, 1px line and for something that just appeared, it isn't noticeable for common eyes.

tl;dr, Released 6.0.3 via 2f3001f!

Please, tell me if it worked for you!

@ftzi ftzi closed this as completed Feb 11, 2022
@ftzi
Copy link
Owner

ftzi commented Feb 11, 2022

If you think that those lines are noticeable on the first render, I could really use a hand on that rectangles algorithm. I am really out of time to do it right now.

@ftzi
Copy link
Owner

ftzi commented Mar 7, 2022

This was recently merged on react-native-svg on 12.3.0: software-mansion/react-native-svg#1658

As it both mentions iOS mask and pixel-density, it probably fixes this issue on the first render, working nicely as it does on Android.

I will it later try without my fix and with this new svg version.

If it fixes this issue, I think I will leave my fix anyway as it's simpler and certainly improves the paintInside performance and memory usage on 2nd render and beyond.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants