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

Jose Fonseca jfonseca at vmware.com
Wed Nov 20 08:49:23 PST 2013



----- Original Message -----
> ----- 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

Sorry, meant  "without" here.

> 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