[Mesa-dev] problem with "meta: Don't stray outside the confines of the API specified in the context"

[Brian Paul]

On 08/29/2012 09:09 PM, Brian Paul wrote:
> On Wed, Aug 29, 2012 at 6:16 PM, Paul Berry<stereotype441 at gmail.com>  wrote:
>> On 29 August 2012 16:16, Brian Paul<brianp at vmware.com>  wrote:
>>>
>>> In enable.c there's a call to _mesa_meta_in_progress().  But the meta code
>>> is not always built or linked in when building gallium targets.
>>>
>>> For now, I think I can put a _mesa_meta_in_progress() dummy function in
>>> the state_tracker code.  But that's a pretty ugly hack.
>>>
>>> What's the reason for calling _mesa_meta_in_progress() for
>>> glEnable/Disable(GL_MULTISAMPLE_ARB)?
>>>
>>> -Brian
>>
>>
>> The difficulty here is that the clear meta-op needs to be able to disable
>> GL_MULTISAMPLE_ARB in order to ensure that GL_SAMPLE_ALPHA_TO_COVERAGE,
>> GL_SAMPLE_ALPHA_TO_ONE, and GL_SAMPLE_COVERAGE are all properly ignored
>> during clear operations.  However, GL_MULTISAMPLE_ARB does not exist in the
>> GLES API (GLES behaves as though it is always enabled).  Calling
>> _mesa_meta_in_progress allows Meta to disable GL_MULTISAMPLE_ARB even when
>> the API would ordinarily prohibit it.
>>
>> I agree that putting a dummy _mesa_meta_in_progress() function in the state
>> tracker is an ugly hack.
>>
>> Here are some possible alternatives:
>>
>> 1. We could simply drop the call to _mesa_meta_in_progress(), and let
>> glEnable/Disable(GL_MULTISAMPLE_ARB) always succeed, regardless of which API
>> is in use.  But this also seems like an ugly hack, because it makes Mesa
>> deliberately non-compliant with the GLES spec.
>>
>> 2. Is there a #define we could consult to determine whether the driver
>> currently being compiled includes Meta?  If so, we could use an ifdef to
>> avoid the call to _mesa_meta_in_progress() for drivers that don't support
>> Meta (those drivers would then prohibit GL_MULTISAMPLE_ARB in GLES mode,
>> consistent with the GLES spec).  I think this would be the cleanest
>> solution.
>>
>> 3. We could make a backdoor function that enables or disables
>> GL_MULTISAMPLE_ARB without doing error checking, and have Meta call the
>> backdoor function instead of _mesa_set_enable().   This seems slightly less
>> clean than option 2 because it involves some code duplication, but it's not
>> too bad.
>>
>> 4. We could change Meta so that instead of enabling/disabling
>> GL_MULTISAMPLE_ARB it saves and restores the state of
>> GL_SAMPLE_ALPHA_TO_COVERAGE, GL_SAMPLE_ALPHA_TO_ONE, and GL_SAMPLE_COVERAGE.
>> (It would have to skip saving/restoring of GL_SAMPLE_ALPHA_TO_ONE in GLES
>> mode, since that enum doesn't exist in GLES).  This would have the advantage
>> that core Mesa wouldn't have to do anything special to account for the needs
>> of Meta, at the expense of adding a minor amount of complexity to Meta (Meta
>> has to save the state of 2 or 3 enables, depending on API, rather than just
>> 1).
>>
>> My personal preference is slightly in favor of 2, but I could be talked into
>> 3 or 4.  I'd really rather avoid option 1, since strictly speaking it's
>> non-compliant.
>>
>> (Cc'ing Ian since I'm guessing he has an opinion about this too)
>
> I'd go with something like #3.  Just add a _meta_set_enable() function
> which accepts GL_MULTISAMPLE_ARB and does what _mesa_set_enable() does
> for that case, minus the error checking.  There might be other
> enable/disables cases in the future that also require special
> handling.  So far, the meta code does almost all state changes via
> _mesa_foo() calls but that's not a requirement.

I've got patch that does this.  I'll post it for review.

-Brian