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

Per-material shadow bias #19094

Closed
makc opened this issue Apr 9, 2020 · 9 comments
Closed

Per-material shadow bias #19094

makc opened this issue Apr 9, 2020 · 9 comments

Comments

@makc
Copy link
Contributor

makc commented Apr 9, 2020

It currently sits in the light, but as it happens the same value of bias would fix one object in the scene while causing problems for another. How hard would it be to add an optional bias to the material, and will this work for e g #12919

Looking at the code, it is just passed as the uniform parameter, so altering it should not be a problem?
Screen Shot 2020-04-09 at 19 54 39

Ok no, looks like those are some different uniforms, for shadow pass or something. It is not on material uniforms in onBeforeCompile :(

It is actually in param.uniforms.directionalLights.value[0]

Ok, so I was able to do this hack:

material.onBeforeCompile = function(stuff) {
	var chunk = THREE.ShaderChunk.shadowmap_pars_fragment
		.split ('z += shadowBias')
		.join ('z += shadowBias - 0.001');
	stuff.fragmentShader = stuff.fragmentShader
		.split ('#include <shadowmap_pars_fragment>')
		.join (chunk);
};

so this is definitely possible.

@gkjohnson
Copy link
Collaborator

#18915 should hopefully lead to some improvements in this area without having to tweak individual materials. I think ideally the user wouldn't have to modify shadow properties per material.

@makc
Copy link
Contributor Author

makc commented Apr 9, 2020

ideally the user wouldn't have to modify shadow properties per material

ideally I should not set the bias either - I should just enable shadows and have nice shadows. this ticket, however, was born out of the situation that was far from ideal.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 22, 2020

ideally I should not set the bias either

Even engines like Unity can't provide perfect shadows automatically. If you closely study how shadow mapping works, it should be clear that manual adjustments are inevitable.

Regarding the feature request itself: I do not vote for supporting shadow properties on material level. Unity also provides shadow bias and shadow normal bias only on lights level. That should be sufficient.

@makc
Copy link
Contributor Author

makc commented Jun 22, 2020 via email

@makc
Copy link
Contributor Author

makc commented Jun 22, 2020

...that said, I do not insist on this feature, as long as the onBeforeCompile is there we can hack any feature we want in. If you do feel like this is too much work for making people lives only so much easier, feel free to close the ticket.

@mrdoob
Copy link
Owner

mrdoob commented Jun 24, 2020

@makc Have you tried the new normalBias?

@makc
Copy link
Contributor Author

makc commented Jul 8, 2020

@mrdoob nope, is there an example?

@mrdoob
Copy link
Owner

mrdoob commented Jul 27, 2020

I don't think so. It's a new property in the shadow.

You can try it in the editor:

shadow.normalBias = 0:

Screen Shot 2020-07-27 at 6 40 47 PM

shadow.normalBias = 0.05:

Screen Shot 2020-07-27 at 6 41 11 PM

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 4, 2021

Instead of moving shadow properties to materials, it's better to focus on #13108. Meaning investigating and introducing new shadow approaches for tweaking shadows (like normalBias).

@Mugen87 Mugen87 closed this as completed Mar 4, 2021
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

No branches or pull requests

4 participants