[mutter] cogl: Remove viewport scissor workaround



commit b624e94ab1032a5a811548bc17572dc32526833f
Author: Adam Jackson <ajax redhat com>
Date:   Wed Mar 6 12:34:36 2019 -0500

    cogl: Remove viewport scissor workaround
    
    This is effectively a revert of:
    
        commit 6cfc93f26f64c506922bf119d5079d83de7543d2
        Author: Robert Bragg <robert linux intel com>
        Date:   Tue Oct 2 11:44:00 2012 +0100
    
            clip-stack: workaround intel gen6 viewport clip bug
    
    It's been over six years, if this bug is still present we should just
    fix Mesa already.
    
    https://gitlab.gnome.org/GNOME/mutter/merge_requests/481

 cogl/cogl/cogl-context-private.h         |  3 --
 cogl/cogl/cogl-context.c                 | 16 --------
 cogl/cogl/cogl-framebuffer.c             | 69 +-------------------------------
 cogl/cogl/driver/gl/cogl-clip-stack-gl.c | 36 +----------------
 4 files changed, 4 insertions(+), 120 deletions(-)
---
diff --git a/cogl/cogl/cogl-context-private.h b/cogl/cogl/cogl-context-private.h
index 9fd5b57d4..35e5faa01 100644
--- a/cogl/cogl/cogl-context-private.h
+++ b/cogl/cogl/cogl-context-private.h
@@ -103,9 +103,6 @@ struct _CoglContext
   unsigned long private_features
     [COGL_FLAGS_N_LONGS_FOR_SIZE (COGL_N_PRIVATE_FEATURES)];
 
-  gboolean needs_viewport_scissor_workaround;
-  CoglFramebuffer *viewport_scissor_workaround_framebuffer;
-
   CoglPipeline *default_pipeline;
   CoglPipelineLayer *default_layer_0;
   CoglPipelineLayer *default_layer_n;
