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

Fix PMREM banding on Pixel #18326

Merged
merged 1 commit into from Jan 7, 2020
Merged

Fix PMREM banding on Pixel #18326

merged 1 commit into from Jan 7, 2020

Conversation

elalish
Copy link
Contributor

@elalish elalish commented Jan 7, 2020

Fixes #18265

The issue is that floor( x / pixelRatio * pixelRatio ) != x when pixelRatio is non-integer, due to floating point error. By adding half a pixel, we ensure floor always goes the right way.

@elalish
Copy link
Contributor Author

elalish commented Jan 7, 2020

The other option would be to change this line https://github.com/mrdoob/three.js/blob/dev/src/renderers/WebGLRenderer.js#L467 from floor to round, but I'm guessing there are reasons it is the way it is.

@sciecode
Copy link
Collaborator

sciecode commented Jan 7, 2020

Confirmed solved on my end. Thank you.

Dev Example
Branch Example - updated build*

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 7, 2020

@sciecode I'm afraid the live link does not make sense since the build files are not updated yet.

@sciecode
Copy link
Collaborator

sciecode commented Jan 7, 2020

@sciecode I'm afraid the live links do work since the build files are not updated yet.

Forgot PMREMGenerator is part of source, thank you.
Will update link with a custom branch with updated build.

Edit: done.

@Mugen87 Mugen87 added this to the r113 milestone Jan 7, 2020
@WestLangley
Copy link
Collaborator

The other option would be to change this line https://github.com/mrdoob/three.js/blob/dev/src/renderers/WebGLRenderer.js#L467 from floor to round, but I'm guessing there are reasons it is the way it is.

#16821 explains the change. I do not recall if I considered round().

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 7, 2020

Yes, the banding is now gone on my Pixel 1 👍

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 7, 2020

Merging this and updating dev for more testing.

@Mugen87 Mugen87 merged commit 98a4a7e into mrdoob:dev Jan 7, 2020
@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 7, 2020

72018fa

@elalish elalish deleted the banding branch January 7, 2020 19:49
@mrdoob
Copy link
Owner

mrdoob commented Jan 8, 2020

Thanks!

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

Successfully merging this pull request may close these issues.

New MeshStandardMaterial in r112 has banding artifacts on some GPUs
5 participants