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

[Ian Romanick]

-----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?

> That said, I don't think nouveau is ever going to care in this 
> particular case, I don't mind if you want to get rid of them.  The 
> following patches:
> 
> [PATCH 1/2] nouveau: Use _NEW_VIEWPORT instead of hooking through
> dd_function_table [PATCH 2/2] nouveau: Use _NEW_SCISSOR instead of
> hooking through dd_function_table
> 
> are Reviewed-by: Francisco Jerez <currojerez at riseup.net>.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (GNU/Linux)

iEYEARECAAYFAlJ36tgACgkQX1gOwKyEAw+M6wCgkdljRbs5qESfZMmz4TycebY/
ypoAn0Hw3OtNgYcMfR1dBD5qomrsFHO9
=wRW3
-----END PGP SIGNATURE-----