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

Add switch to enable/disable specular color with metalness workflow #4428

Merged
merged 12 commits into from
Jul 14, 2022

Conversation

GSterbrant
Copy link
Contributor

Description

The metalness workflow recently exposed specular color modulation for F0 reflections, however it caused some issues related to default specular color. This PR addresses this issue by adding a switch which explicitly enables specular tinting with metalness, forcing users who want this feature to first turn the flag on, and then assign specular color values.

Relates to #4423 #4386 and #4419.

@GSterbrant GSterbrant self-assigned this Jul 13, 2022
@GSterbrant GSterbrant changed the title Add switch to enable/disable specular color with metalness workflow. Add switch to enable/disable specular color with metalness workflow Jul 13, 2022
I added tint as to not confuse it with enabling/disabling specular with metalness workflow.
@GSterbrant
Copy link
Contributor Author

Decided to call it useMetalnessSpecularTint as to not make it sound like it's used to enable/disable specular effects with metalness.

Co-authored-by: Donovan Hutchence <slimbuck7@gmail.com>
GSterbrant and others added 3 commits July 13, 2022 15:34
…om:playcanvas/engine into gsterbrant_metalness_specular_color_switch
Co-authored-by: Donovan Hutchence <slimbuck7@gmail.com>
Copy link
Contributor

@mvaligursky mvaligursky left a comment

Choose a reason for hiding this comment

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

Looks great, should work well.
Please wait for @slimbuck 's review as well.

Because we have specular maps which will be used if assigned, we need some way to turn off the specular colours on the shader level instead of just controlling the tint.
Changed name of the useMetalnessSpecularTint to useMetalnessSpecularColor.
Copy link
Member

@slimbuck slimbuck left a comment

Choose a reason for hiding this comment

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

approving with some suggestions

src/scene/materials/standard-material.js Outdated Show resolved Hide resolved
src/scene/materials/standard-material.js Outdated Show resolved Hide resolved
@GSterbrant GSterbrant merged commit 86c8591 into main Jul 14, 2022
@GSterbrant GSterbrant deleted the gsterbrant_metalness_specular_color_switch branch July 14, 2022 09:17
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

3 participants