Skip to content

Conversation

WestLangley
Copy link
Collaborator

Changed 'reverse' -> 'reversed', plus some subjective changes for clarity

@WestLangley WestLangley added this to the r180 milestone Aug 4, 2025
Copy link

github-actions bot commented Aug 4, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 338.62
79.03
338.73
79.06
+108 B
+30 B
WebGPU 566.96
156.76
566.96
156.76
+0 B
+0 B
WebGPU Nodes 565.56
156.51
565.56
156.51
+0 B
+0 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 470.05
113.73
470.15
113.76
+108 B
+31 B
WebGPU 638.44
172.77
638.44
172.77
+0 B
+0 B
WebGPU Nodes 593.09
162.03
593.09
162.03
+0 B
+0 B

@mrdoob
Copy link
Owner

mrdoob commented Aug 4, 2025

Is this easier to read?

USE_LOGARITHMIC_DEPTHBUFFER
USE_REVERSED_DEPTHBUFFER

@WestLangley
Copy link
Collaborator Author

Is this easier to read?

Maybe, but I don't think "depthbuffer" is a word.

@mrdoob
Copy link
Owner

mrdoob commented Aug 4, 2025

Up to you 👌

@WestLangley
Copy link
Collaborator Author

It's a toss-up, but I think this is OK.

@WestLangley WestLangley merged commit 224f26c into mrdoob:dev Aug 7, 2025
9 checks passed
@WestLangley WestLangley deleted the dev-nomenclature branch August 7, 2025 13:11
@WestLangley
Copy link
Collaborator Author

new build needed, please.

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 7, 2025

950dbb5 🚀

@vanruesc
Copy link
Contributor

vanruesc commented Sep 27, 2025

@Mugen87 This change broke user code and should at least be mentioned in the release notes / migration guide 🙏

Related: pmndrs/postprocessing#731

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 27, 2025

Updated: https://github.com/mrdoob/three.js/wiki/Migration-Guide#179--180

@vanruesc
Copy link
Contributor

Thank you!

@WestLangley
Copy link
Collaborator Author

We only put API changes in the Migration Guide -- not internal changes. The API change was noted in the prior release, r179.

If the user is hacking the library, he is responsible for reading what he is hacking.

@vanruesc
Copy link
Contributor

The library actively provides these macros to ShaderMaterial and users rely on this functionality. This behaviour is also (partially) documented. It's great that these flags are provided to custom materials because managing all of that manually for every custom material would be tedious and inefficient.

I don't see why relying on these predefined macros would be considered hacking.

The API change was noted in the prior release

I'd respect the decision to remove the notes from the migration guide, but I think it's very helpful information.

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 27, 2025

Yeah, the shader lib is not really internal. At least since the introduction of onBeforeCompile() the engine encourages users to overwrite/modify shader chunks to inject custom logic in built-in materials. Every time we change defines, update shader code or just rename shader chunks, user level code can break. IMO. it's correct to mentioned the renaming in the migration guide.

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.

4 participants