[Mesa-dev] GLES3 dispatch table creation issue

[Brian Paul]

On 11/13/2012 01:52 PM, Chad Versace wrote:
> Hi mesa-dev,
>
> The Intel team has been discussing how fix population of the
> GLES3 dispatch table. The fix may require a modification of the data
> flow during context creation for all drivers. I just wanted to
> post the discussion here for public record.
>
> On 11/13/2012 12:21 PM, Kenneth Graunke wrote:
>> On 11/13/2012 11:55 AM, Chad Versace wrote:
>>> CC'ing mesa-dev, with some private bits removed.
>>>
>>> On 11/13/2012 09:57 AM, Paul Berry wrote:
>>>> On 13 November 2012 09:16, Jordan Justen<jljusten at gmail.com
>>>> <mailto:jljusten at gmail.com>>  wrote:
>>>>
>>>>       On Mon, Nov 12, 2012 at 10:18 PM, Paul Berry<stereotype441 at gmail.com
>>>>       <mailto:stereotype441 at gmail.com>>  wrote:
>>>>       >  Ideally I'd like to change the initialization order so that
>>>>       >  _mesa_create_exec_table happens after _mesa_compute_version.  Ian and
>>>> I have
>>>>       >  been planning to make this change in the long run for other reasons
>>>> (so that
>>>>       >  _mesa_create_exec_table can avoid populating dispatch table entries for
>>>>       >  unsupported extensions).  What I don't know is whether other yaks have
>>>> to be
>>>>       >  shaved before we can make that change.
>>>>
>>>>       I'm not sure, but I think we currently have:
>>>>       1. Create the core dispatch tables
>>>>       2. Return back to mesa driver
>>>>       3. Return to app code and wait for context to be made current
>>>>       4. Compute version
>>>>
>>>>       It seems like a lot could be happening between #1 and #4 that would
>>>>       make it challenging to move #1 after #4.
>>>>
>>>>
>>>> Wow, I had no idea.  I don't think it's necessary to postpone computing the
>>>> version until this late--it should be possible to do during context creation.
>>>> In fact, several of the other drivers do exactly that: Nouveau, r200, and radeon
>>>> all call _mesa_compute_version unconditionally from their context creation
>>>> functionsm and the gallium state tracker calls _mesa_compute_version from its
>>>> context creation function when the client requests an API version>  1.0.  I
>>>> think that means that the only other driver that does stuff in the same order we
>>>> do is swrast.
>>>>
>>>> I propose that we switch to what the other drivers do--just call
>>>> _mesa_compute_version unconditionally in intelInitContext.  As long as we do it
>>>> after the call to intelInitExtensions we should be fine.
>>>>
>>>> As a follow up task, we should see if it's feasible to modify swrast to do the
>>>> same, and modify gallium so that it calls _mesa_compute_version
>>>> unconditionally.  If that works, then we can drop the call to
>>>> _mesa_compute_version from _mesa_make_current, since all drivers will have
>>>> already called _mesa_compute_version by the time their context is made current
>>>> for the first time.
>>>
>>> I agree. The Intel drivers and swrast should call _mesa_compute_version during
>>> context creation, just like the gallium drivers. Then the call to
>>> _mesa_compute_version should be removed from *MakeCurrent.
>>>
>>> Not only does _mesa_compute_version need to be called during context creation,
>>> but it must be called from within or before _mesa_initialize_context, because
>>> that function initializes the dispatch tables. Therefore, I'd like to go one
>>> step further: the responsibility of calling _mesa_compute_version
>>> should be moved outside the drivers and into _mesa_initialize_context. Doing so
>>> would require some modifications to the context creation data flow, but Jordan
>>> and my investigations determined that it wouldn't be too difficult.
>>
>> Seems like a fine plan.

Sounds OK to me.  Just be sure there's some comments indicating the 
expected order of events.  And if possible, add some assertions to 
make sure things are done in the right order (such as 
assert(ctx->VersionMajor != 0) when creating the dispatch table).

-Brian