On Fri, Jan 6, 2012 at 4:50 PM, Christoph Bumiller
<e0425...@student.tuwien.ac.at> wrote:
> 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<bri...@vmware.com> wrote:
>>>> On 01/06/2012 06:57 AM, Dave Airlie wrote:
>>>>>
>>>>> From: Dave Airlie<airl...@redhat.com>
>>>>>
>>>>> 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<airl...@redhat.com>
>>>>> ---
>>>>> 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 ...
I agree with this point. r300g ignores the TGSI interpolate modes,
because such state doesn't exist in hardware. It's a GL3 feature. All
I can do is to follow pipe_rasterizer_state::flatshade.
Marek
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