diff --git a/cogl/cogl/cogl-context.c b/cogl/cogl/cogl-context.c
index aac25f2d2..6fffda18f 100644
--- a/cogl/cogl/cogl-context.c
+++ b/cogl/cogl/cogl-context.c
@@ -265,22 +265,6 @@ cogl_context_new (CoglDisplay *display,
   /* Initialise the driver specific state */
   _cogl_init_feature_overrides (context);
 
-  /* XXX: ONGOING BUG: Intel viewport scissor
-   *
-   * Intel gen6 drivers don't currently correctly handle offset
-   * viewports, since primitives aren't clipped within the bounds of
-   * the viewport.  To workaround this we push our own clip for the
-   * viewport that will use scissoring to ensure we clip as expected.
-   *
-   * TODO: file a bug upstream!
-   */
-  if (context->gpu.driver_package == COGL_GPU_INFO_DRIVER_PACKAGE_MESA &&
-      context->gpu.architecture == COGL_GPU_INFO_ARCHITECTURE_SANDYBRIDGE &&
-      !getenv ("COGL_DISABLE_INTEL_VIEWPORT_SCISSORT_WORKAROUND"))
-    context->needs_viewport_scissor_workaround = TRUE;
-  else
-    context->needs_viewport_scissor_workaround = FALSE;
-
   context->sampler_cache = _cogl_sampler_cache_new (context);
 
   _cogl_pipeline_init_default_pipeline ();
diff --git a/cogl/cogl/cogl-framebuffer.c b/cogl/cogl/cogl-framebuffer.c
index bd8a7fa42..bc10a8ce5 100644
--- a/cogl/cogl/cogl-framebuffer.c
+++ b/cogl/cogl/cogl-framebuffer.c
@@ -191,9 +191,6 @@ _cogl_framebuffer_free (CoglFramebuffer *framebuffer)
 
   cogl_object_unref (framebuffer->journal);
 
-  if (ctx->viewport_scissor_workaround_framebuffer == framebuffer)
-    ctx->viewport_scissor_workaround_framebuffer = NULL;
-
   ctx->framebuffers = g_list_remove (ctx->framebuffers, framebuffer);
 
   if (ctx->current_draw_buffer == framebuffer)
@@ -260,13 +257,11 @@ cogl_framebuffer_clear4f (CoglFramebuffer *framebuffer,
                           float blue,
                           float alpha)
 {
-  CoglContext *ctx = framebuffer->context;
   CoglClipStack *clip_stack = _cogl_framebuffer_get_clip_stack (framebuffer);
   int scissor_x0;
   int scissor_y0;
   int scissor_x1;
   int scissor_y1;
-  gboolean saved_viewport_scissor_workaround;
 
   if (!framebuffer->depth_buffer_clear_needed &&
       (buffers & COGL_BUFFER_BIT_DEPTH))
@@ -361,31 +356,6 @@ cogl_framebuffer_clear4f (CoglFramebuffer *framebuffer,
 
   _cogl_framebuffer_flush_journal (framebuffer);
 
-  /* XXX: ONGOING BUG: Intel viewport scissor
-   *
-   * The semantics of cogl_framebuffer_clear() are that it should not
-   * be affected by the current viewport and so if we are currently
-   * applying a workaround for viewport scissoring we need to
-   * temporarily disable the workaround before clearing so any
-   * special scissoring for the workaround will be removed first.
-   *
-   * Note: we only need to disable the workaround if the current
-   * viewport doesn't match the framebuffer's size since otherwise
-   * the workaround wont affect clearing anyway.
-   */
-  if (ctx->needs_viewport_scissor_workaround &&
-      (framebuffer->viewport_x != 0 ||
-       framebuffer->viewport_y != 0 ||
-       framebuffer->viewport_width != framebuffer->width ||
-       framebuffer->viewport_height != framebuffer->height))
-    {
-      saved_viewport_scissor_workaround = TRUE;
-      ctx->needs_viewport_scissor_workaround = FALSE;
-      ctx->current_draw_buffer_changes |= COGL_FRAMEBUFFER_STATE_CLIP;
-    }
-  else
-    saved_viewport_scissor_workaround = FALSE;
-
   /* NB: _cogl_framebuffer_flush_state may disrupt various state (such
    * as the pipeline state) when flushing the clip stack, so should
    * always be done first when preparing to draw. */
@@ -395,16 +365,6 @@ cogl_framebuffer_clear4f (CoglFramebuffer *framebuffer,
   _cogl_framebuffer_clear_without_flush4f (framebuffer, buffers,
                                            red, green, blue, alpha);
 
-  /* XXX: ONGOING BUG: Intel viewport scissor
-   *
-   * See comment about temporarily disabling this workaround above
-   */
-  if (saved_viewport_scissor_workaround)
-    {
-      ctx->needs_viewport_scissor_workaround = TRUE;
-      ctx->current_draw_buffer_changes |= COGL_FRAMEBUFFER_STATE_CLIP;
-    }
-
   /* This is a debugging variable used to visually display the quad
    * batches from the journal. It is reset here to increase the
    * chances of getting the same colours for each frame during an
@@ -552,12 +512,7 @@ cogl_framebuffer_set_viewport (CoglFramebuffer *framebuffer,
   framebuffer->viewport_age++;
 
   if (context->current_draw_buffer == framebuffer)
-    {
-      context->current_draw_buffer_changes |= COGL_FRAMEBUFFER_STATE_VIEWPORT;
-
-      if (context->needs_viewport_scissor_workaround)
-        context->current_draw_buffer_changes |= COGL_FRAMEBUFFER_STATE_CLIP;
-    }
+    context->current_draw_buffer_changes |= COGL_FRAMEBUFFER_STATE_VIEWPORT;
 }
 
 float
@@ -831,27 +786,7 @@ _cogl_framebuffer_compare_viewport_state (CoglFramebuffer *a,
       /* NB: we render upside down to offscreen framebuffers and that
        * can affect how we setup the GL viewport... */
       a->type != b->type)
-    {
-      unsigned long differences = COGL_FRAMEBUFFER_STATE_VIEWPORT;
-      CoglContext *context = a->context;
-
-      /* XXX: ONGOING BUG: Intel viewport scissor
-       *
-       * Intel gen6 drivers don't currently correctly handle offset
-       * viewports, since primitives aren't clipped within the bounds of
-       * the viewport.  To workaround this we push our own clip for the
-       * viewport that will use scissoring to ensure we clip as expected.
-       *
-       * This workaround implies that a change in viewport state is
-       * effectively also a change in the clipping state.
-       *
-       * TODO: file a bug upstream!
-       */
-      if (G_UNLIKELY (context->needs_viewport_scissor_workaround))
-          differences |= COGL_FRAMEBUFFER_STATE_CLIP;
-
-      return differences;
-    }
+    return COGL_FRAMEBUFFER_STATE_VIEWPORT;
   else
     return 0;
 }
diff --git a/cogl/cogl/driver/gl/cogl-clip-stack-gl.c b/cogl/cogl/driver/gl/cogl-clip-stack-gl.c
index 78eddcc9d..f8a471828 100644
--- a/cogl/cogl/driver/gl/cogl-clip-stack-gl.c
+++ b/cogl/cogl/driver/gl/cogl-clip-stack-gl.c
@@ -435,12 +435,7 @@ _cogl_clip_stack_gl_flush (CoglClipStack *stack,
      anything */
   if (ctx->current_clip_stack_valid)
     {
-      if (ctx->current_clip_stack == stack &&
-          (ctx->needs_viewport_scissor_workaround == FALSE ||
-           (framebuffer->viewport_age ==
-            framebuffer->viewport_age_for_scissor_workaround &&
-            ctx->viewport_scissor_workaround_framebuffer ==
-            framebuffer)))
+      if (ctx->current_clip_stack == stack)
         return;
 
       _cogl_clip_stack_unref (ctx->current_clip_stack);
@@ -457,10 +452,8 @@ _cogl_clip_stack_gl_flush (CoglClipStack *stack,
   GE( ctx, glDisable (GL_STENCIL_TEST) );
 
   /* If the stack is empty then there's nothing else to do
-   *
-   * See comment below about ctx->needs_viewport_scissor_workaround
    */
-  if (stack == NULL && !ctx->needs_viewport_scissor_workaround)
+  if (stack == NULL)
     {
       COGL_NOTE (CLIPPING, "Flushed empty clip stack");
 
@@ -477,31 +470,6 @@ _cogl_clip_stack_gl_flush (CoglClipStack *stack,
                                &scissor_x0, &scissor_y0,
                                &scissor_x1, &scissor_y1);
 
-  /* XXX: ONGOING BUG: Intel viewport scissor
-   *
-   * Intel gen6 drivers don't correctly handle offset viewports, since
-   * primitives aren't clipped within the bounds of the viewport.  To
-   * workaround this we push our own clip for the viewport that will
-   * use scissoring to ensure we clip as expected.
-   *
-   * TODO: file a bug upstream!
-   */
-  if (ctx->needs_viewport_scissor_workaround)
-    {
-      _cogl_util_scissor_intersect (framebuffer->viewport_x,
-                                    framebuffer->viewport_y,
-                                    framebuffer->viewport_x +
-                                      framebuffer->viewport_width,
-                                    framebuffer->viewport_y +
-                                      framebuffer->viewport_height,
-                                    &scissor_x0, &scissor_y0,
-                                    &scissor_x1, &scissor_y1);
-      framebuffer->viewport_age_for_scissor_workaround =
-        framebuffer->viewport_age;
-      ctx->viewport_scissor_workaround_framebuffer =
-        framebuffer;
-    }
-
   /* Enable scissoring as soon as possible */
   if (scissor_x0 >= scissor_x1 || scissor_y0 >= scissor_y1)
     scissor_x0 = scissor_y0 = scissor_x1 = scissor_y1 = scissor_y_start = 0;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]