Announcement

Collapse
No announcement yet.

Dynamic shadows artifacts

Collapse
X
 
  • Filter
  • Time
  • Show
Clear All
new posts

    Originally posted by NilsonLima View Post

    I think the intention is to downscale the CotanOuterCone to a radius form instead of diameter (CotanOuterCone == diameter right?)
    Lets do some math.

    DeferredLightUniforms.SpotAngles.x is cos(OuterCone).
    So equation is:
    cos(OuterCone) / sqrt(1.0 - cos(OuterCone)^2) which simplifies to cot(x) when x is positive. Cot(x) is 1 / Tan(X) which is standard perspective correction scale term. I don't see any reason why 0.5 is there.

    Comment


      Hmm... I see now... Which issue that would fix? I mean, there is a reason for you to reach that part of the code, so the correct answer maybe is behind the question: Why are you there?

      Also, as a side comment, sometimes people just multiply stuff for 0.5 instead of dividing by 2. Modern compilers usually will do the best choice regardless the intention, not sure how the shader compiler is handling such thing thou. Anyways, this might lead to a copied and pasted code with an error inherited or there is an reason behind this which only a comment in code would dismiss (not this case unfortunetelly).

      You finding is solid, as for the repercussion changing it, I don't know. The usage ahead would tell I guess.
      Last edited by NilsonLima; 03-01-2018, 12:32 PM.
      Nilson Lima
      Technical Director @ Rigel Studios Ltda - twitter: @RigelStudios
      Join us at Discord: https://discord.gg/FUwTvzr

      UE4 Marketplace: Cloudscape Seasons
      supporting: Community FREE Ocean plugin

      Comment


        Originally posted by Kalle_H View Post
        Code:
        #if SPOT_LIGHT_PCSS
        {
        float CotanOuterCone = DeferredLightUniforms.SpotAngles.x * rsqrt(1. - DeferredLightUniforms.SpotAngles.x * DeferredLightUniforms.SpotAngles.x);
        float WorldLightDistance = dot(DeferredLightUniforms.NormalizedLightDirection, DeferredLightUniforms.LightPosition - WorldPosition);
        Settings.ProjectedSourceRadius = 0.5 * DeferredLightUniforms.SourceRadius * CotanOuterCone / WorldLightDistance;
        Settings.TanLightSourceAngle = 0;
        }
        #endif
        Can someone confirm that extra scaling with 0.5 is wrong? It's seems that code is just confusing diameter and radius. Also CotanOuterCone could be precalculated.
        Considering that SpotAngles.x store cosine of half outer spotlight angle, indeed 0.5 seems incorrect here. Stuff can(and as best practice, should be) precalced, but it should be folded to unifrom by compiler.
        Last edited by Deathrey; 03-01-2018, 02:36 PM.

        Comment


          Originally posted by Deathrey View Post

          Considering that SpotAngles.x store cosine of half outer spotlight angle, indeed 0.5 seems incorrect here.
          Good to get second opinion. Trigonometry isn't my strong suit. I will send PR to correct this.


          Originally posted by NilsonLima View Post
          Hmm... I see now... Which issue that would fix? I mean, there is a reason for you to reach that part of the code, so the correct answer maybe is behind the question: Why are you there?

          Also, as a side comment, sometimes people just multiply stuff for 0.5 instead of dividing by 2. Modern compilers usually will do the best choice regardless the intention, not sure how the shader compiler is handling such thing thou. Anyways, this might lead to a copied and pasted code with an error inherited or there is an reason behind this which only a comment in code would dismiss (not this case unfortunetelly).

          You finding is solid, as for the repercussion changing it, I don't know. The usage ahead would tell I guess.

          I just noticed that stationary spotlight softshadows are not as soft for dynamic objects that for static. Then I noticed that 0.5 multiplier which seemed off. Then I needed to do math and my assumption seemed to be correct. I used lightmass as reference so I can compare against ground truth.

          Comment


            Originally posted by Deathrey View Post

            Considering that SpotAngles.x store cosine of half outer spotlight angle, indeed 0.5 seems incorrect here. Stuff can(and as best practice, should be) precalced, but it should be folded to unifrom by compiler.
            Could the 0.5 be multplier because after perspective projection coordinates are in clip space (-1 to 1) but we need radius to be at texture space(0 to 1). I am not sure about this at all.

            Ps. Ligthmass reference says that even after removing 0.5 scale, dynamic shadows were bit too sharp.

            Comment


              Originally posted by Kalle_H View Post

              Could the 0.5 be multplier because after perspective projection coordinates are in clip space (-1 to 1) but we need radius to be at texture space(0 to 1). I am not sure about this at all.

              Ps. Ligthmass reference says that even after removing 0.5 scale, dynamic shadows were bit too sharp.
              Search radius is intended to be used as in shadow space, but shader gets source radius as is, from the light, correct me if i'm wrong.


              Also, what happens when source radius is 0 ? Projecteded source radius is also 0, and all the softenss comes just from PCF.

              Me thinks that Light Source Radius needs to be adjusted to be in shadow space before being used.



              Comment


                Originally posted by Deathrey View Post

                Search radius is intended to be used as in shadow space, but shader gets source radius as is, from the light, correct me if i'm wrong.


                Also, what happens when source radius is 0 ? Projecteded source radius is also 0, and all the softenss comes just from PCF.

                Me thinks that Light Source Radius needs to be adjusted to be in shadow space before being used.


                When radius is 0 the projected value is clamped to one texel.

                Source radius is projected from world space to shadow clip space:
                Code:
                radius * (1 / tan(fov/2)) / ShadowSpaceDepth

                Comment


                  0.5 there seems to be correct visually. Lightmass difference can be attributed to exposure, biasing, sharpening and filter radius clamping.

                  Lightmass:

                  Click image for larger version  Name:	LightMass.png Views:	1 Size:	531.1 KB ID:	1437746


                  PCSS:

                  Click image for larger version  Name:	PCSS.png Views:	1 Size:	431.9 KB ID:	1437747

                  PCSS low exposure:

                  Click image for larger version  Name:	PCSS_LowExposure.png Views:	1 Size:	261.1 KB ID:	1437748

                  PCSS high exposure:

                  Click image for larger version  Name:	PCSS_HighExposure.png Views:	1 Size:	448.9 KB ID:	1437749

                  As to why is it there, needs in depth look. I don't have source accessible at the moment, will check in a few days.

                  Comment


                    I will make more lightmass testing. If the multiplier is indeed correct I will remove my PR.

                    Comment


                      Original shader:
                      Click image for larger version  Name:	WithMultiplier.jpg Views:	1 Size:	84.4 KB ID:	1438121

                      Without Multiplier:
                      Click image for larger version  Name:	WithoutMultiplier.jpg Views:	1 Size:	89.6 KB ID:	1438122Reference:Click image for larger version  Name:	Lightmass.jpg Views:	1 Size:	81.9 KB ID:	1438120

                      Comment


                        Returning to my previous post, is spotlight projection adjusted for source radius? I believe it is not and it is root of all problems.

                        Comment


                          Originally posted by Deathrey View Post
                          Returning to my previous post, is spotlight projection adjusted for source radius? I believe it is not and it is root of all problems.
                          CotanOuterCone variable is the adjustment.

                          Comment


                            Originally posted by Kalle_H View Post
                            CotanOuterCone variable is the adjustment.
                            What I was asking, aren't you supposed to shift whole shadow projection behind, so that source is at spotlight's near clip plane? That is.. at distance zero, your source raidus will be radius, not zero.

                            I haven't implemented PCSS for spot and omni lights myself, so bear with me.


                            In case, that above is a brainfart, what is happening with PCSSParameters.y ?

                            Comment


                              Red Color: Expected Umbra
                              Green Color: Expected Penumbra
                              Blue Color: Received umbra at defaults

                              default:
                              Click image for larger version  Name:	UnadjustedPCSS.png Views:	1 Size:	501.2 KB ID:	1438307



                              0.5 replaced with 1.0,
                              and max kernel size inflated:




                              Click image for larger version  Name:	PCSS.png Views:	1 Size:	459.8 KB ID:	1438302
                              Last edited by Deathrey; 03-05-2018, 11:14 AM.

                              Comment


                                Originally posted by Deathrey View Post

                                What I was asking, aren't you supposed to shift whole shadow projection behind, so that source is at spotlight's near clip plane? That is.. at distance zero, your source raidus will be radius, not zero.

                                I haven't implemented PCSS for spot and omni lights myself, so bear with me.


                                In case, that above is a brainfart, what is happening with PCSSParameters.y ?
                                I don't know about that shifting. There is only division by projected distance. Near plane distance isn't factored at all. Not sure about should it.

                                PCSSParameters.y = MaxKernelSize / float(ShadowInfo->ResolutionX). So this should be correctly adjusted.

                                Comment

                                Working...
                                X