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

Docs: Improved PMREMGenerator page. #23514

Merged
merged 3 commits into from
Feb 23, 2022
Merged

Docs: Improved PMREMGenerator page. #23514

merged 3 commits into from
Feb 23, 2022

Conversation

mrdoob
Copy link
Owner

@mrdoob mrdoob commented Feb 17, 2022

Related issue: #23428 (comment)

Description

@elalish @WestLangley How does the explanation sound to you?

@mrdoob mrdoob added this to the r138 milestone Feb 17, 2022
resolution to smoothly interpolate diffuse lighting while limiting sampling computation.
resolution to smoothly interpolate diffuse lighting while limiting sampling computation.<br/><br/>

Note: The minimum [page:MeshStandardMaterial]'s roughness depends on the size of the provided texture:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might also note this is a slightly conservative bound. If your render has small dimensions or the shiny parts have a lot of curvature, you may still be able to get away with a smaller texture size.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added that additional info verbatim.

@@ -15,7 +15,36 @@ <h1>[name]</h1>
CubeUV format that allows us to perform custom interpolation so that we can support nonlinear formats such as RGBE.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the RGBE comment still apply?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, good catch, since it's all float now.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this page and the comments in the source code may be outdated. Can you please have a look? 😇

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@mrdoob
Copy link
Owner Author

mrdoob commented Feb 23, 2022

Merging for now...

@mrdoob mrdoob merged commit e5b7217 into dev Feb 23, 2022
@mrdoob mrdoob deleted the docs branch February 23, 2022 00:22
donmccurdy pushed a commit to donmccurdy/three.js that referenced this pull request Mar 10, 2022
* Docs: Improved PMREMGenerator page.

* Update PMREMGenerator.html

* Update PMREMGenerator.html
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.

3 participants