[Mesa-dev] [PATCH 1/3] draw: cleanup the extra attribs

Stéphane Marchesin stephane.marchesin at gmail.com
Wed Sep 4 14:39:27 PDT 2013


On Wed, Sep 4, 2013 at 2:33 PM, Stéphane Marchesin <
stephane.marchesin at gmail.com> wrote:

>
>
>
> On Wed, Sep 4, 2013 at 12:46 PM, Zack Rusin <zackr at vmware.com> wrote:
>
>> Hi, Stéphane.
>>
>> No we should not revert to the old behavior. The old behavior was
>> incorrect. Consider this:
>>
>> -- setup state that draws a wireframe -> draw should inject frontface
>> -- the driver needs to be able to find the injected wireframe output
>> -- draw
>> -- setup state the draws solid fill with fragment shader using primid
>> input -> draw should inject primid but not frontface
>> -- driver needs to be able to find the injected primid but not frontface
>> info
>> -- draw
>>
>> Without cleaning the attributed before the second draw the draw will keep
>> the frontface id in the extra attribs, incorrectly pointing the driver to a
>> non-existing crash. That's why the attribs need to be cleaned before
>> rendering.
>>
>
> Hmm good point. I don't care about primid at all in i915 so it didn't
> occur to me :)
>
>
>>
>> i915g simply shouldn't call draw_prepare_shader_outputs because it
>> doesn't know what to do with the injected front-face or primid anyway. That
>> part I'd suggest you remove. It will get you back to the old behavior.
>>
>
> Yup that does the trick, thanks!
>
>
Hmm, it seems like we should do the same for r300, as it doesn't use PRIMID
either. CC'd Marek.

Stéphane


> Stéphane
>
>
>>
>> z
>>
>> ------------------------------
>>
>> Hi Zack,
>>
>> This change regresses a bunch of point sprite piglit tests on i915g.
>> Should we revert back to the old behaviour? As far as I can see, it was
>> correct (it was keeping the attributes in case another stage is using them).
>>
>> Stéphane
>>
>>
>>
>> On Thu, Aug 8, 2013 at 12:46 PM, Zack Rusin <zackr at vmware.com> wrote:
>>
>>> Before inserting new front face and prim id outputs cleanup
>>> the old extra outputs, otherwise our cache will use previous
>>> output slots which will break as soon as outputs of the current
>>> shader don't match the last.
>>>
>>> Signed-off-by: Zack Rusin <zackr at vmware.com>
>>> ---
>>>  src/gallium/auxiliary/draw/draw_context.c |    1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/src/gallium/auxiliary/draw/draw_context.c
>>> b/src/gallium/auxiliary/draw/draw_context.c
>>> index af9caee..2dc6772 100644
>>> --- a/src/gallium/auxiliary/draw/draw_context.c
>>> +++ b/src/gallium/auxiliary/draw/draw_context.c
>>> @@ -555,6 +555,7 @@ draw_get_shader_info(const struct draw_context *draw)
>>>  void
>>>  draw_prepare_shader_outputs(struct draw_context *draw)
>>>  {
>>> +   draw_remove_extra_vertex_attribs(draw);
>>>     draw_ia_prepare_outputs(draw, draw->pipeline.ia);
>>>     draw_unfilled_prepare_outputs(draw, draw->pipeline.unfilled);
>>>  }
>>> --
>>> 1.7.10.4
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130904/477e9a6b/attachment.html>


More information about the mesa-dev mailing list