From dbff3a357eb556aac577a2aecfbb5b037a9defca Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Thu, 15 Sep 2011 11:25:39 +0100 Subject: [PATCH] Make backface culling be part of the legacy state This adds an internal function to set the backface culling state on a pipeline. This includes properties to set the culling mode (front, back or both) and also to set which face is considered the front (COGL_WINDING_CLOCKWISE or COGL_WINDING_COUNTER_CLOCKWISE). The actual front face flushed to GL depends on whether we are rendering to an offscreen buffer or not. This means that when changing between on- and off- screen framebuffers it now checks whether the last flushed pipeline has backface culling enabled and forces a reflush of the cull face state if so. The backface culling is now set on a pipeline as part of the legacy state. This is important because some code in Cogl assumes it can flush a temporary pipeline to revert to a known state, but previously this wouldn't disable backface culling so things such as flushing the clip stack could get confused. Reviewed-by: Robert Bragg --- cogl/cogl-attribute.c | 4 -- cogl/cogl-context-private.h | 3 +- cogl/cogl-context.c | 4 +- cogl/cogl-ext-functions.h | 2 + cogl/cogl-framebuffer.c | 35 +++++++++--- cogl/cogl-internal.h | 10 ---- cogl/cogl-pipeline-opengl.c | 52 +++++++++++++++++ cogl/cogl-pipeline-private.h | 30 +++++++++- cogl/cogl-pipeline-state-private.h | 16 ++++++ cogl/cogl-pipeline-state.c | 90 ++++++++++++++++++++++++++++++ cogl/cogl-pipeline.c | 36 +++++++++++- cogl/cogl.c | 79 +++----------------------- 12 files changed, 258 insertions(+), 103 deletions(-) diff --git a/cogl/cogl-attribute.c b/cogl/cogl-attribute.c index aa2632b30..118937e7b 100644 --- a/cogl/cogl-attribute.c +++ b/cogl/cogl-attribute.c @@ -549,9 +549,6 @@ enable_gl_state (CoglDrawFlags flags, _cogl_pipeline_flush_gl_state (source, skip_gl_color, n_tex_coord_attribs); - if (ctx->enable_backface_culling) - enable_flags |= COGL_ENABLE_BACKFACE_CULLING; - _cogl_bitmask_clear_all (&ctx->temp_bitmask); /* Bind the attribute pointers. We need to do this after the @@ -726,7 +723,6 @@ enable_gl_state (CoglDrawFlags flags, set_enabled_arrays (&ctx->arrays_enabled, &ctx->temp_bitmask); _cogl_enable (enable_flags); - _cogl_flush_face_winding (); return source; } diff --git a/cogl/cogl-context-private.h b/cogl/cogl-context-private.h index 175a2c914..277116628 100644 --- a/cogl/cogl-context-private.h +++ b/cogl/cogl-context-private.h @@ -74,8 +74,7 @@ struct _CoglContext /* Enable cache */ unsigned long enable_flags; - gboolean enable_backface_culling; - CoglFrontWinding flushed_front_winding; + gboolean legacy_backface_culling_enabled; /* A few handy matrix constants */ CoglMatrix identity_matrix; diff --git a/cogl/cogl-context.c b/cogl/cogl-context.c index fe86721b6..d4bbc2046 100644 --- a/cogl/cogl-context.c +++ b/cogl/cogl-context.c @@ -216,8 +216,7 @@ cogl_context_new (CoglDisplay *display, context->current_clip_stack_valid = FALSE; context->current_clip_stack = NULL; - context->enable_backface_culling = FALSE; - context->flushed_front_winding = COGL_FRONT_WINDING_COUNTER_CLOCKWISE; + context->legacy_backface_culling_enabled = FALSE; cogl_matrix_init_identity (&context->identity_matrix); cogl_matrix_init_identity (&context->y_flip_matrix); @@ -369,7 +368,6 @@ cogl_context_new (CoglDisplay *display, cogl_push_source (context->opaque_color_pipeline); _cogl_pipeline_flush_gl_state (context->opaque_color_pipeline, FALSE, 0); _cogl_enable (enable_flags); - _cogl_flush_face_winding (); context->atlases = NULL; g_hook_list_init (&context->atlas_reorganize_callbacks, sizeof (GHook)); diff --git a/cogl/cogl-ext-functions.h b/cogl/cogl-ext-functions.h index 47d5192fc..7c3ab7aee 100644 --- a/cogl/cogl-ext-functions.h +++ b/cogl/cogl-ext-functions.h @@ -106,6 +106,8 @@ COGL_EXT_FUNCTION (void, glFlush, (void)) COGL_EXT_FUNCTION (void, glFrontFace, (GLenum mode)) +COGL_EXT_FUNCTION (void, glCullFace, + (GLenum mode)) COGL_EXT_FUNCTION (void, glGenTextures, (GLsizei n, GLuint* textures)) COGL_EXT_FUNCTION (GLenum, glGetError, diff --git a/cogl/cogl-framebuffer.c b/cogl/cogl-framebuffer.c index 2ba034f99..e086e1062 100644 --- a/cogl/cogl-framebuffer.c +++ b/cogl/cogl-framebuffer.c @@ -39,6 +39,7 @@ #include "cogl-clip-stack.h" #include "cogl-journal-private.h" #include "cogl-winsys-private.h" +#include "cogl-pipeline-state-private.h" #ifndef GL_FRAMEBUFFER #define GL_FRAMEBUFFER 0x8D40 @@ -1143,16 +1144,32 @@ notify_buffers_changed (CoglFramebuffer *old_draw_buffer, _cogl_clip_stack_dirty (); - /* If the two draw framebuffers have a different color mask then we - need to ensure the logic ops are reflushed the next time - something is drawn */ - if (old_draw_buffer && new_draw_buffer && - cogl_framebuffer_get_color_mask (old_draw_buffer) != - cogl_framebuffer_get_color_mask (new_draw_buffer)) + if (old_draw_buffer && new_draw_buffer) { - ctx->current_pipeline_changes_since_flush |= - COGL_PIPELINE_STATE_LOGIC_OPS; - ctx->current_pipeline_age--; + /* If the two draw framebuffers have a different color mask then + we need to ensure the logic ops are reflushed the next time + something is drawn */ + if (cogl_framebuffer_get_color_mask (old_draw_buffer) != + cogl_framebuffer_get_color_mask (new_draw_buffer)) + { + ctx->current_pipeline_changes_since_flush |= + COGL_PIPELINE_STATE_LOGIC_OPS; + ctx->current_pipeline_age--; + } + + /* If we're switching from onscreen to offscreen and the last + flush pipeline is using backface culling then we also need to + reflush the cull face state because the winding order of the + front face is flipped for offscreen buffers */ + if (old_draw_buffer->type != new_draw_buffer->type && + ctx->current_pipeline && + _cogl_pipeline_get_cull_face_mode (ctx->current_pipeline) != + COGL_PIPELINE_CULL_FACE_MODE_NONE) + { + ctx->current_pipeline_changes_since_flush |= + COGL_PIPELINE_STATE_CULL_FACE; + ctx->current_pipeline_age--; + } } /* XXX: diff --git a/cogl/cogl-internal.h b/cogl/cogl-internal.h index ca9e3958a..bedb74be1 100644 --- a/cogl/cogl-internal.h +++ b/cogl/cogl-internal.h @@ -32,12 +32,6 @@ #include #endif -typedef enum -{ - COGL_FRONT_WINDING_CLOCKWISE, - COGL_FRONT_WINDING_COUNTER_CLOCKWISE -} CoglFrontWinding; - typedef enum { COGL_BOXED_NONE, COGL_BOXED_INT, @@ -98,7 +92,6 @@ cogl_gl_error_to_string (GLenum error_code); #define COGL_ENABLE_ALPHA_TEST (1<<1) #define COGL_ENABLE_VERTEX_ARRAY (1<<2) #define COGL_ENABLE_COLOR_ARRAY (1<<3) -#define COGL_ENABLE_BACKFACE_CULLING (1<<4) int _cogl_get_format_bpp (CoglPixelFormat format); @@ -109,9 +102,6 @@ _cogl_enable (unsigned long flags); unsigned long _cogl_get_enable (void); -void -_cogl_flush_face_winding (void); - void _cogl_transform_point (const CoglMatrix *matrix_mv, const CoglMatrix *matrix_p, diff --git a/cogl/cogl-pipeline-opengl.c b/cogl/cogl-pipeline-opengl.c index 28693e604..caede90db 100644 --- a/cogl/cogl-pipeline-opengl.c +++ b/cogl/cogl-pipeline-opengl.c @@ -605,6 +605,58 @@ _cogl_pipeline_flush_color_blend_alpha_depth_state ( ctx->current_gl_color_mask = color_mask; } + if (pipelines_difference & COGL_PIPELINE_STATE_CULL_FACE) + { + CoglPipeline *authority = + _cogl_pipeline_get_authority (pipeline, COGL_PIPELINE_STATE_CULL_FACE); + CoglPipelineCullFaceState *cull_face_state + = &authority->big_state->cull_face_state; + + if (cull_face_state->mode == COGL_PIPELINE_CULL_FACE_MODE_NONE) + GE( ctx, glDisable (GL_CULL_FACE) ); + else + { + CoglFramebuffer *draw_framebuffer = cogl_get_draw_framebuffer (); + gboolean invert_winding; + + GE( ctx, glEnable (GL_CULL_FACE) ); + + switch (cull_face_state->mode) + { + case COGL_PIPELINE_CULL_FACE_MODE_NONE: + g_assert_not_reached (); + + case COGL_PIPELINE_CULL_FACE_MODE_FRONT: + GE( ctx, glCullFace (GL_FRONT) ); + break; + + case COGL_PIPELINE_CULL_FACE_MODE_BACK: + GE( ctx, glCullFace (GL_BACK) ); + break; + + case COGL_PIPELINE_CULL_FACE_MODE_BOTH: + GE( ctx, glCullFace (GL_FRONT_AND_BACK) ); + break; + } + + /* If we are painting to an offscreen framebuffer then we + need to invert the winding of the front face because + everything is painted upside down */ + invert_winding = cogl_is_offscreen (draw_framebuffer); + + switch (cull_face_state->front_winding) + { + case COGL_WINDING_CLOCKWISE: + GE( ctx, glFrontFace (invert_winding ? GL_CCW : GL_CW) ); + break; + + case COGL_WINDING_COUNTER_CLOCKWISE: + GE( ctx, glFrontFace (invert_winding ? GL_CW : GL_CCW) ); + break; + } + } + } + if (pipeline->real_blend_enable != ctx->gl_blend_enable_cache) { if (pipeline->real_blend_enable) diff --git a/cogl/cogl-pipeline-private.h b/cogl/cogl-pipeline-private.h index de96c6c75..346026e89 100644 --- a/cogl/cogl-pipeline-private.h +++ b/cogl/cogl-pipeline-private.h @@ -429,6 +429,7 @@ typedef enum COGL_PIPELINE_STATE_FOG_INDEX, COGL_PIPELINE_STATE_POINT_SIZE_INDEX, COGL_PIPELINE_STATE_LOGIC_OPS_INDEX, + COGL_PIPELINE_STATE_CULL_FACE_INDEX, /* non-sparse */ COGL_PIPELINE_STATE_REAL_BLEND_ENABLE_INDEX, @@ -474,6 +475,8 @@ typedef enum _CoglPipelineState 1L<color_mask == logic_ops_state1->color_mask; } +gboolean +_cogl_pipeline_cull_face_state_equal (CoglPipeline *authority0, + CoglPipeline *authority1) +{ + CoglPipelineCullFaceState *cull_face_state0 + = &authority0->big_state->cull_face_state; + CoglPipelineCullFaceState *cull_face_state1 + = &authority1->big_state->cull_face_state; + + /* The cull face state is considered equal if two pipelines are both + set to no culling. If the front winding property is ever used for + anything else or the comparison is used not just for drawing then + this would have to change */ + + if (cull_face_state0->mode == COGL_PIPELINE_CULL_FACE_MODE_NONE) + return cull_face_state1->mode == COGL_PIPELINE_CULL_FACE_MODE_NONE; + + return (cull_face_state0->mode == cull_face_state1->mode && + cull_face_state0->front_winding == cull_face_state1->front_winding); +} + gboolean _cogl_pipeline_user_shader_equal (CoglPipeline *authority0, CoglPipeline *authority1) @@ -1155,6 +1176,52 @@ _cogl_pipeline_set_fog_state (CoglPipeline *pipeline, _cogl_pipeline_fog_state_equal); } +void +_cogl_pipeline_set_cull_face_state (CoglPipeline *pipeline, + const CoglPipelineCullFaceState * + cull_face_state) +{ + CoglPipelineState state = COGL_PIPELINE_STATE_CULL_FACE; + CoglPipeline *authority; + CoglPipelineCullFaceState *current_cull_face_state; + + g_return_if_fail (cogl_is_pipeline (pipeline)); + + authority = _cogl_pipeline_get_authority (pipeline, state); + + current_cull_face_state = &authority->big_state->cull_face_state; + + if (current_cull_face_state->mode == cull_face_state->mode && + current_cull_face_state->front_winding == cull_face_state->front_winding) + return; + + /* - Flush journal primitives referencing the current state. + * - Make sure the pipeline has no dependants so it may be modified. + * - If the pipeline isn't currently an authority for the state being + * changed, then initialize that state from the current authority. + */ + _cogl_pipeline_pre_change_notify (pipeline, state, NULL, FALSE); + + pipeline->big_state->cull_face_state = *cull_face_state; + + _cogl_pipeline_update_authority (pipeline, authority, state, + _cogl_pipeline_cull_face_state_equal); +} + +CoglPipelineCullFaceMode +_cogl_pipeline_get_cull_face_mode (CoglPipeline *pipeline) +{ + CoglPipelineState state = COGL_PIPELINE_STATE_CULL_FACE; + CoglPipeline *authority; + + g_return_val_if_fail (cogl_is_pipeline (pipeline), + COGL_PIPELINE_CULL_FACE_MODE_NONE); + + authority = _cogl_pipeline_get_authority (pipeline, state); + + return authority->big_state->cull_face_state.mode; +} + float cogl_pipeline_get_point_size (CoglPipeline *pipeline) { @@ -1365,3 +1432,26 @@ _cogl_pipeline_hash_logic_ops_state (CoglPipeline *authority, state->hash = _cogl_util_one_at_a_time_hash (state->hash, &logic_ops_state->color_mask, sizeof (CoglColorMask)); } + +void +_cogl_pipeline_hash_cull_face_state (CoglPipeline *authority, + CoglPipelineHashState *state) +{ + CoglPipelineCullFaceState *cull_face_state + = &authority->big_state->cull_face_state; + + /* The cull face state is considered equal if two pipelines are both + set to no culling. If the front winding property is ever used for + anything else or the hashing is used not just for drawing then + this would have to change */ + if (cull_face_state->mode == COGL_PIPELINE_CULL_FACE_MODE_NONE) + state->hash = + _cogl_util_one_at_a_time_hash (state->hash, + &cull_face_state->mode, + sizeof (CoglPipelineCullFaceMode)); + else + state->hash = + _cogl_util_one_at_a_time_hash (state->hash, + cull_face_state, + sizeof (CoglPipelineCullFaceState)); +} diff --git a/cogl/cogl-pipeline.c b/cogl/cogl-pipeline.c index d0e32d94c..42923be83 100644 --- a/cogl/cogl-pipeline.c +++ b/cogl/cogl-pipeline.c @@ -190,6 +190,7 @@ _cogl_pipeline_init_default_pipeline (void) CoglPipelineBlendState *blend_state = &big_state->blend_state; CoglDepthState *depth_state = &big_state->depth_state; CoglPipelineLogicOpsState *logic_ops_state = &big_state->logic_ops_state; + CoglPipelineCullFaceState *cull_face_state = &big_state->cull_face_state; _COGL_GET_CONTEXT (ctx, NO_RETVAL); @@ -297,6 +298,9 @@ _cogl_pipeline_init_default_pipeline (void) logic_ops_state->color_mask = COGL_COLOR_MASK_ALL; + cull_face_state->mode = COGL_PIPELINE_CULL_FACE_MODE_NONE; + cull_face_state->front_winding = COGL_WINDING_COUNTER_CLOCKWISE; + ctx->default_pipeline = _cogl_pipeline_object_new (pipeline); } @@ -1047,6 +1051,13 @@ _cogl_pipeline_copy_differences (CoglPipeline *dest, sizeof (CoglPipelineLogicOpsState)); } + if (differences & COGL_PIPELINE_STATE_CULL_FACE) + { + memcpy (&big_state->cull_face_state, + &src->big_state->cull_face_state, + sizeof (CoglPipelineCullFaceState)); + } + /* XXX: we shouldn't bother doing this in most cases since * _copy_differences is typically used to initialize pipeline state * by copying it from the current authority, so it's not actually @@ -1124,6 +1135,13 @@ _cogl_pipeline_init_multi_property_sparse_state (CoglPipeline *pipeline, sizeof (CoglPipelineLogicOpsState)); break; } + case COGL_PIPELINE_STATE_CULL_FACE: + { + memcpy (&pipeline->big_state->cull_face_state, + &authority->big_state->cull_face_state, + sizeof (CoglPipelineCullFaceState)); + break; + } } } @@ -2752,6 +2770,12 @@ _cogl_pipeline_equal (CoglPipeline *pipeline0, _cogl_pipeline_fog_state_equal)) goto done; + if (!simple_property_equal (authorities0, authorities1, + pipelines_difference, + COGL_PIPELINE_STATE_CULL_FACE_INDEX, + _cogl_pipeline_cull_face_state_equal)) + goto done; + if (!simple_property_equal (authorities0, authorities1, pipelines_difference, COGL_PIPELINE_STATE_POINT_SIZE_INDEX, @@ -3208,6 +3232,14 @@ _cogl_pipeline_apply_legacy_state (CoglPipeline *pipeline) if (ctx->legacy_fog_state.enabled) _cogl_pipeline_set_fog_state (pipeline, &ctx->legacy_fog_state); + + if (ctx->legacy_backface_culling_enabled) + { + CoglPipelineCullFaceState state; + state.mode = COGL_PIPELINE_CULL_FACE_MODE_BACK; + state.front_winding = COGL_WINDING_COUNTER_CLOCKWISE; + _cogl_pipeline_set_cull_face_state (pipeline, &state); + } } void @@ -3346,13 +3378,15 @@ _cogl_pipeline_init_state_hash_functions (void) _cogl_pipeline_hash_depth_state; state_hash_functions[COGL_PIPELINE_STATE_FOG_INDEX] = _cogl_pipeline_hash_fog_state; + state_hash_functions[COGL_PIPELINE_STATE_CULL_FACE_INDEX] = + _cogl_pipeline_hash_cull_face_state; state_hash_functions[COGL_PIPELINE_STATE_POINT_SIZE_INDEX] = _cogl_pipeline_hash_point_size_state; state_hash_functions[COGL_PIPELINE_STATE_LOGIC_OPS_INDEX] = _cogl_pipeline_hash_logic_ops_state; /* So we get a big error if we forget to update this code! */ - g_assert (COGL_PIPELINE_STATE_SPARSE_COUNT == 12); + g_assert (COGL_PIPELINE_STATE_SPARSE_COUNT == 13); } unsigned int diff --git a/cogl/cogl.c b/cogl/cogl.c index 261348fbb..9bbc3220c 100644 --- a/cogl/cogl.c +++ b/cogl/cogl.c @@ -141,33 +141,6 @@ cogl_clear (const CoglColor *color, unsigned long buffers) cogl_framebuffer_clear (cogl_get_draw_framebuffer (), buffers, color); } -static gboolean -toggle_flag (CoglContext *ctx, - unsigned long new_flags, - unsigned long flag, - GLenum gl_flag) -{ - /* Toggles and caches a single enable flag on or off - * by comparing to current state - */ - if (new_flags & flag) - { - if (!(ctx->enable_flags & flag)) - { - GE( ctx, glEnable (gl_flag) ); - ctx->enable_flags |= flag; - return TRUE; - } - } - else if (ctx->enable_flags & flag) - { - GE( ctx, glDisable (gl_flag) ); - ctx->enable_flags &= ~flag; - } - - return FALSE; -} - #if defined (HAVE_COGL_GL) || defined (HAVE_COGL_GLES) static gboolean @@ -209,10 +182,6 @@ _cogl_enable (unsigned long flags) */ _COGL_GET_CONTEXT (ctx, NO_RETVAL); - toggle_flag (ctx, flags, - COGL_ENABLE_BACKFACE_CULLING, - GL_CULL_FACE); - #if defined (HAVE_COGL_GL) || defined (HAVE_COGL_GLES) if (ctx->driver != COGL_DRIVER_GLES2) { @@ -264,13 +233,15 @@ cogl_set_backface_culling_enabled (gboolean setting) { _COGL_GET_CONTEXT (ctx, NO_RETVAL); - if (ctx->enable_backface_culling == setting) + if (ctx->legacy_backface_culling_enabled == setting) return; - /* Currently the journal can't track changes to backface culling state... */ - _cogl_framebuffer_flush_journal (cogl_get_draw_framebuffer ()); + ctx->legacy_backface_culling_enabled = setting; - ctx->enable_backface_culling = setting; + if (ctx->legacy_backface_culling_enabled) + ctx->legacy_state_set++; + else + ctx->legacy_state_set--; } gboolean @@ -278,39 +249,7 @@ cogl_get_backface_culling_enabled (void) { _COGL_GET_CONTEXT (ctx, FALSE); - return ctx->enable_backface_culling; -} - -void -_cogl_flush_face_winding (void) -{ - CoglFrontWinding winding; - - _COGL_GET_CONTEXT (ctx, NO_RETVAL); - - /* The front face winding doesn't matter if we aren't performing any - * backface culling... */ - if (!ctx->enable_backface_culling) - return; - - /* NB: We use a clockwise face winding order when drawing offscreen because - * all offscreen rendering is done upside down resulting in reversed winding - * for all triangles. - */ - if (cogl_is_offscreen (cogl_get_draw_framebuffer ())) - winding = COGL_FRONT_WINDING_CLOCKWISE; - else - winding = COGL_FRONT_WINDING_COUNTER_CLOCKWISE; - - if (winding != ctx->flushed_front_winding) - { - - if (winding == COGL_FRONT_WINDING_CLOCKWISE) - GE (ctx, glFrontFace (GL_CW)); - else - GE (ctx, glFrontFace (GL_CCW)); - ctx->flushed_front_winding = winding; - } + return ctx->legacy_backface_culling_enabled; } void @@ -726,11 +665,7 @@ cogl_begin_gl (void) FALSE, cogl_pipeline_get_n_layers (pipeline)); - if (ctx->enable_backface_culling) - enable_flags |= COGL_ENABLE_BACKFACE_CULLING; - _cogl_enable (enable_flags); - _cogl_flush_face_winding (); /* Disable any cached vertex arrays */ _cogl_attribute_disable_cached_arrays ();