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 IBL specular persisting when specular disabled #33694

Closed

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Nov 18, 2019

Fixes: #33616

SPECULAR_DISABLED wasn't affecting IBL specular. So in PBR scenes, there were still specular highlights even when specular was disabled.

@clayjohn clayjohn added this to the 3.2 milestone Nov 18, 2019
@clayjohn clayjohn requested a review from reduz as a code owner November 18, 2019 07:02
@reduz
Copy link
Member

reduz commented Nov 19, 2019

Not sure about this, it seems to me that the problem in the original issue is something else and it should be properly researched.

@clayjohn
Copy link
Member Author

From what I understand, the problem the user is having is that they are seeing specular highlights even when specular is disabled.

They also think that setting "specular" to 0 should reduce the effect of specular highlights. But they are misunderstanding what the specular parameter does. fracteed does a nice job of explaining what the slider is for.

From a design perspective. We are giving the users the ability to disable specular, but it only disables specular from lights, not from IBL. The user is trying to disable all specular but is seeing no change when in an IBL-only scene. They also mistake fully-rough specular for specular being turned off.

@clayjohn clayjohn modified the milestones: 3.2, 4.0 Jan 16, 2020
@clayjohn
Copy link
Member Author

Reduz and I discussed this over IRC. We concluded the best option would be to clarify that the current Specular Mode settings is actually a "Specular Lobe Mode" and to add a further flag that disables specular reflections.

It makes sense to treat the specular lobe differently from specular reflections. Once the Vulkan branch has been merged into master I will update this PR to reflect the above decision.

@aaronfranke
Copy link
Member

@clayjohn Vulkan has been merged into master, so you can update this PR now.

@akien-mga
Copy link
Member

Superseded by #41415?

@clayjohn
Copy link
Member Author

@akien-mga not quite. I will edit this to expose a disable-specular property to StandardMaterial3D

@YuriSizov YuriSizov requested a review from a team September 1, 2021 20:32
@clayjohn
Copy link
Member Author

Superseded by #63587

@clayjohn clayjohn closed this Aug 11, 2022
@clayjohn clayjohn removed this from the 4.0 milestone Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Roughness Texture breaks Specular
5 participants