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

ensure bump map scale is texture UV scale invariant. adjust examples. #26899

Merged
merged 1 commit into from Oct 5, 2023

Conversation

bhouston
Copy link
Contributor

@bhouston bhouston commented Oct 4, 2023

Description

Right now our bump scale is dependent upon the texture UV scale in world space. This is incorrect, rather a bump map should look the same regardless of the texture UV scale in world space. Our normal factors are invariant to texture UV scale, it is just the bump scale that isn't.

By fixing this, this allows us to use more sensible scale factors that are close to 1.0 by default. Otherwise we had to use weird 0.01 scale factors or similar if the texture UV scale in world space wasn't close to unity.

You can see here how we made the normal factors texture UV scale invariant, it is the normalization right here using the determinant scale:

https://github.com/mrdoob/three.js/blob/dev/src/renderers/shaders/ShaderChunk/normalmap_pars_fragment.glsl.js#L36C2-L36C2

Also in Babylon.js, they are normalizing the texture UV space in this code, which they label "construct a scale-invariant frame":

https://github.com/BabylonJS/Babylon.js/blob/master/packages/dev/core/src/Shaders/ShadersInclude/bumpFragmentMainFunctions.fx#L105

This is a breaking change to anyone who is using bump map scale factors. But I believe it should increase our compatibility with other systems who do this correctly.

This contribution is funded by Threekit

@bhouston
Copy link
Contributor Author

bhouston commented Oct 4, 2023

We made this change to Exocortex/Threekit's internal Three.js 7 years ago and forgot to contribute it back to the community.

@github-actions
Copy link

github-actions bot commented Oct 4, 2023

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
649.1 kB (160.9 kB) 649.2 kB (160.9 kB) +26 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
442.4 kB (107 kB) 442.4 kB (107.1 kB) +26 B

@Mugen87 Mugen87 added this to the r158 milestone Oct 4, 2023
@mrdoob mrdoob merged commit 7b13bb5 into mrdoob:dev Oct 5, 2023
19 checks passed
@Acyapinsa
Copy link

Sorry something is not right with this now. The bump has some relation to the camera distance?
near
image
far
image

@Shane-oo
Copy link

Is it possible to know the factor on which we should increase our bump scales by?
Is it using the textures scale? I am not able to simply go and increase bump scale as they are configurable and have over 1000.
So I'm looking for a mathematical formula I can use to increase all bump map scales deterministically.
Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Dec 12, 2023

@bhouston ping?

@LR17
Copy link
Contributor

LR17 commented Dec 21, 2023

It seems that the bump is more visible when the camera is far from the object. I have this impression too.
Screenshot 2023-12-21 141044
Screenshot 2023-12-21 141101

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.

None yet

6 participants