Luca Barbieri wrote: > Changes in v3: > - Use positive caps instead of negative ones > > Changes in v2: > - Updated formatting > > The state tracker will use the TGSI convention properties if the hardware > exposes the appropriate capability, and otherwise adjust WPOS itself. > > This will also fix some drivers that were previously broken due to their > incorrect, inadvertent, use of conventions other than upper_left+half_integer.
Minor comments below. > --- > src/mesa/state_tracker/st_extensions.c | 1 + > src/mesa/state_tracker/st_mesa_to_tgsi.c | 74 ++++++++++++++++++++++++++++- > 2 files changed, 72 insertions(+), 3 deletions(-) > > diff --git a/src/mesa/state_tracker/st_extensions.c > b/src/mesa/state_tracker/st_extensions.c > index 60732f3..982a1fb 100644 > --- a/src/mesa/state_tracker/st_extensions.c > +++ b/src/mesa/state_tracker/st_extensions.c > @@ -147,6 +147,7 @@ void st_init_extensions(struct st_context *st) > * Extensions that are supported by all Gallium drivers: > */ > ctx->Extensions.ARB_copy_buffer = GL_TRUE; > + ctx->Extensions.ARB_fragment_coord_conventions = GL_TRUE; We can't really expose this extension until we've updated GLSL to understand layout qualifiers. And, unfortunately, that builds on GLSL 1.40 which we don't support yet. > ctx->Extensions.ARB_fragment_program = GL_TRUE; > ctx->Extensions.ARB_half_float_vertex = GL_TRUE; > ctx->Extensions.ARB_map_buffer_range = GL_TRUE; > diff --git a/src/mesa/state_tracker/st_mesa_to_tgsi.c > b/src/mesa/state_tracker/st_mesa_to_tgsi.c > index 05b56c9..366729a 100644 > --- a/src/mesa/state_tracker/st_mesa_to_tgsi.c > +++ b/src/mesa/state_tracker/st_mesa_to_tgsi.c > @@ -34,8 +34,10 @@ > #include "pipe/p_compiler.h" > #include "pipe/p_shader_tokens.h" > #include "pipe/p_state.h" > +#include "pipe/p_context.h" > #include "tgsi/tgsi_ureg.h" > #include "st_mesa_to_tgsi.h" > +#include "st_context.h" > #include "shader/prog_instruction.h" > #include "shader/prog_parameter.h" > #include "shader/prog_print.h" > @@ -665,6 +667,22 @@ compile_instruction( > } > } > > +/** > + * Emit the TGSI instructions to adjust the WPOS pixel center convention > + */ > +static void > +emit_adjusted_wpos( struct st_translate *t, > + const struct gl_program *program, GLfloat value) > +{ > + struct ureg_program *ureg = t->ureg; > + struct ureg_dst wpos_temp = ureg_DECL_temporary(ureg); > + struct ureg_src wpos_input = t->inputs[t->inputMapping[FRAG_ATTRIB_WPOS]]; > + > + ureg_ADD(ureg, ureg_writemask(wpos_temp, TGSI_WRITEMASK_X | > TGSI_WRITEMASK_Y), > + wpos_input, ureg_imm1f(ureg, value)); > + > + t->inputs[t->inputMapping[FRAG_ATTRIB_WPOS]] = ureg_src(wpos_temp); > +} > > /** > * Emit the TGSI instructions for inverting the WPOS y coordinate. > @@ -690,12 +708,17 @@ emit_inverted_wpos( struct st_translate *t, > winSizeState); > > struct ureg_src winsize = ureg_DECL_constant( ureg, winHeightConst ); > - struct ureg_dst wpos_temp = ureg_DECL_temporary( ureg ); > + struct ureg_dst wpos_temp; > struct ureg_src wpos_input = t->inputs[t->inputMapping[FRAG_ATTRIB_WPOS]]; > > /* MOV wpos_temp, input[wpos] > */ > - ureg_MOV( ureg, wpos_temp, wpos_input ); > + if (wpos_input.File == TGSI_FILE_TEMPORARY) > + wpos_temp = ureg_dst(wpos_input); > + else { > + wpos_temp = ureg_DECL_temporary( ureg ); > + ureg_MOV( ureg, wpos_temp, wpos_input ); > + } > > /* SUB wpos_temp.y, winsize_const, wpos_input > */ > @@ -801,6 +824,7 @@ st_translate_mesa_program( > * Declare input attributes. > */ > if (procType == TGSI_PROCESSOR_FRAGMENT) { > + struct gl_fragment_program* fp = (struct gl_fragment_program*)program; > for (i = 0; i < numInputs; i++) { > t->inputs[i] = ureg_DECL_fs_input(ureg, > inputSemanticName[i], > @@ -812,7 +836,51 @@ st_translate_mesa_program( > /* Must do this after setting up t->inputs, and before > * emitting constant references, below: > */ > - emit_inverted_wpos( t, program ); > + struct pipe_screen* pscreen = st_context(ctx)->pipe->screen; > + int invert = 0; This should be "boolean invert = FALSE". > + if (!fp->OriginUpperLeft) { > + if (pscreen->get_param(pscreen, > PIPE_CAP_TGSI_FS_COORD_ORIGIN_LOWER_LEFT)) > + ureg_property_fs_coord_origin(ureg, > TGSI_FS_COORD_ORIGIN_LOWER_LEFT); > + else if (pscreen->get_param(pscreen, > PIPE_CAP_TGSI_FS_COORD_ORIGIN_UPPER_LEFT)) > + invert = 1; > + else > + assert(0); > + } > + else { > + if (pscreen->get_param(pscreen, > PIPE_CAP_TGSI_FS_COORD_ORIGIN_UPPER_LEFT)) { > + } > + else if (pscreen->get_param(pscreen, > PIPE_CAP_TGSI_FS_COORD_ORIGIN_LOWER_LEFT)) { > + ureg_property_fs_coord_origin(ureg, > TGSI_FS_COORD_ORIGIN_LOWER_LEFT); > + invert = 1; > + } > + else > + assert(0); > + } In an if/else like this I think it's a little easier to read if the condition is "if (fp->OriginUpperLeft) {". That is, switch the true/false clauses and remove the "not". As you do in the next piece of code: > + > + if (fp->PixelCenterInteger) { > + if (pscreen->get_param(pscreen, > PIPE_CAP_TGSI_FS_COORD_PIXEL_CENTER_INTEGER)) > + ureg_property_fs_coord_pixel_center(ureg, > TGSI_FS_COORD_PIXEL_CENTER_INTEGER); > + else if (pscreen->get_param(pscreen, > PIPE_CAP_TGSI_FS_COORD_PIXEL_CENTER_HALF_INTEGER)) > + emit_adjusted_wpos(t, program, invert ? 0.5f : -0.5f); > + else > + assert(0); > + } > + else { > + if (pscreen->get_param(pscreen, > PIPE_CAP_TGSI_FS_COORD_PIXEL_CENTER_HALF_INTEGER)) { > + } > + else if (pscreen->get_param(pscreen, > PIPE_CAP_TGSI_FS_COORD_PIXEL_CENTER_INTEGER)) { > + ureg_property_fs_coord_pixel_center(ureg, > TGSI_FS_COORD_PIXEL_CENTER_INTEGER); > + emit_adjusted_wpos(t, program, invert ? -0.5f : 0.5f); > + } > + else > + assert(0); > + } > + > + /* we invert after adjustment so that we avoid the MOV to temporary, > + * and reuse the adjustment ADD instead */ > + if (invert) > + emit_inverted_wpos(t, program); > } > > if (program->InputsRead & FRAG_BIT_FACE) { ------------------------------------------------------------------------------ The Planet: dedicated and managed hosting, cloud storage, colocation Stay online with enterprise data centers and the best network in the business Choose flexible plans and management services without long-term contracts Personal 24x7 support from experience hosting pros just a phone call away. http://p.sf.net/sfu/theplanet-com _______________________________________________ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev