[Mesa-dev] [PATCH] meta: remove call to _meta_in_progress(), fix multisample enable/disable

[Brian Paul]

On 08/30/2012 10:57 AM, Paul Berry wrote:
> On 30 August 2012 09:49, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
>
>     On 08/30/2012 08:14 AM, Paul Berry wrote:
>
>         From: Brian Paul <brianp at vmware.com <mailto:brianp at vmware.com>>
>
>         This partially reverts d638da23d2ec2e9c52655b1ea13824__9e7f8bcccb.
>
>         With gallium the meta code is not always built so the call to
>         _meta_in_progress() was unresolved.  Simply special-case the
>         GL_MULTISAMPLE case in the meta code.  There might be other
>         special
>         cases in the future given all the differences between legacy GL,
>         core GL, GLES, etc.
>
>         Fixes https://bugs.freedesktop.org/__show_bug.cgi?id=54234
>         <https://bugs.freedesktop.org/show_bug.cgi?id=54234>
>         and https://bugs.freedesktop.org/__show_bug.cgi?id=54239
>         <https://bugs.freedesktop.org/show_bug.cgi?id=54239>
>
>         v2 (Paul Berry <stereotype441 at gmail.com
>         <mailto:stereotype441 at gmail.com>>): keep _meta_in_progress
>         function, since it's needed by the i965 driver, but don't call
>         it from
>         core mesa.
>         ---
>            src/mesa/drivers/common/meta.c |   33
>         ++++++++++++++++++++++++++++++__+--
>            src/mesa/main/enable.c         |    7 +------
>            2 files changed, 32 insertions(+), 8 deletions(-)
>
>         diff --git a/src/mesa/drivers/common/__meta.c
>         b/src/mesa/drivers/common/__meta.c
>         index abb7d4e..e38759f 100644
>         --- a/src/mesa/drivers/common/__meta.c
>         +++ b/src/mesa/drivers/common/__meta.c
>         @@ -438,6 +438,35 @@ _mesa_meta_free(struct gl_context *ctx)
>
>
>            /**
>         + * This is an alternative to _mesa_set_enable() to handle
>         some special cases.
>         + * See comments inside.
>         + */
>         +static void
>         +meta_set_enable(struct gl_context *ctx, GLenum cap, GLboolean
>         state)
>         +{
>         +   switch (cap) {
>         +   case GL_MULTISAMPLE:
>         +      /* We need to enable/disable multisample when using
>         GLES but this enum
>         +       * is not supported there.
>         +       */
>         +      if (ctx->Multisample.Enabled == state)
>         +         return;
>         +      FLUSH_VERTICES(ctx, _NEW_MULTISAMPLE);
>         +      ctx->Multisample.Enabled = state;
>         +      break;
>         +   default:
>         +      _mesa_problem(ctx, "Unexpected cap in _meta_set_enable()");
>         +      return;
>         +   }
>         +
>         +   if (ctx->Driver.Enable) {
>         +      ctx->Driver.Enable(ctx, cap, state);
>         +   }
>         +}
>
>
>     I'm not a fan of this ugly layering violation.  Duplicating this
>     logic across multiple places is just asking for trouble.  I'd prefer:
>
>     1. Just allow GL_MULTISAMPLE to be modified regardless of API
>     (what I had originally done).
>
>     2. Add a _mesa_set_multisample in enable.c that is used from meta
>     and _mesa_set_enable.
>
>
> I'd be glad to make a follow-up patch that does #2.  What do you
> think, Brian?

That's OK with me too.

-Brian