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

UV precision bug #5218

Merged
merged 2 commits into from Nov 14, 2018
Merged

UV precision bug #5218

merged 2 commits into from Nov 14, 2018

Conversation

ivanpopelyshev
Copy link
Collaborator

@ivanpopelyshev ivanpopelyshev commented Nov 14, 2018

We lose one binary digit in UV's and its critical

This thing exists through all the life of pixi-v4, since 6 jan 2016, it leads to wrong rendering of sprites with LINEAR scaleMode (it is default!).

The thing is, for numbers like 1/1024 we always lose 1 digit, when we actually dont have to lose anything.

1/1024 - Math.round(1 / 1024 * 65535) / 65535 === -1.4901388570992546e-8
1/1024 - Math.floor(1 / 1024 * 65535) / 65535 === 1.5244120508125375e-5

For center pixel loss is the same for both algorithms:

0.5 - Math.round(0.5 * 65535) / 65535 === -7.629510948348184e-6
0.5 - Math.floor(0.5 * 65535) / 65535 === 7.629510948348184e-6

Size of pixel 1/1024 is 1e-3 , one point of color is 2e-2 , if we multiply it we'll get 2e-5, too close to our precision loss. Experiments show that 1 binary digit in the case is critical. We have to get rid of 1.5e-5.

Lets put single pixel inside an atlas of size 1024 and render it: https://www.pixiplayground.com/#/edit/sr8e~10Ueu1aaaVlj~Rsd

We got alpha=247 and that number is even worse for size=2048. If we put the pixel to (1024,1024) instead of (1,1) we will get 251, which is still wrong!

We even lose on size=256!

You can also notice the shift in coords if we enlarge the pixel:

image

This is the bug we tried to fix with PRECISION_FRAGMENT=HIGHP. Docs say that on MEDIUMP we always have 16 binary digits which is exactly how much we use when we pack numbers in uint16.

MEDIUMP (16 bits) is enough for sprite rendering, while 15 is not.

@ivanpopelyshev ivanpopelyshev added 🕷 Bug Verified that it’s actually a legit bug that exists in the current release. Renderer: WebGL 🔥 High Priority These are PRs or issues that are extremely important to address either because they are from sponsor 💾 v4.x (Legacy) Legacy version 4 support labels Nov 14, 2018
@bigtimebuddy bigtimebuddy merged commit 8db11a4 into dev Nov 14, 2018
@bigtimebuddy bigtimebuddy deleted the dev-uv-round branch November 14, 2018 18:34
@ivanpopelyshev
Copy link
Collaborator Author

ivanpopelyshev commented Nov 15, 2018

One more detail:

https://chromium.googlesource.com/angle/angle/+/chromium/3611/src/compiler/translator/BuiltInFunctionEmulatorGLSL.cpp#121

That's part of chromium, branch chromium/3611 .

uint packUnorm2x16_emu(vec2 v)
{
    int x = int(round(clamp(v.x, 0.0, 1.0) * 65535.0));
    int y = int(round(clamp(v.y, 0.0, 1.0) * 65535.0));
    return uint((y << 16) | (x & 0xFFFF));
}

Same round function call

@ivanpopelyshev
Copy link
Collaborator Author

ivanpopelyshev commented Nov 15, 2018

Bad news. Take my pixi-playground example and try put px=894 there. It'll give you 253, and for px=600 it'll be 251.

Lets try bigger texture: size=8192. The most extreme case, px=4090 i've got 224.
size=4096, px=2040 , result is 239.

We still got slight precision loss, but for our old code i've got 195.

However, if we look into v5 that doesn't use that packing method, and even if we set highp precision for fragment shader, we will get same 224 in the most extreme case.

@lock
Copy link

lock bot commented Nov 15, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Nov 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🕷 Bug Verified that it’s actually a legit bug that exists in the current release. 🔥 High Priority These are PRs or issues that are extremely important to address either because they are from sponsor 💾 v4.x (Legacy) Legacy version 4 support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants