On 01/06/2012 04:35 PM, Brian Paul wrote: > On 01/06/2012 08:26 AM, Dave Airlie wrote: >> On Fri, Jan 6, 2012 at 3:13 PM, Brian Paul<[email protected]> wrote: >>> On 01/06/2012 06:57 AM, Dave Airlie wrote: >>>> >>>> From: Dave Airlie<[email protected]> >>>> >>>> Brian mentioned that mesa-demos/reflect was broken on softpipe, >>>> by my previous commit. The problem was were blindly translating none >>>> to perspective, when color/pntc at least need it linear. >>>> >>>> v2: no regressions version. >>>> use shademodel to pick what none means. >>>> >>>> Signed-off-by: Dave Airlie<[email protected]> >>>> --- >>>> src/mesa/state_tracker/st_program.c | 19 ++++++++++++------- >>>> 1 files changed, 12 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/src/mesa/state_tracker/st_program.c >>>> b/src/mesa/state_tracker/st_program.c >>>> index 146a9f3..1f6094e 100644 >>>> --- a/src/mesa/state_tracker/st_program.c >>>> +++ b/src/mesa/state_tracker/st_program.c >>>> @@ -436,10 +436,14 @@ st_get_vp_variant(struct st_context *st, >>>> >>>> >>>> static unsigned >>>> -st_translate_interp(enum glsl_interp_qualifier glsl_qual) >>>> +st_translate_interp(enum glsl_interp_qualifier glsl_qual, bool >>>> is_color, >>>> GLenum shade_model) >>>> { >>>> switch (glsl_qual) { >>>> case INTERP_QUALIFIER_NONE: >>>> + if (is_color) >>>> + if (shade_model == GL_FLAT) >>>> + return TGSI_INTERPOLATE_LINEAR; >>>> + return TGSI_INTERPOLATE_PERSPECTIVE; >>> >>> >>> This doesn't look right. If shade_mode == GL_FLAT, shouldn't we return >>> TGSI_INTERPOLATE_CONSTANT? >> >> Yeah the code is very wrong, I was confused by the fact that softpipe >> perspective interp is broken and some piglit results. >> >>>> case INTERP_QUALIFIER_SMOOTH: >>>> return TGSI_INTERPOLATE_PERSPECTIVE; >>>> case INTERP_QUALIFIER_FLAT: >>>> @@ -542,12 +546,14 @@ st_translate_fragment_program(struct >>>> st_context *st, >>>> case FRAG_ATTRIB_COL0: >>>> input_semantic_name[slot] = TGSI_SEMANTIC_COLOR; >>>> input_semantic_index[slot] = 0; >>>> - interpMode[slot] = >>>> st_translate_interp(stfp->Base.InterpQualifier[attr]); >>>> + interpMode[slot] = >>>> st_translate_interp(stfp->Base.InterpQualifier[attr], >>>> + TRUE, >>>> st->ctx->Light.ShadeModel); >>>> break; >>>> case FRAG_ATTRIB_COL1: >>>> input_semantic_name[slot] = TGSI_SEMANTIC_COLOR; >>>> input_semantic_index[slot] = 1; >>>> - interpMode[slot] = >>>> st_translate_interp(stfp->Base.InterpQualifier[attr]); >>>> + interpMode[slot] = >>>> st_translate_interp(stfp->Base.InterpQualifier[attr], >>>> + TRUE, >>>> st->ctx->Light.ShadeModel); >>>> break; >>>> case FRAG_ATTRIB_FOGC: >>>> input_semantic_name[slot] = TGSI_SEMANTIC_FOG; >>>> @@ -601,10 +607,9 @@ st_translate_fragment_program(struct st_context >>>> *st, >>>> assert(attr>= FRAG_ATTRIB_TEX0); >>>> input_semantic_index[slot] = (attr - >>>> FRAG_ATTRIB_TEX0); >>>> input_semantic_name[slot] = TGSI_SEMANTIC_GENERIC; >>>> - if (attr == FRAG_ATTRIB_PNTC) >>>> - interpMode[slot] = TGSI_INTERPOLATE_LINEAR; >>>> - else >>>> - interpMode[slot] = >>>> st_translate_interp(stfp->Base.InterpQualifier[attr]); >>>> + interpMode[slot] = >>>> st_translate_interp(stfp->Base.InterpQualifier[attr], >>>> + (attr == >>>> FRAG_ATTRIB_PNTC), >>>> + >>>> st->ctx->Light.ShadeModel); >>> >>> >>> The ShadeModel value should only apply to color attibutes so it >>> shouldn't >>> appear here in the texcoords/generic/point-coord case. >>> >>> I think the code should read: >>> >>> if (attr == FRAG_ATTRIB_PNTC) >>> interpMode[slot] = TGSI_INTERPOLATE_LINEAR; >>> else >>> interpMode[slot] = >>> st_translate_interp((stfp->Base.InterpQualifier[attr], false, 0); >> >> Yeah I'll probably just commit v1 + that change. >> >> then I'll try and figure why softpipe gives different answer for >> perspective than everyone else. >> >> Dave. > > Looking forward, I think we'll eventually want to remove the > pipe_rasterizer_state::flatshade field and always use the fragment > shader interpolation qualifiers. > > This would mean that if a shader was used both with > glShadeModel(GL_FLAT) and GL_SMOOTH we'd wind up with two variants of > the shader, but that should be rare. >
But all the radeon and NV GPUs have the shade model switch built-in, they don't need 2 shader variants ... > -Brian > _______________________________________________ > mesa-dev mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
