[Mesa-dev] [PATCH] gallium: Make TGSI_SEMANTIC_FOG register four-component wide.

Jose Fonseca jfonseca at vmware.com
Wed Nov 20 08:48:42 PST 2013


----- Original Message -----
> IIRC, all Radeons older than HD 7700 or so can do fixed-function fog,
> but Gallium doesn't have a proper interface for it. There is no
> dedicated shader output though, so r300g and r600g use a texcoord slot
> if there is any left. I think all drivers except
> softpipe/llvmpipe/svga already do the X001 swizzling.
> 
> Considering that the fog varying is being turned into a generic vec4,
> I wonder why we don't just use TGSI_SEMANTIC_GENERIC for it.

Same could be said for many TGSI_SEMANTIC_*.  At very least, having a TGSI_SEMANTIC_FOG allows add registers for fog with the need to do complicated renumbering of TGSI_SEMANTIC_GENERIC indices, and it also provides helpful hints for  human reading the shaders.  

I wouldn't mind if we axed most of these fixed function relics, but it's probably a fair amount of work to do so, and I'm not sure the benefits justify.

Jose

> 
> Marek
> 
> On Wed, Nov 20, 2013 at 5:28 PM, Roland Scheidegger <sroland at vmware.com>
> wrote:
> > On 11/20/2013 03:23 PM, jfonseca at vmware.com wrote:
> >>
> >> From: José Fonseca <jfonseca at vmware.com>
> >>
> >> D3D9 Shader Model 2 restricted the fog register to one component,
> >> https://urldefense.proofpoint.com/v1/url?u=http://msdn.microsoft.com/en-us/library/windows/desktop/bb172945.aspx&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=NMr9uy2iTjWVixC0wOcYCWEIYhfo80qKwRgdodpoDzA%3D%0A&m=QYVehdp8Eys%2F%2BELh4mOPqGab2c6TppOFGBX7pi8YkdM%3D%0A&s=b367b797783ff401bc5b67fffa82046a1ab34e8e0663ab252b580c358d6d76c7
> >> ,
> >> but that restriction no longer exists in Shader Model 3, and several
> >> WHCK tests enforce that.
> >>
> >> So this change:
> >> - lifts the single-component restriction TGSI_SEMANTIC_FOG
> >>    from Gallium interface
> >> - updates the Mesa state tracker to enforce output fog has (f, 0, 0, 1)
> >> - draw module was updated to leave TGSI_SEMANTIC_FOG output registers
> >>    alone
> >>
> >> Several gallium drivers that are going out of their way to clear
> >> TGSI_SEMANTIC_FOG components could be simplified in the future.
> >>
> >> Thanks to Si Chen and Michal Krol for identifying the problem.
> >>
> >> Testing done: piglit fogcoord-*.vpfp tests
> >> ---
> >>   src/gallium/auxiliary/draw/draw_llvm.c     |  6 ------
> >>   src/gallium/auxiliary/draw/draw_vs_exec.c  |  7 +------
> >>   src/gallium/docs/source/tgsi.rst           | 10 +++-------
> >>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  7 +++++++
> >>   src/mesa/state_tracker/st_mesa_to_tgsi.c   |  7 +++++++
> >>   5 files changed, 18 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/src/gallium/auxiliary/draw/draw_llvm.c
> >> b/src/gallium/auxiliary/draw/draw_llvm.c
> >> index fe49b86..71cc45f 100644
> >> --- a/src/gallium/auxiliary/draw/draw_llvm.c
> >> +++ b/src/gallium/auxiliary/draw/draw_llvm.c
> >> @@ -659,12 +659,6 @@ generate_vs(struct draw_llvm_variant *variant,
> >>                        LLVMBuildStore(builder, out,
> >> outputs[attrib][chan]);
> >>                     }
> >>                     break;
> >> -               case TGSI_SEMANTIC_FOG:
> >> -                  if (chan == 1 || chan == 2)
> >> -                     LLVMBuildStore(builder, bld.zero,
> >> outputs[attrib][chan]);
> >> -                  else if (chan == 3)
> >> -                     LLVMBuildStore(builder, bld.one,
> >> outputs[attrib][chan]);
> >> -                  break;
> >>                  }
> >>               }
> >>            }
> >> diff --git a/src/gallium/auxiliary/draw/draw_vs_exec.c
> >> b/src/gallium/auxiliary/draw/draw_vs_exec.c
> >> index 6100394..83cc5fd 100644
> >> --- a/src/gallium/auxiliary/draw/draw_vs_exec.c
> >> +++ b/src/gallium/auxiliary/draw/draw_vs_exec.c
> >> @@ -167,12 +167,7 @@ vs_exec_run_linear( struct draw_vertex_shader
> >> *shader,
> >>                  output[slot][2] =
> >> CLAMP(machine->Outputs[slot].xyzw[2].f[j], 0.0f, 1.0f);
> >>                  output[slot][3] =
> >> CLAMP(machine->Outputs[slot].xyzw[3].f[j], 0.0f, 1.0f);
> >>               }
> >> -            else if (name == TGSI_SEMANTIC_FOG) {
> >> -               output[slot][0] = machine->Outputs[slot].xyzw[0].f[j];
> >> -               output[slot][1] = 0;
> >> -               output[slot][2] = 0;
> >> -               output[slot][3] = 1;
> >> -           } else
> >> +            else
> >>               {
> >>                  output[slot][0] = machine->Outputs[slot].xyzw[0].f[j];
> >>                  output[slot][1] = machine->Outputs[slot].xyzw[1].f[j];
> >> diff --git a/src/gallium/docs/source/tgsi.rst
> >> b/src/gallium/docs/source/tgsi.rst
> >> index f80c08d..1070619 100644
> >> --- a/src/gallium/docs/source/tgsi.rst
> >> +++ b/src/gallium/docs/source/tgsi.rst
> >> @@ -2420,13 +2420,9 @@ TGSI_SEMANTIC_FOG
> >>
> >>   Vertex shader inputs and outputs and fragment shader inputs may be
> >>   labeled with TGSI_SEMANTIC_FOG to indicate that the register contains
> >> -a fog coordinate in the form (F, 0, 0, 1).  Typically, the fragment
> >> -shader will use the fog coordinate to compute a fog blend factor which
> >> -is used to blend the normal fragment color with a constant fog color.
> >> -
> >> -Only the first component matters when writing from the vertex shader;
> >> -the driver will ensure that the coordinate is in this format when used
> >> -as a fragment shader input.
> >> +a fog coordinate.  Typically, the fragment shader will use the fog
> >> coordinate
> >> +to compute a fog blend factor which is used to blend the normal fragment
> >> color
> >> +with a constant fog color.
> >
> > Maybe add some text that the "fog coord" really is just an ordinary vec4
> > reg
> > like regular semantics?
> >
> >
> >
> >>
> >>
> >>   TGSI_SEMANTIC_PSIZE
> >> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> >> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> >> index 6319079..74b3e5b 100644
> >> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> >> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> >> @@ -4889,6 +4889,13 @@ st_translate_program(
> >>            t->outputs[i] = ureg_DECL_output(ureg,
> >>                                             outputSemanticName[i],
> >>                                             outputSemanticIndex[i]);
> >> +         if (outputSemanticName[i] == TGSI_SEMANTIC_FOG) {
> >> +            /* force register to contain a fog coordinate in the form (F,
> >> 0, 0, 1). */
> >> +            ureg_MOV(ureg,
> >> +                     ureg_writemask(t->outputs[i], TGSI_WRITEMASK_XYZW &
> >> ~TGSI_WRITEMASK_X),
> >> +                     ureg_imm4f(ureg, 0.0f, 0.0f, 0.0f, 1.0f));
> >> +            t->outputs[i] = ureg_writemask(t->outputs[i],
> >> TGSI_WRITEMASK_X);
> >> +        }
> >>         }
> >>         if (passthrough_edgeflags)
> >>            emit_edgeflags(t);
> >> diff --git a/src/mesa/state_tracker/st_mesa_to_tgsi.c
> >> b/src/mesa/state_tracker/st_mesa_to_tgsi.c
> >> index 921b0f9..7d79c62 100644
> >> --- a/src/mesa/state_tracker/st_mesa_to_tgsi.c
> >> +++ b/src/mesa/state_tracker/st_mesa_to_tgsi.c
> >> @@ -1121,6 +1121,13 @@ st_translate_mesa_program(
> >>            t->outputs[i] = ureg_DECL_output( ureg,
> >>                                              outputSemanticName[i],
> >>                                              outputSemanticIndex[i] );
> >> +         if (outputSemanticName[i] == TGSI_SEMANTIC_FOG) {
> >> +            /* force register to contain a fog coordinate in the form (F,
> >> 0, 0, 1). */
> >> +            ureg_MOV(ureg,
> >> +                     ureg_writemask(t->outputs[i], TGSI_WRITEMASK_XYZW &
> >> ~TGSI_WRITEMASK_X),
> >> +                     ureg_imm4f(ureg, 0.0f, 0.0f, 0.0f, 1.0f));
> >> +            t->outputs[i] = ureg_writemask(t->outputs[i],
> >> TGSI_WRITEMASK_X);
> >> +        }
> >>         }
> >>         if (passthrough_edgeflags)
> >>            emit_edgeflags( t, program );
> >>
> >
> > Otherwise looks good to me. Can't say I'm a big fan of our half-baked
> > d3d10-bolted-on-d3d9 semantics style, but this just changes a weirdo
> > semantic into one which doesn't really have much meaning which is fine by
> > me
> > - I think you get lucky because there aren't any drivers which can't
> > actually do this, even r300 which can't do SM3 looks like it treats fog
> > interpolation like any other generic semantic (though that was just a very
> > quick glance).
> >
> > Roland
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/mesa-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=NMr9uy2iTjWVixC0wOcYCWEIYhfo80qKwRgdodpoDzA%3D%0A&m=QYVehdp8Eys%2F%2BELh4mOPqGab2c6TppOFGBX7pi8YkdM%3D%0A&s=bf0162e7afd51cd3d39a500bd60b6997783b1be741baeb2bb52a2d9293ace2dd
>


More information about the mesa-dev mailing list