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

In-pass shadows (WIP) #723

Open
wants to merge 56 commits into
base: master
from

Conversation

@shadowislord
Copy link
Member

commented Sep 9, 2017

Teapot lit by spot lights

This is the PR implementing in-pass shadows as shown in the April 2016 screenshot thread:
https://hub.jmonkeyengine.org/t/april-2016-monthly-wip-screenshot-thread/35512/128

How it works:
PreShadowArrayRenderer is responsible for generating a texture array containing shadow maps for every light that should be shadowed. Directional lights use PSSM with 1-4 slices, spot lights use 1 slice, and point lights will use 6 slices (not implemented yet). The information of which texture array to use and the slices is represented in the ShadowMap interface. This is then associated with the light so that later it can be retrieved.
When the scene is rendered, ShadowStaticPassLightingLogic is the TechniqueDefLogic implementation that is capable of rendering in-pass shadows. The light types are statically known which allows the shader to avoid control flow. It works similarly to SinglePass, except that the lights are first sorted by type and then their counts are known statically, so instead of this loop:

for each light do ...

It becomes

for each directional light do ...
for each point light do ...
for each spot light do ...

This then allows using different logic depending on the light type. The way shadow maps are fetched varies depending on the type of light, which is why this is required.

Theoretically, if we have control flow in the shader then we don't need to know the light type in advance and we can calculate the slice to fetch per each light. This is probably how in-pass shadows will be integrated with the rest of the lighting shaders.

TODO:

  • Point light support
  • Remove / fix the hacks from ShadowUtil
  • Remove all the static lighting crap

Optional:

  • Port the old shadow renderers to the new system
  • Implement and test PSSM-only in-pass shadows
  • Test on OpenGL ES 2.0

@shadowislord shadowislord changed the base branch from master to opengles2-fixes Sep 9, 2017

@Nehon
Copy link
Contributor

left a comment

Several remarks:
1- Don't you have a Test class to see the usage and well... test it.
2- It's seems that inpass shadows use a different technique and techniqueDefLogic so how could I combine them with the PBR shader?
3- It seems there is 1 shadow renderer by light type is that correct?
4- Could be nice that you explain the whole principle in a short summary.

@shadowislord

This comment has been minimized.

Copy link
Member Author

commented Sep 10, 2017

  1. Yes I have it... It kinda crappy though so I am going to clean it a bit and commit later
  2. So the challenge is supporting any light type in the shader without knowing its type statically. Unfortunately this is more complicated so I took the easier route initially where all types are known ahead of time. Theoretically the shader is faster but due to the large number of shader permutations it causes micro-stuttering when the shader variants are compiled. Supporting arbitrary light types requires several passes where first the shadow slices to fetch are determined (based on NB_LIGHTS) and then fetching them. The next part is using those results in the lighting loop.
  3. Not sure what you mean by renderer. Generating the shadow maps is handled by PreShadowArrayRenderer and then later they are used by ShadowStaticPassLightingLogic.
  4. I will update the description ...
