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
