On Wed, Aug 29, 2012 at 6:16 PM, Paul Berry <stereotype...@gmail.com> wrote: > On 29 August 2012 16:16, Brian Paul <bri...@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. BTW, bug 54234 reports this issue too. -Brian _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