@@ -817,7 +819,14 @@ private int updateShaderMaterialParameters(Renderer renderer, Shader shader,

private void updateRenderState(RenderManager renderManager, Renderer renderer, TechniqueDef techniqueDef) {
if (renderManager.getForcedRenderState() != null) {
renderer.applyRenderState(renderManager.getForcedRenderState());
if (techniqueDef.getForcedRenderState() != null) {

This comment has been minimized.

Copy link
@shadowislord

shadowislord Mar 19, 2018

Author Member

This could break compatibility... I think it fixes a bug, or changes behavior


protected float encodeLightType(Light light) {
switch (light.getType()) {
case Directional:

This comment has been minimized.

Copy link
@shadowislord

shadowislord Mar 19, 2018

Author Member

FYI, this breaks any custom lighting shaders

import com.jme3.texture.TextureArray;
import java.util.EnumSet;

public class ShadowStaticPassLightingLogic extends StaticPassLightingLogic {

This comment has been minimized.

Copy link
@shadowislord

shadowislord Mar 19, 2018

Author Member

Static pass should probably be removed. It doesn't add any value now

return super.makeCurrent(assetManager, renderManager, rendererCaps, lights, defines);
ambientLightColor.a = 1.0f;

filteredLightList.sort(new Comparator<Light>() {

This comment has been minimized.

Copy link
@shadowislord

shadowislord Mar 19, 2018

Author Member

Should refactor this comparator to avoid allocation

@@ -57,23 +57,23 @@
*
* @author Kirill Vainer
*/
public final class StaticPassLightingLogic extends DefaultTechniqueDefLogic {
public class StaticPassLightingLogic extends DefaultTechniqueDefLogic {

This comment has been minimized.

Copy link
@shadowislord

shadowislord Mar 19, 2018

Author Member

Remove this

*
* @author Kirill Vainer
*/
public class PreShadowRenderer implements SceneProcessor {

This comment has been minimized.

Copy link
@shadowislord

shadowislord Mar 19, 2018

Author Member

This needs a test... TestInPassShadows uses the regular PreShadowArrayRenderer

*
* @author Kirill Vainer
*/
final class ShadowDebugControl extends AbstractControl {

This comment has been minimized.

Copy link
@shadowislord

shadowislord Mar 19, 2018

Author Member

This needs to be used by the test

/**
* @author Kirill Vainer
*/
public interface ShadowParameters {

This comment has been minimized.

Copy link
@shadowislord

shadowislord Mar 19, 2018

Author Member

What's the point of this?

This comment has been minimized.

Copy link
@Nehon

Nehon May 27, 2018

Contributor

DirectionalShadowParameters implement it... not sure why it's needed though.
There is no other type of Shadow parameter and directional parameters are used with the type DirectionalShadowParameters So this interface is IMO useless. I'll remove it if you agree.

super(array, layer, textureSize, true);
}

private static boolean isParallelToYUp(Vector3f direction) {

This comment has been minimized.

Copy link
@shadowislord

shadowislord Mar 19, 2018

Author Member

Should this be used for directional lights too?

This comment has been minimized.

Copy link
@Nehon

Nehon May 27, 2018

Contributor

yes, done.

@Override
public Matrix4f getBiasedViewProjectionMatrix() {
// return shadowCamera.getViewProjectionMatrix();
throw new UnsupportedOperationException();

This comment has been minimized.

Copy link
@shadowislord

shadowislord Mar 19, 2018

Author Member

???

}

public void setNumSplits(int numSplits) {
// TODO: ensure it is 1 to 4

This comment has been minimized.

Copy link
@shadowislord

shadowislord Mar 19, 2018

Author Member

Add a check here

vec3 tempVec = position.xyz * sign(posLight - 0.5) - (worldPos * posLight);
lightVec = tempVec;
float dist = length(tempVec);
#ifdef SRGB
lightDir.w = (1.0 - position.w * dist) / (1.0 + position.w * dist * dist);
lightDir.w = clamp(lightDir.w, 1.0 - posLight, 1.0);
#else
lightDir.w = clamp(1.0 - position.w * dist * posLight, 0.0, 1.0);
lightDir.w = 1.0; // clamp(1.0 - position.w * dist * posLight, 0.0, 1.0);

This comment has been minimized.

Copy link
@shadowislord

shadowislord Mar 19, 2018

Author Member

revert this

@shadowislord shadowislord requested a review from Nehon Mar 19, 2018

Nehon added 3 commits May 26, 2018
Changes the name of the array shadow renderer to InPassShadowRenderer…
…. Also addressed some comments on the PR
@empirephoenix

This comment has been minimized.

Copy link
Contributor

commented Feb 24, 2019

What is the state of this one btw? is this still alive?

@shadowislord

This comment has been minimized.

Copy link
Member Author

commented Mar 26, 2019

@empirephoenix
It breaks backwards compatibility, mainly with custom lighting shaders. Also there are some significant core changes that require user testing. Might make sense to include in jME 3.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.