[Mesa-dev] [PATCH] mesa: Change driver interface for ARB_viewport_array

[Brian Paul]

On 11/04/2013 11:43 AM, Ian Romanick wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 11/01/2013 04:12 PM, Francisco Jerez wrote:
>> Ian Romanick <idr at freedesktop.org> writes:
>>
>>> On 11/01/2013 02:04 PM, Courtney Goeltzenleuchter wrote:
>>>> [...]
>>> More often, the dd_function_table functions allow core Mesa to
>>> signal the driver driver that something happened... and the
>>> driver may need to do something in response.  For DRI2 drivers,
>>> the Viewport function is a good example.  DRI2 drivers use that
>>> signal as a hint that the window may have been resized.
>>>
>>> Other dd_function_table functions are used by core Mesa to
>>> request the driver create or destroy some resource (e.g., texture
>>> storage).
>>>
>>> If it weren't for the way nouveau used the DepthRange and Scissor
>>> hooks, I think we could just about get rid of them altogether.  I
>>> wish that driver just used the dirty state bits like everyone
>>> else. :(
>>
>> Cases like the new dd_function_table::Scissor and ::Viewport hooks
>> introduced in this patch series are the reason why nouveau tends
>> to prefer overriding the dd_function_table to keep track of state
>> changes rather than looking at the ctx->NewState bits, because the
>> latter tend to be very coarse-grained -- e.g. there's one big
>> _NEW_TEXTURE flag for all the state of all texture units while
>> nouveau is able to update a subset of the texture state
>> independently for only those texture units that have changed.
>>
>> With the dd_function_table interface proposed in this patch series
>> it would be possible for the drivers to update the state of each
>> viewport in the viewport array independently, which might be
>> beneficial for some hardware someday, removing ::DepthRange and
>> ::Scissor would preclude that possibility.
>
> Right... I wonder if we might be better of just tracking a bit per
> viewport in the gl_context.  I'm assuming we'll end up with something
> like:
>
>      struct gl_viewport_attrib Viewports[MAX_VIEWPORTS];
>
> in the gl_context.  It would be easy to add
>
>      GLbitfield  _DirtyViewports;
>
> along side it.  The various Viewport and DepthRange functions would
> set bits in that field along with _NEW_VIEWPORT.  Drivers that care
> could examine (and clear) those bits.
>
> We'd do similar for Scissor.
>
> Looking at the i965 hardware (and our driver architecture), I believe
> we have to upload all of the viewports anytime there's a change
> anyway.  The viewports are just stored as an array in a BO.  See
> gen7_upload_sf_clip_viewport and similar functions.
>
> Marek:  Do you know what Radeon / Gallium want?

The gallium interface takes a start,count array of viewports.  The 
st/mesa state tracker could use the bitfield to determine the changed 
range.  But we also have the CSO module to help filter out redundant 
state changes.

-Brian