#238 Stencil shadow bugfix and optimize

closed-accepted
bugfix (702)
5
2005-01-08
2005-01-04
Genva
No

The attached patch file has bugfix and some optimize as
describe below:
1. Fixed incorrect near clip volume generate for near
plane plane.
2. Use Plane::Plane(const Vector3& rkNormal, const
Vector3& rkPoint) constructor instead of
Plane::Plane(const Vector3& rkNormal, Real fConstant)
to construct plane.
3. Make sure zfail compatible with zpass algorithm even
if two-side stencil enabled.
4. Optimize zfail/zpass algorithm determine, use zfail
for the object is EXACTLY required it ONLY, not for ALL
objects when at least one object requires it.
5. Dark cap only require for zfail algorithm.
6. Clean code and minimize stencil state change for
rendering shadow renderables.
7. Use stencil wrap always if it's present.
8. Some other code clean ...

Discussion

  • Genva

    Genva - 2005-01-04

    Logged In: YES
    user_id=1142737

    Addtional patch file (StencilShadowExtra.zip) for remove
    getLastShadowVolumeRenderableIterator() method because not
    necessary anymore.

     
  • Genva

    Genva - 2005-01-04

    Logged In: YES
    user_id=1142737

    There are still has bug when camera is reflected, the bug
    occur because Light::_getNearClipVolume() and
    Light::_getFrustumClipVolumes() can not dealing with
    reflected camera correct.

    D3D9RenderSystem::setStencilBufferParams() may be also has
    same bug when mInvertVertexWinding is set to 'true'.

    I will try to resolve this issues in next couple of day and
    submit an addtional patch as soon as possible.

    So far as I know, the patch already submitted still work
    fine except mirrored camera.

     
  • Genva

    Genva - 2005-01-05

    New patch file replace all previous post

     
  • Genva

    Genva - 2005-01-05

    Logged In: YES
    user_id=1142737

    I upload the new patch file (StencilShadow_v2.zip) which
    take care with all my talking about, including render
    stencil in reflected camera bug fixed, plus some useful
    optimize. This patch intended to replace with
    StencilShadow.zip and StencilShadowExtra.zip I previous post.

    There have a little hack code in the patch because a bug in
    Camera::getDerivedPosition() which never take reflection
    into account.

    Hopeful some one check in CVS because is a very important
    bugfix, and I do a lot of test on it.

     
  • Genva

    Genva - 2005-01-07

    Logged In: YES
    user_id=1142737

    The new patch (StencilShadow_v3.zip) fixup the back facing
    light caps bug in the way of GPU overhead. Plus improve
    two-sided stencil front face consistently with default
    culling mode, this is necessary because we enabled 2-sided
    stencil and use culling mode to select which face need to be
    render.

    There has a little work todo:
    1. Separate light caps rendering doesn't need for the
    'closed' mesh that never touch the near plane, because in
    this instance, we couldn't 'see' any back facing triangles
    directly, therefore no artefacts should be occur. The
    'closed' mesh must be determinate by edge list builder.
    2. There still exists 'depth fighting' bug with coplane
    triangles that has opposite facing. This usually occur when
    use two side material in the modeling tools and the model
    exporting tools exporting double triangles to represent this
    model. This bug can't fixed in GPU only, there must has
    extra work on edge list builder and shadow volume generater
    to fix it.

     
  • Genva

    Genva - 2005-01-07

    New patch plus fix for back facing light caps bug, replace all others

     
  • Steve Streeting

    Steve Streeting - 2005-01-08

    Logged In: YES
    user_id=16123

    Just reading the description of this, you will have to
    remove optimisation 4. if it means what I think it means
    (that you only render the object which caused zfail as
    zfail, everything else as zpass).

    This was how the algorithm was implemented originally before
    the 0.14.0 release and it didn't work. IIRC the problem was
    when volumes for zpass and zfail overlapped in screen space.
    The whole pass for that light needs to be done in either
    zpass or zfail to give accurate results.

     
  • Steve Streeting

    Steve Streeting - 2005-01-08

    Logged In: YES
    user_id=16123

    Also dark cap is not only required for zfail. It is also
    required for zpass if infinite extrusion (vertex programs)
    is not supported, since it is possible to see the cap from
    some zpass angles in this case. So change 5 will also need
    removing.

     
  • Steve Streeting

    Steve Streeting - 2005-01-08

    Logged In: YES
    user_id=16123

    Hmm, sorry to keep picking holes in this.

    But it's also not a good idea to always use stencil wrap.
    It's not available on some hardware, especially that which
    doesn't have 2-sided stencilling. You don't need it when you
    do non-2 sided stencilling since you always make sure you
    increment everything first, then decrement everything. If
    you try to use wrap all the time it will fail on some
    hardware. Please remove that change too.

     
  • Steve Streeting

    Steve Streeting - 2005-01-08

    Logged In: YES
    user_id=16123

    Ignore the last comment, I read the code and see that you
    detect it. I still think it's not worth changing it though
    since you have to increment first anyway in case you don't
    have wrap available.

     
  • Steve Streeting

    Steve Streeting - 2005-01-08
    • assigned_to: nobody --> sinbad
     
  • Steve Streeting

    Steve Streeting - 2005-01-08

    Logged In: YES
    user_id=16123

    I've accepted this patch, but I've removed changes 4 and 5
    as discussed below.

    I was a bit concerned about the light cap change (rendering
    it twice to deal with seeing back faces), since it now
    results in 2 renders of the light cap instead of one when
    using vertex programs to extrude. However, performance tests
    still show a slight increase in speed (about 3% ish in
    Demo_Shadows) so I've left it in. At some point we can
    address the TODO so that this only happens for non-manifold
    meshes.

    Thanks

    NB I also found another bug in the original Ogre code -
    texture units were not being turned off when rendering
    stencils, and certain combinations of the previous texture
    unit state and the shadow extrusion program caused it to
    fail. I've fixed this at the same time.

     
  • Steve Streeting

    Steve Streeting - 2005-01-08
    • status: open --> closed-accepted
     
  • Steve Streeting

    Steve Streeting - 2005-01-09

    Logged In: YES
    user_id=16123

    After thinking about this some more, I've re-evaluated 4,
    and I realise that further changes made in 0.15 mean that
    the original reason for doing ALL zfail has now gone; it was
    required before (and there was a bugfix for it in fact) but
    the ordering has since changed. Therefore I've put 4 back.

    5 was still incorrect as it was (glancing angles when finite
    extrusion is used can cause incorrect shadowing out of the
    end of the volume in zpass when dark caps are not used).
    However, I see your intent, so I've re-written it with
    clearer comments, and added an additional separate check in
    zpass which will deal with this case. I'm doing yet more
    testing but I think this will work and will preserve the
    spirit of change 5.

     
  • Steve Streeting

    Steve Streeting - 2005-01-09

    Logged In: YES
    user_id=16123

    In fact there's another zpass case which requires a dark cap
    - infinite extrusion of point/spotlights where there are
    areas of uninitialised depth. You can see this in
    Demo_Shadows with your original patch for 5 if you switch to
    modulative mode - large bands of shadow on the skybox. This
    is because infinite extrusion places the dark area on an
    infinite sphere so that the depth may pass on the way in but
    never hit the volume on the way out. I've put this in as
    another check in zpass. I'm still testing.

     
  • Steve Streeting

    Steve Streeting - 2005-01-09

    Logged In: YES
    user_id=16123

    Ok, I'm committing change 4, and my replacement for change 5
    which takes into account the 2 reasons you might need dark
    caps for zpass.

     

Log in to post a comment.