Re: [Mesa-dev] [PATCH 1/3] gallium: switch dedicated centroid field to interpolation location
Am 09.07.2014 02:37, schrieb Ilia Mirkin: > On Sat, Jul 5, 2014 at 5:20 PM, Roland Scheidegger wrote: >> Am 05.07.2014 06:07, schrieb Ilia Mirkin: >>> The new location field can be either center, centroid, or sample, which >>> indicates the location that the shader should interpolate at. >> Looks good though maybe it should be mentioned in the interpolator >> section in tgsi.rst (yeah centroid wasn't neither). > > I added this bit: > > """ > +The Location field specifies the location inside the pixel that the > +interpolation should be done at, one of ``TGSI_INTERPOLATE_LOC_*``. > """ > > Let me know if you want me to resend. No that's just fine - there's a reason I added Reviewed-by and still having some nitpicks... Roland > >> In any case, >> Reviewed-by: Roland Scheidegger >> >>> >>> Signed-off-by: Ilia Mirkin >>> --- >>> >>> I tried to make sure I hit all the uses, but I guess a bunch of drivers >>> don't >>> look at the Centroid field at all? >> Well drivers not doing msaa or just fake one like llvmpipe aren't >> interested in it. >> And, this wasn't exposed via d3d9, though r300 reportedly can do it (the >> docs even mention it for r5xx), I suspect using some of the crazy >> backdoor mechanisms of d3d9. Some very quick look at the docs though >> don't indicate how this could be done (if it's even possible per >> interpolator) for these chips, which is I guess why centroid isn't handled. >> >>> >>> src/gallium/auxiliary/tgsi/tgsi_build.c | 8 >>> src/gallium/auxiliary/tgsi/tgsi_dump.c| 5 +++-- >>> src/gallium/auxiliary/tgsi/tgsi_scan.c| 2 +- >>> src/gallium/auxiliary/tgsi/tgsi_scan.h| 2 +- >>> src/gallium/auxiliary/tgsi/tgsi_strings.c | 7 +++ >>> src/gallium/auxiliary/tgsi/tgsi_strings.h | 2 ++ >>> src/gallium/auxiliary/tgsi/tgsi_ureg.c| 12 ++-- >>> src/gallium/auxiliary/tgsi/tgsi_ureg.h| 2 +- >>> src/gallium/drivers/ilo/shader/toy_tgsi.c | 2 +- >>> src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 2 +- >>> src/gallium/drivers/r600/r600_shader.c| 4 ++-- >>> src/gallium/drivers/radeonsi/si_shader.c | 6 +++--- >>> src/gallium/include/pipe/p_shader_tokens.h| 9 +++-- >>> src/mesa/state_tracker/st_glsl_to_tgsi.cpp| 5 +++-- >>> src/mesa/state_tracker/st_glsl_to_tgsi.h | 2 +- >>> src/mesa/state_tracker/st_program.c | 13 >>> + >>> 16 files changed, 52 insertions(+), 31 deletions(-) >>> >>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_build.c >>> b/src/gallium/auxiliary/tgsi/tgsi_build.c >>> index 7621b6a..bef5c75 100644 >>> --- a/src/gallium/auxiliary/tgsi/tgsi_build.c >>> +++ b/src/gallium/auxiliary/tgsi/tgsi_build.c >>> @@ -200,7 +200,7 @@ tgsi_default_declaration_interp( void ) >>> struct tgsi_declaration_interp di; >>> >>> di.Interpolate = TGSI_INTERPOLATE_CONSTANT; >>> - di.Centroid = 0; >>> + di.Location = TGSI_INTERPOLATE_LOC_CENTER; >>> di.CylindricalWrap = 0; >>> di.Padding = 0; >>> >>> @@ -209,7 +209,7 @@ tgsi_default_declaration_interp( void ) >>> >>> static struct tgsi_declaration_interp >>> tgsi_build_declaration_interp(unsigned interpolate, >>> - unsigned centroid, >>> + unsigned interpolate_location, >>>unsigned cylindrical_wrap, >>>struct tgsi_declaration *declaration, >>>struct tgsi_header *header) >>> @@ -217,7 +217,7 @@ tgsi_build_declaration_interp(unsigned interpolate, >>> struct tgsi_declaration_interp di; >>> >>> di.Interpolate = interpolate; >>> - di.Centroid = centroid; >>> + di.Location = interpolate_location; >>> di.CylindricalWrap = cylindrical_wrap; >>> di.Padding = 0; >>> >>> @@ -433,7 +433,7 @@ tgsi_build_full_declaration( >>>size++; >>> >>>*di = tgsi_build_declaration_interp(full_decl->Interp.Interpolate, >>> - full_decl->Interp.Centroid, >>> + full_decl->Interp.Location, >>> >>> full_decl->Interp.CylindricalWrap, >>>declaration, >>>header); >>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c >>> b/src/gallium/auxiliary/tgsi/tgsi_dump.c >>> index 8e09bac..884d8cf 100644 >>> --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c >>> +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c >>> @@ -349,8 +349,9 @@ iter_declaration( >>> ENM( decl->Interp.Interpolate, tgsi_interpolate_names ); >>>} >>> >>> - if (decl->Interp.Centroid) { >>> - TXT( ", CENTROID" ); >>> + if (decl->Interp.Loc
Re: [Mesa-dev] [PATCH 1/3] gallium: switch dedicated centroid field to interpolation location
On Sat, Jul 5, 2014 at 5:20 PM, Roland Scheidegger wrote: > Am 05.07.2014 06:07, schrieb Ilia Mirkin: >> The new location field can be either center, centroid, or sample, which >> indicates the location that the shader should interpolate at. > Looks good though maybe it should be mentioned in the interpolator > section in tgsi.rst (yeah centroid wasn't neither). I added this bit: """ +The Location field specifies the location inside the pixel that the +interpolation should be done at, one of ``TGSI_INTERPOLATE_LOC_*``. """ Let me know if you want me to resend. > In any case, > Reviewed-by: Roland Scheidegger > >> >> Signed-off-by: Ilia Mirkin >> --- >> >> I tried to make sure I hit all the uses, but I guess a bunch of drivers don't >> look at the Centroid field at all? > Well drivers not doing msaa or just fake one like llvmpipe aren't > interested in it. > And, this wasn't exposed via d3d9, though r300 reportedly can do it (the > docs even mention it for r5xx), I suspect using some of the crazy > backdoor mechanisms of d3d9. Some very quick look at the docs though > don't indicate how this could be done (if it's even possible per > interpolator) for these chips, which is I guess why centroid isn't handled. > >> >> src/gallium/auxiliary/tgsi/tgsi_build.c | 8 >> src/gallium/auxiliary/tgsi/tgsi_dump.c| 5 +++-- >> src/gallium/auxiliary/tgsi/tgsi_scan.c| 2 +- >> src/gallium/auxiliary/tgsi/tgsi_scan.h| 2 +- >> src/gallium/auxiliary/tgsi/tgsi_strings.c | 7 +++ >> src/gallium/auxiliary/tgsi/tgsi_strings.h | 2 ++ >> src/gallium/auxiliary/tgsi/tgsi_ureg.c| 12 ++-- >> src/gallium/auxiliary/tgsi/tgsi_ureg.h| 2 +- >> src/gallium/drivers/ilo/shader/toy_tgsi.c | 2 +- >> src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 2 +- >> src/gallium/drivers/r600/r600_shader.c| 4 ++-- >> src/gallium/drivers/radeonsi/si_shader.c | 6 +++--- >> src/gallium/include/pipe/p_shader_tokens.h| 9 +++-- >> src/mesa/state_tracker/st_glsl_to_tgsi.cpp| 5 +++-- >> src/mesa/state_tracker/st_glsl_to_tgsi.h | 2 +- >> src/mesa/state_tracker/st_program.c | 13 + >> 16 files changed, 52 insertions(+), 31 deletions(-) >> >> diff --git a/src/gallium/auxiliary/tgsi/tgsi_build.c >> b/src/gallium/auxiliary/tgsi/tgsi_build.c >> index 7621b6a..bef5c75 100644 >> --- a/src/gallium/auxiliary/tgsi/tgsi_build.c >> +++ b/src/gallium/auxiliary/tgsi/tgsi_build.c >> @@ -200,7 +200,7 @@ tgsi_default_declaration_interp( void ) >> struct tgsi_declaration_interp di; >> >> di.Interpolate = TGSI_INTERPOLATE_CONSTANT; >> - di.Centroid = 0; >> + di.Location = TGSI_INTERPOLATE_LOC_CENTER; >> di.CylindricalWrap = 0; >> di.Padding = 0; >> >> @@ -209,7 +209,7 @@ tgsi_default_declaration_interp( void ) >> >> static struct tgsi_declaration_interp >> tgsi_build_declaration_interp(unsigned interpolate, >> - unsigned centroid, >> + unsigned interpolate_location, >>unsigned cylindrical_wrap, >>struct tgsi_declaration *declaration, >>struct tgsi_header *header) >> @@ -217,7 +217,7 @@ tgsi_build_declaration_interp(unsigned interpolate, >> struct tgsi_declaration_interp di; >> >> di.Interpolate = interpolate; >> - di.Centroid = centroid; >> + di.Location = interpolate_location; >> di.CylindricalWrap = cylindrical_wrap; >> di.Padding = 0; >> >> @@ -433,7 +433,7 @@ tgsi_build_full_declaration( >>size++; >> >>*di = tgsi_build_declaration_interp(full_decl->Interp.Interpolate, >> - full_decl->Interp.Centroid, >> + full_decl->Interp.Location, >>full_decl->Interp.CylindricalWrap, >>declaration, >>header); >> diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c >> b/src/gallium/auxiliary/tgsi/tgsi_dump.c >> index 8e09bac..884d8cf 100644 >> --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c >> +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c >> @@ -349,8 +349,9 @@ iter_declaration( >> ENM( decl->Interp.Interpolate, tgsi_interpolate_names ); >>} >> >> - if (decl->Interp.Centroid) { >> - TXT( ", CENTROID" ); >> + if (decl->Interp.Location != TGSI_INTERPOLATE_LOC_CENTER) { >> + TXT( ", " ); >> + ENM( decl->Interp.Location, tgsi_interpolate_locations ); >>} >> >>if (decl->Interp.CylindricalWrap) { >> diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.c >> b/src/gallium/auxil
Re: [Mesa-dev] [PATCH 1/3] gallium: switch dedicated centroid field to interpolation location
On Sat, Jul 5, 2014 at 11:20 PM, Roland Scheidegger wrote: > Am 05.07.2014 06:07, schrieb Ilia Mirkin: >> The new location field can be either center, centroid, or sample, which >> indicates the location that the shader should interpolate at. > Looks good though maybe it should be mentioned in the interpolator > section in tgsi.rst (yeah centroid wasn't neither). > In any case, > Reviewed-by: Roland Scheidegger > >> >> Signed-off-by: Ilia Mirkin >> --- >> >> I tried to make sure I hit all the uses, but I guess a bunch of drivers don't >> look at the Centroid field at all? > Well drivers not doing msaa or just fake one like llvmpipe aren't > interested in it. > And, this wasn't exposed via d3d9, though r300 reportedly can do it (the > docs even mention it for r5xx), I suspect using some of the crazy > backdoor mechanisms of d3d9. Some very quick look at the docs though > don't indicate how this could be done (if it's even possible per > interpolator) for these chips, which is I guess why centroid isn't handled. d3d9 does support centroid. I used to use it in HLSL using this syntax: float4 v : TEXCOORD0_CENTROID: The reason centroid isn't supported by r300g is that the hw documentation doesn't say how to enable it. Centroid is also required by GL2. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] gallium: switch dedicated centroid field to interpolation location
Am 05.07.2014 06:07, schrieb Ilia Mirkin: > The new location field can be either center, centroid, or sample, which > indicates the location that the shader should interpolate at. Looks good though maybe it should be mentioned in the interpolator section in tgsi.rst (yeah centroid wasn't neither). In any case, Reviewed-by: Roland Scheidegger > > Signed-off-by: Ilia Mirkin > --- > > I tried to make sure I hit all the uses, but I guess a bunch of drivers don't > look at the Centroid field at all? Well drivers not doing msaa or just fake one like llvmpipe aren't interested in it. And, this wasn't exposed via d3d9, though r300 reportedly can do it (the docs even mention it for r5xx), I suspect using some of the crazy backdoor mechanisms of d3d9. Some very quick look at the docs though don't indicate how this could be done (if it's even possible per interpolator) for these chips, which is I guess why centroid isn't handled. > > src/gallium/auxiliary/tgsi/tgsi_build.c | 8 > src/gallium/auxiliary/tgsi/tgsi_dump.c| 5 +++-- > src/gallium/auxiliary/tgsi/tgsi_scan.c| 2 +- > src/gallium/auxiliary/tgsi/tgsi_scan.h| 2 +- > src/gallium/auxiliary/tgsi/tgsi_strings.c | 7 +++ > src/gallium/auxiliary/tgsi/tgsi_strings.h | 2 ++ > src/gallium/auxiliary/tgsi/tgsi_ureg.c| 12 ++-- > src/gallium/auxiliary/tgsi/tgsi_ureg.h| 2 +- > src/gallium/drivers/ilo/shader/toy_tgsi.c | 2 +- > src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 2 +- > src/gallium/drivers/r600/r600_shader.c| 4 ++-- > src/gallium/drivers/radeonsi/si_shader.c | 6 +++--- > src/gallium/include/pipe/p_shader_tokens.h| 9 +++-- > src/mesa/state_tracker/st_glsl_to_tgsi.cpp| 5 +++-- > src/mesa/state_tracker/st_glsl_to_tgsi.h | 2 +- > src/mesa/state_tracker/st_program.c | 13 + > 16 files changed, 52 insertions(+), 31 deletions(-) > > diff --git a/src/gallium/auxiliary/tgsi/tgsi_build.c > b/src/gallium/auxiliary/tgsi/tgsi_build.c > index 7621b6a..bef5c75 100644 > --- a/src/gallium/auxiliary/tgsi/tgsi_build.c > +++ b/src/gallium/auxiliary/tgsi/tgsi_build.c > @@ -200,7 +200,7 @@ tgsi_default_declaration_interp( void ) > struct tgsi_declaration_interp di; > > di.Interpolate = TGSI_INTERPOLATE_CONSTANT; > - di.Centroid = 0; > + di.Location = TGSI_INTERPOLATE_LOC_CENTER; > di.CylindricalWrap = 0; > di.Padding = 0; > > @@ -209,7 +209,7 @@ tgsi_default_declaration_interp( void ) > > static struct tgsi_declaration_interp > tgsi_build_declaration_interp(unsigned interpolate, > - unsigned centroid, > + unsigned interpolate_location, >unsigned cylindrical_wrap, >struct tgsi_declaration *declaration, >struct tgsi_header *header) > @@ -217,7 +217,7 @@ tgsi_build_declaration_interp(unsigned interpolate, > struct tgsi_declaration_interp di; > > di.Interpolate = interpolate; > - di.Centroid = centroid; > + di.Location = interpolate_location; > di.CylindricalWrap = cylindrical_wrap; > di.Padding = 0; > > @@ -433,7 +433,7 @@ tgsi_build_full_declaration( >size++; > >*di = tgsi_build_declaration_interp(full_decl->Interp.Interpolate, > - full_decl->Interp.Centroid, > + full_decl->Interp.Location, >full_decl->Interp.CylindricalWrap, >declaration, >header); > diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c > b/src/gallium/auxiliary/tgsi/tgsi_dump.c > index 8e09bac..884d8cf 100644 > --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c > +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c > @@ -349,8 +349,9 @@ iter_declaration( > ENM( decl->Interp.Interpolate, tgsi_interpolate_names ); >} > > - if (decl->Interp.Centroid) { > - TXT( ", CENTROID" ); > + if (decl->Interp.Location != TGSI_INTERPOLATE_LOC_CENTER) { > + TXT( ", " ); > + ENM( decl->Interp.Location, tgsi_interpolate_locations ); >} > >if (decl->Interp.CylindricalWrap) { > diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.c > b/src/gallium/auxiliary/tgsi/tgsi_scan.c > index 00fdcfb..563d2c5 100644 > --- a/src/gallium/auxiliary/tgsi/tgsi_scan.c > +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.c > @@ -187,7 +187,7 @@ tgsi_scan_shader(const struct tgsi_token *tokens, >info->input_semantic_name[reg] = (ubyte) semName; >info->input_semantic_index[reg] = (ubyte) semI
[Mesa-dev] [PATCH 1/3] gallium: switch dedicated centroid field to interpolation location
The new location field can be either center, centroid, or sample, which indicates the location that the shader should interpolate at. Signed-off-by: Ilia Mirkin --- I tried to make sure I hit all the uses, but I guess a bunch of drivers don't look at the Centroid field at all? src/gallium/auxiliary/tgsi/tgsi_build.c | 8 src/gallium/auxiliary/tgsi/tgsi_dump.c| 5 +++-- src/gallium/auxiliary/tgsi/tgsi_scan.c| 2 +- src/gallium/auxiliary/tgsi/tgsi_scan.h| 2 +- src/gallium/auxiliary/tgsi/tgsi_strings.c | 7 +++ src/gallium/auxiliary/tgsi/tgsi_strings.h | 2 ++ src/gallium/auxiliary/tgsi/tgsi_ureg.c| 12 ++-- src/gallium/auxiliary/tgsi/tgsi_ureg.h| 2 +- src/gallium/drivers/ilo/shader/toy_tgsi.c | 2 +- src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 2 +- src/gallium/drivers/r600/r600_shader.c| 4 ++-- src/gallium/drivers/radeonsi/si_shader.c | 6 +++--- src/gallium/include/pipe/p_shader_tokens.h| 9 +++-- src/mesa/state_tracker/st_glsl_to_tgsi.cpp| 5 +++-- src/mesa/state_tracker/st_glsl_to_tgsi.h | 2 +- src/mesa/state_tracker/st_program.c | 13 + 16 files changed, 52 insertions(+), 31 deletions(-) diff --git a/src/gallium/auxiliary/tgsi/tgsi_build.c b/src/gallium/auxiliary/tgsi/tgsi_build.c index 7621b6a..bef5c75 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_build.c +++ b/src/gallium/auxiliary/tgsi/tgsi_build.c @@ -200,7 +200,7 @@ tgsi_default_declaration_interp( void ) struct tgsi_declaration_interp di; di.Interpolate = TGSI_INTERPOLATE_CONSTANT; - di.Centroid = 0; + di.Location = TGSI_INTERPOLATE_LOC_CENTER; di.CylindricalWrap = 0; di.Padding = 0; @@ -209,7 +209,7 @@ tgsi_default_declaration_interp( void ) static struct tgsi_declaration_interp tgsi_build_declaration_interp(unsigned interpolate, - unsigned centroid, + unsigned interpolate_location, unsigned cylindrical_wrap, struct tgsi_declaration *declaration, struct tgsi_header *header) @@ -217,7 +217,7 @@ tgsi_build_declaration_interp(unsigned interpolate, struct tgsi_declaration_interp di; di.Interpolate = interpolate; - di.Centroid = centroid; + di.Location = interpolate_location; di.CylindricalWrap = cylindrical_wrap; di.Padding = 0; @@ -433,7 +433,7 @@ tgsi_build_full_declaration( size++; *di = tgsi_build_declaration_interp(full_decl->Interp.Interpolate, - full_decl->Interp.Centroid, + full_decl->Interp.Location, full_decl->Interp.CylindricalWrap, declaration, header); diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c b/src/gallium/auxiliary/tgsi/tgsi_dump.c index 8e09bac..884d8cf 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c @@ -349,8 +349,9 @@ iter_declaration( ENM( decl->Interp.Interpolate, tgsi_interpolate_names ); } - if (decl->Interp.Centroid) { - TXT( ", CENTROID" ); + if (decl->Interp.Location != TGSI_INTERPOLATE_LOC_CENTER) { + TXT( ", " ); + ENM( decl->Interp.Location, tgsi_interpolate_locations ); } if (decl->Interp.CylindricalWrap) { diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.c b/src/gallium/auxiliary/tgsi/tgsi_scan.c index 00fdcfb..563d2c5 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_scan.c +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.c @@ -187,7 +187,7 @@ tgsi_scan_shader(const struct tgsi_token *tokens, info->input_semantic_name[reg] = (ubyte) semName; info->input_semantic_index[reg] = (ubyte) semIndex; info->input_interpolate[reg] = (ubyte)fulldecl->Interp.Interpolate; - info->input_centroid[reg] = (ubyte)fulldecl->Interp.Centroid; + info->input_interpolate_loc[reg] = (ubyte)fulldecl->Interp.Location; info->input_cylindrical_wrap[reg] = (ubyte)fulldecl->Interp.CylindricalWrap; info->num_inputs++; diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.h b/src/gallium/auxiliary/tgsi/tgsi_scan.h index 0be2feb..1869b41 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_scan.h +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.h @@ -45,7 +45,7 @@ struct tgsi_shader_info ubyte input_semantic_name[PIPE_MAX_SHADER_INPUTS]; /**< TGSI_SEMANTIC_x */ ubyte input_semantic_index[PIPE_MAX_SHADER_INPUTS]; ubyte input_interpolate[PIPE_MA