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

[Brian Paul]

On 11/19/2013 04:53 PM, Courtney Goeltzenleuchter wrote:
> The Gallium state tracker has a st_viewport that it puts into
> driver->Viewport function table.
>
> It's not clear to me how _NEW_VIEWPORT replaces the function of the
> Driver->Viewport call.
>
> Is it really safe to remove the driver->Viewport call for Gallium?

For multiple viewports, I think you only have to be concerned with the 
st_atom_viewport.c code.  Have you looked at that?

-Brian


>
> Courtney
>
>
> On Mon, Nov 4, 2013 at 12:31 PM, Brian Paul <brianp at vmware.com
> <mailto:brianp at vmware.com>> wrote:
>
>     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
>             <mailto: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
>
>
>     _________________________________________________
>     mesa-dev mailing list
>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     http://lists.freedesktop.org/__mailman/listinfo/mesa-dev
>     <https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/mesa-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=lGQMzzTgII0I7jefp2FHq7WtZ%2BTLs8wadB%2BiIj9xpBY%3D%0A&m=VA9eAusZ3%2F58PQhMUXx1cXtP63BThETAT9Sz8PNRq0o%3D%0A&s=5f6224c40a28fafa01d3c8134d9417d64470d4ee9013c10642127799ff93258d>
>
>
>
>
> --
> Courtney Goeltzenleuchter
> LunarG
>