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.

Sign up for GitHub

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

Adding metallic feature in p5.js for both IBL and non-IBL codes. #6618

Merged
merged 31 commits into from
Jan 14, 2024
Merged

Adding metallic feature in p5.js for both IBL and non-IBL codes. #6618

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
249ba2b
Adding metalness() function in p5
perminder-17 Dec 10, 2023
98d6248
fixed failing test.js
perminder-17 Dec 10, 2023
0c6b97b
docs.js
perminder-17 Dec 11, 2023
75dfefa
fixes.js
perminder-17 Dec 20, 2023
6c50096
fixes.glsl
perminder-17 Dec 20, 2023
383f7a4
fixes
perminder-17 Dec 22, 2023
deaf539
fixes
perminder-17 Dec 22, 2023
665783e
fixes
perminder-17 Dec 23, 2023
5c8b04a
fixes
perminder-17 Dec 23, 2023
1a65ffa
fixes
perminder-17 Dec 23, 2023
4d1982f
fix testFail
perminder-17 Dec 23, 2023
b2c3c71
fixes
perminder-17 Dec 23, 2023
bbf5868
fixes
perminder-17 Dec 24, 2023
324f18e
fixes
perminder-17 Dec 30, 2023
ddb69cd
fixes
perminder-17 Dec 30, 2023
5bd10ce
fixes
perminder-17 Dec 30, 2023
c09b677
fixes
perminder-17 Dec 30, 2023
c8d3011
fixes
perminder-17 Jan 3, 2024
b06b016
fixes
perminder-17 Jan 3, 2024
fba2818
fixes in shaders
perminder-17 Jan 3, 2024
dc5c4f2
working tests
perminder-17 Jan 3, 2024
0ad6575
lint fix
perminder-17 Jan 3, 2024
1e1f91d
test fixes
perminder-17 Jan 5, 2024
a2f1342
fixes .every
perminder-17 Jan 5, 2024
846d542
docs
perminder-17 Jan 5, 2024
0d4b700
indentation fix
perminder-17 Jan 5, 2024
cc8dce4
docs
perminder-17 Jan 9, 2024
c001e10
Merge branch 'processing:main' into add-metalnness
perminder-17 Jan 11, 2024
dbe3363
docs fixes
perminder-17 Jan 11, 2024
c27b9de
minor fixes
perminder-17 Jan 11, 2024
a9f5a3e
Update material.js
perminder-17 Jan 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/webgl/p5.RendererGL.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2041,6 +2041,17 @@ p5.RendererGL = class RendererGL extends p5.Renderer {
_setFillUniforms(fillShader) {
fillShader.bindShader();

if (this._useMetalness > 0 && !this.curFillColor.every((value, index) =>
value === 1)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mind explaining a bit what the scenario is that we're checking for here with the .every(...)? Is it necessary, since we don't do it for ambient colors?

const metalnessFactor = Math.min(this._useMetalness / 100, 0.6);
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you have some visual examples that lead you to arrive at 0.6 as the max metalness and 0.4 as the min non-metalness instead of letting them both go from 0 to 1?

const nonMetalnessFactor = Math.max(1 - metalnessFactor, 0.4);

this.curSpecularColor = this.curSpecularColor.map(
Copy link
Contributor

Choose a reason for hiding this comment

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

By modifying its current value, I think that means the color will be slightly different for every successive shape we render. Instead maybe we can do something like let specularColor = [...this.curSpecularColor] outside of the if statement and modify that, and pass that in to the uniform on line 2066 below? That way we aren't storing the new color, so it gets recalculated fresh for the next object.

(specularColor, index) =>
this.curFillColor[index] * metalnessFactor +
specularColor * nonMetalnessFactor
);
}
// TODO: optimize
fillShader.setUniform('uUseVertexColor', this._useVertexColor);
fillShader.setUniform('uMaterialColor', this.curFillColor);
Expand Down
33 changes: 12 additions & 21 deletions src/webgl/shaders/lighting.glsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,18 +74,14 @@ LightResult _light(vec3 viewDirection, vec3 normal, vec3 lightVector) {

//compute our diffuse & specular terms
LightResult lr;
float invertValue = (1.0 - (metallic / 100.0));
float specularIntensity = mix(0.5 , 1.0, invertValue);
float diffuseIntensity = mix(0.2, 1.0, invertValue);
if(metallic != 0.0){
if (uSpecular)
lr.specular = (_phongSpecular(lightDir, viewDirection, normal, uShininess)) * specularIntensity;
lr.diffuse = _lambertDiffuse(lightDir, normal) * diffuseIntensity;
}else{
if (uSpecular)
lr.specular = (_phongSpecular(lightDir, viewDirection, normal, uShininess)) ;
lr.diffuse = _lambertDiffuse(lightDir, normal) ;
}

float invertValue = 1.0 - (metallic / 100.0);
float specularIntensity = mix(0.4, 1.0, invertValue);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is equivalent to this:

Suggested change
float specularIntensity = mix(0.4, 1.0, invertValue);
float specularIntensity = mix(1.0, 0.4, metallic / 100.0);

Might be a bit easier to reason about without adding the extra in-between variable with the flipped value.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, it looks like these changes have a different indentation level compared to the surroinding code, we should probably make it match

float diffuseIntensity = mix(0.1, 1.0, invertValue);

if (uSpecular)
lr.specular = (_phongSpecular(lightDir, viewDirection, normal, uShininess)) * specularIntensity;
lr.diffuse = _lambertDiffuse(lightDir, normal) * diffuseIntensity;
return lr;
}

Expand Down Expand Up @@ -125,7 +121,7 @@ vec3 calculateImageDiffuse( vec3 vNormal, vec3 vViewPosition ){
// this is to make the darker sections more dark
// png and jpg usually flatten the brightness so it is to reverse that
float invertedMetallic = 1.0 - metallic / 100.0;
return mix(vec3(0.0), smoothstep(vec3(0.0), vec3(1.0), texture.xyz), invertedMetallic);
return mix(vec3(0.0), smoothstep(vec3(0.0), vec3(1.0), texture.xyz), invertedMetallic);
}

vec3 calculateImageSpecular( vec3 vNormal, vec3 vViewPosition ){
Expand All @@ -141,14 +137,9 @@ vec3 calculateImageSpecular( vec3 vNormal, vec3 vViewPosition ){
#endif
// this is to make the darker sections more dark
// png and jpg usually flatten the brightness so it is to reverse that
if(metallic != 0.0){
float invertedMetallic = 1.0 - (metallic / 100.0);
float mappedValue = mix(1.35, 8.0, (invertedMetallic));
return pow(outColor.xyz, vec3(mappedValue));
}
else{
return pow(outColor.xyz, vec3(10.0));
}
float invertedMetallic = 1.0 - (metallic / 100.0);
float mappedValue = mix(1.2, 10.0, invertedMetallic);
return pow(outColor.xyz, vec3(mappedValue));
Copy link
Contributor

Choose a reason for hiding this comment

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

I notice that as I bright the metalness slider up, in the middle, the shadows dip darker before they finally brighten:

image image image

I wonder if this is because we change the specular/diffuse balance linearly, but the power exponentially (since we linearly interpolate the exponent.) Maybe we'll avoid this dip if, instead of linearly interpolating the exponent, we linearly mix between to fixed exponents, like this?

return mix(
  pow(outColor.xyz, vec3(1.2)),
  pow(outColor.xyz, vec3(10)),
  invertedMetallic
);

}

void totalLight(
Expand Down
Loading