From b624e94ab1032a5a811548bc17572dc32526833f Mon Sep 17 00:00:00 2001 From: Adam Jackson Date: Wed, 6 Mar 2019 12:34:36 -0500 Subject: [PATCH] cogl: Remove viewport scissor workaround This is effectively a revert of: commit 6cfc93f26f64c506922bf119d5079d83de7543d2 Author: Robert Bragg 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;