diff --git a/cogl/cogl-pipeline-cache.c b/cogl/cogl-pipeline-cache.c index df4c4338b..3a96490c5 100644 --- a/cogl/cogl-pipeline-cache.c +++ b/cogl/cogl-pipeline-cache.c @@ -53,7 +53,7 @@ _cogl_pipeline_cache_new (void) _COGL_GET_CONTEXT (ctx, 0); vertex_state = - COGL_PIPELINE_STATE_AFFECTS_VERTEX_CODEGEN; + _cogl_pipeline_get_state_for_vertex_codegen (ctx); layer_vertex_state = COGL_PIPELINE_LAYER_STATE_AFFECTS_VERTEX_CODEGEN; fragment_state = diff --git a/cogl/cogl-pipeline-private.h b/cogl/cogl-pipeline-private.h index 5c875e3a8..de220450b 100644 --- a/cogl/cogl-pipeline-private.h +++ b/cogl/cogl-pipeline-private.h @@ -117,6 +117,7 @@ typedef enum COGL_PIPELINE_STATE_USER_SHADER_INDEX, COGL_PIPELINE_STATE_DEPTH_INDEX, COGL_PIPELINE_STATE_FOG_INDEX, + COGL_PIPELINE_STATE_NON_ZERO_POINT_SIZE_INDEX, COGL_PIPELINE_STATE_POINT_SIZE_INDEX, COGL_PIPELINE_STATE_PER_VERTEX_POINT_SIZE_INDEX, COGL_PIPELINE_STATE_LOGIC_OPS_INDEX, @@ -166,6 +167,8 @@ typedef enum _CoglPipelineState 1L<big_state->non_zero_point_size == + authority1->big_state->non_zero_point_size); +} + CoglBool _cogl_pipeline_point_size_equal (CoglPipeline *authority0, CoglPipeline *authority1) @@ -1379,6 +1387,30 @@ cogl_pipeline_get_point_size (CoglPipeline *pipeline) return authority->big_state->point_size; } +static void +_cogl_pipeline_set_non_zero_point_size (CoglPipeline *pipeline, + CoglBool value) +{ + CoglPipelineState state = COGL_PIPELINE_STATE_NON_ZERO_POINT_SIZE; + CoglPipeline *authority; + + _COGL_RETURN_IF_FAIL (cogl_is_pipeline (pipeline)); + + authority = _cogl_pipeline_get_authority (pipeline, state); + + /* - 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->non_zero_point_size = !!value; + + _cogl_pipeline_update_authority (pipeline, authority, state, + _cogl_pipeline_non_zero_point_size_equal); +} + void cogl_pipeline_set_point_size (CoglPipeline *pipeline, float point_size) @@ -1393,6 +1425,12 @@ cogl_pipeline_set_point_size (CoglPipeline *pipeline, if (authority->big_state->point_size == point_size) return; + /* Changing the point size may additionally modify + * COGL_PIPELINE_STATE_NON_ZERO_POINT_SIZE. */ + + if ((authority->big_state->point_size > 0.0f) != (point_size > 0.0f)) + _cogl_pipeline_set_non_zero_point_size (pipeline, point_size > 0.0f); + /* - 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 @@ -1888,6 +1926,17 @@ _cogl_pipeline_hash_fog_state (CoglPipeline *authority, state->hash = hash; } +void +_cogl_pipeline_hash_non_zero_point_size_state (CoglPipeline *authority, + CoglPipelineHashState *state) +{ + CoglBool non_zero_point_size = authority->big_state->non_zero_point_size; + + state->hash = _cogl_util_one_at_a_time_hash (state->hash, + &non_zero_point_size, + sizeof (non_zero_point_size)); +} + void _cogl_pipeline_hash_point_size_state (CoglPipeline *authority, CoglPipelineHashState *state) diff --git a/cogl/cogl-pipeline-state.h b/cogl/cogl-pipeline-state.h index 1c68db79d..4c958dc3d 100644 --- a/cogl/cogl-pipeline-state.h +++ b/cogl/cogl-pipeline-state.h @@ -487,12 +487,17 @@ cogl_pipeline_set_blend_constant (CoglPipeline *pipeline, * @point_size: the new point size. * * Changes the size of points drawn when %COGL_VERTICES_MODE_POINTS is - * used with the vertex buffer API. Note that typically the GPU will - * only support a limited minimum and maximum range of point sizes. If - * the chosen point size is outside that range then the nearest value - * within that range will be used instead. The size of a point is in - * screen space so it will be the same regardless of any - * transformations. The default point size is 1.0. + * used with the attribute buffer API. Note that typically the GPU + * will only support a limited minimum and maximum range of point + * sizes. If the chosen point size is outside that range then the + * nearest value within that range will be used instead. The size of a + * point is in screen space so it will be the same regardless of any + * transformations. + * + * If the point size is set to 0.0 then drawing points with the + * pipeline will have undefined results. This is the default value so + * if an application wants to draw points it must make sure to use a + * pipeline that has an explicit point size set on it. * * Since: 2.0 * Stability: Unstable diff --git a/cogl/cogl-pipeline.c b/cogl/cogl-pipeline.c index 4ef83399b..01cd5f3ec 100644 --- a/cogl/cogl-pipeline.c +++ b/cogl/cogl-pipeline.c @@ -216,7 +216,7 @@ _cogl_pipeline_init_default_pipeline (void) cogl_depth_state_init (&big_state->depth_state); - big_state->point_size = 1.0f; + big_state->point_size = 0.0f; logic_ops_state->color_mask = COGL_COLOR_MASK_ALL; @@ -1048,6 +1048,9 @@ _cogl_pipeline_copy_differences (CoglPipeline *dest, sizeof (CoglPipelineFogState)); } + if (differences & COGL_PIPELINE_STATE_NON_ZERO_POINT_SIZE) + big_state->non_zero_point_size = src->big_state->non_zero_point_size; + if (differences & COGL_PIPELINE_STATE_POINT_SIZE) big_state->point_size = src->big_state->point_size; @@ -1135,6 +1138,7 @@ _cogl_pipeline_init_multi_property_sparse_state (CoglPipeline *pipeline, case COGL_PIPELINE_STATE_BLEND_ENABLE: case COGL_PIPELINE_STATE_ALPHA_FUNC: case COGL_PIPELINE_STATE_ALPHA_FUNC_REFERENCE: + case COGL_PIPELINE_STATE_NON_ZERO_POINT_SIZE: case COGL_PIPELINE_STATE_POINT_SIZE: case COGL_PIPELINE_STATE_USER_SHADER: case COGL_PIPELINE_STATE_PER_VERTEX_POINT_SIZE: @@ -2318,6 +2322,11 @@ _cogl_pipeline_equal (CoglPipeline *pipeline0, authorities1[bit])) goto done; break; + case COGL_PIPELINE_STATE_NON_ZERO_POINT_SIZE_INDEX: + if (!_cogl_pipeline_non_zero_point_size_equal (authorities0[bit], + authorities1[bit])) + goto done; + break; case COGL_PIPELINE_STATE_POINT_SIZE_INDEX: if (!_cogl_pipeline_point_size_equal (authorities0[bit], authorities1[bit])) @@ -2775,6 +2784,8 @@ _cogl_pipeline_init_state_hash_functions (void) _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_NON_ZERO_POINT_SIZE_INDEX] = + _cogl_pipeline_hash_non_zero_point_size_state; state_hash_functions[COGL_PIPELINE_STATE_POINT_SIZE_INDEX] = _cogl_pipeline_hash_point_size_state; state_hash_functions[COGL_PIPELINE_STATE_PER_VERTEX_POINT_SIZE_INDEX] = @@ -2790,7 +2801,7 @@ _cogl_pipeline_init_state_hash_functions (void) { /* So we get a big error if we forget to update this code! */ - _COGL_STATIC_ASSERT (COGL_PIPELINE_STATE_SPARSE_COUNT == 17, + _COGL_STATIC_ASSERT (COGL_PIPELINE_STATE_SPARSE_COUNT == 18, "Make sure to install a hash function for " "newly added pipeline state and update assert " "in _cogl_pipeline_init_state_hash_functions"); @@ -3057,6 +3068,25 @@ _cogl_pipeline_find_equivalent_parent (CoglPipeline *pipeline, return authority1; } +CoglPipelineState +_cogl_pipeline_get_state_for_vertex_codegen (CoglContext *context) +{ + CoglPipelineState state = (COGL_PIPELINE_STATE_LAYERS | + COGL_PIPELINE_STATE_USER_SHADER | + COGL_PIPELINE_STATE_PER_VERTEX_POINT_SIZE | + COGL_PIPELINE_STATE_VERTEX_SNIPPETS); + + /* If we don't have the builtin point size uniform then we'll add + * one in the GLSL but we'll only do this if the point size is + * non-zero. Whether or not the point size is zero is represented by + * COGL_PIPELINE_STATE_NON_ZERO_POINT_SIZE */ + if (!(context->private_feature_flags & + COGL_PRIVATE_FEATURE_BUILTIN_POINT_SIZE_UNIFORM)) + state |= COGL_PIPELINE_STATE_NON_ZERO_POINT_SIZE; + + return state; +} + CoglPipelineLayerState _cogl_pipeline_get_layer_state_for_fragment_codegen (CoglContext *context) { diff --git a/cogl/driver/gl/cogl-pipeline-progend-glsl.c b/cogl/driver/gl/cogl-pipeline-progend-glsl.c index d695f2d15..ee8de2ada 100644 --- a/cogl/driver/gl/cogl-pipeline-progend-glsl.c +++ b/cogl/driver/gl/cogl-pipeline-progend-glsl.c @@ -658,7 +658,7 @@ _cogl_pipeline_progend_glsl_end (CoglPipeline *pipeline, state */ authority = _cogl_pipeline_find_equivalent_parent (pipeline, - (COGL_PIPELINE_STATE_AFFECTS_VERTEX_CODEGEN | + (_cogl_pipeline_get_state_for_vertex_codegen (ctx) | _cogl_pipeline_get_state_for_fragment_codegen (ctx)) & ~COGL_PIPELINE_STATE_LAYERS, _cogl_pipeline_get_layer_state_for_fragment_codegen (ctx) | @@ -830,8 +830,8 @@ _cogl_pipeline_progend_glsl_pre_change_notify (CoglPipeline *pipeline, { _COGL_GET_CONTEXT (ctx, NO_RETVAL); - if ((change & (_cogl_pipeline_get_state_for_fragment_codegen (ctx) | - COGL_PIPELINE_STATE_AFFECTS_VERTEX_CODEGEN))) + if ((change & (_cogl_pipeline_get_state_for_vertex_codegen (ctx) | + _cogl_pipeline_get_state_for_fragment_codegen (ctx)))) { dirty_program_state (pipeline); } diff --git a/cogl/driver/gl/cogl-pipeline-vertend-fixed.c b/cogl/driver/gl/cogl-pipeline-vertend-fixed.c index be1713538..f1b6e52fb 100644 --- a/cogl/driver/gl/cogl-pipeline-vertend-fixed.c +++ b/cogl/driver/gl/cogl-pipeline-vertend-fixed.c @@ -95,7 +95,8 @@ _cogl_pipeline_vertend_fixed_end (CoglPipeline *pipeline, CoglPipeline *authority = _cogl_pipeline_get_authority (pipeline, COGL_PIPELINE_STATE_POINT_SIZE); - GE( ctx, glPointSize (authority->big_state->point_size) ); + if (authority->big_state->point_size > 0.0f) + GE( ctx, glPointSize (authority->big_state->point_size) ); } return TRUE; diff --git a/cogl/driver/gl/cogl-pipeline-vertend-glsl.c b/cogl/driver/gl/cogl-pipeline-vertend-glsl.c index 069573a18..57b62653d 100644 --- a/cogl/driver/gl/cogl-pipeline-vertend-glsl.c +++ b/cogl/driver/gl/cogl-pipeline-vertend-glsl.c @@ -31,6 +31,8 @@ #include +#include + #include "cogl-context-private.h" #include "cogl-util-gl-private.h" #include "cogl-pipeline-private.h" @@ -209,7 +211,7 @@ _cogl_pipeline_vertend_glsl_start (CoglPipeline *pipeline, state */ authority = _cogl_pipeline_find_equivalent_parent (pipeline, - COGL_PIPELINE_STATE_AFFECTS_VERTEX_CODEGEN & + _cogl_pipeline_get_state_for_vertex_codegen (ctx) & ~COGL_PIPELINE_STATE_LAYERS, COGL_PIPELINE_LAYER_STATE_AFFECTS_VERTEX_CODEGEN); @@ -297,12 +299,18 @@ _cogl_pipeline_vertend_glsl_start (CoglPipeline *pipeline, COGL_PRIVATE_FEATURE_BUILTIN_POINT_SIZE_UNIFORM)) { /* There is no builtin uniform for the point size on GLES2 so we - need to copy it from the custom uniform in the vertex shader if - we're not using per-vertex point sizes */ - g_string_append (shader_state->header, - "uniform float cogl_point_size_in;\n"); - g_string_append (shader_state->source, - " cogl_point_size_out = cogl_point_size_in;\n"); + need to copy it from the custom uniform in the vertex shader + if we're not using per-vertex point sizes, however we'll only + do this if the point-size is non-zero. Toggle the point size + between zero and non-zero causes a state change which + generates a new program */ + if (cogl_pipeline_get_point_size (pipeline) > 0.0f) + { + g_string_append (shader_state->header, + "uniform float cogl_point_size_in;\n"); + g_string_append (shader_state->source, + " cogl_point_size_out = cogl_point_size_in;\n"); + } } } @@ -532,7 +540,8 @@ _cogl_pipeline_vertend_glsl_end (CoglPipeline *pipeline, CoglPipeline *authority = _cogl_pipeline_get_authority (pipeline, COGL_PIPELINE_STATE_POINT_SIZE); - GE( ctx, glPointSize (authority->big_state->point_size) ); + if (authority->big_state->point_size > 0.0f) + GE( ctx, glPointSize (authority->big_state->point_size) ); } #endif /* HAVE_COGL_GL */ @@ -544,7 +553,9 @@ _cogl_pipeline_vertend_glsl_pre_change_notify (CoglPipeline *pipeline, CoglPipelineState change, const CoglColor *new_color) { - if ((change & COGL_PIPELINE_STATE_AFFECTS_VERTEX_CODEGEN)) + _COGL_GET_CONTEXT (ctx, NO_RETVAL); + + if ((change & _cogl_pipeline_get_state_for_vertex_codegen (ctx))) dirty_shader_state (pipeline); } @@ -588,4 +599,63 @@ const CoglPipelineVertend _cogl_pipeline_glsl_vertend = _cogl_pipeline_vertend_glsl_layer_pre_change_notify }; +UNIT_TEST (check_point_size_shader, + 0 /* no requirements */, + 0 /* no failure cases */) +{ + CoglPipeline *pipelines[4]; + CoglPipelineShaderState *shader_states[G_N_ELEMENTS (pipelines)]; + int i; + + /* Default pipeline with zero point size */ + pipelines[0] = cogl_pipeline_new (test_ctx); + + /* Point size 1 */ + pipelines[1] = cogl_pipeline_new (test_ctx); + cogl_pipeline_set_point_size (pipelines[1], 1.0f); + + /* Point size 2 */ + pipelines[2] = cogl_pipeline_new (test_ctx); + cogl_pipeline_set_point_size (pipelines[2], 2.0f); + + /* Same as the first pipeline, but reached by restoring the old + * state from a copy */ + pipelines[3] = cogl_pipeline_copy (pipelines[1]); + cogl_pipeline_set_point_size (pipelines[3], 0.0f); + + /* Draw something with all of the pipelines to make sure their state + * is flushed */ + for (i = 0; i < G_N_ELEMENTS (pipelines); i++) + cogl_framebuffer_draw_rectangle (test_fb, + pipelines[i], + 0.0f, 0.0f, + 10.0f, 10.0f); + cogl_framebuffer_finish (test_fb); + + /* Get all of the shader states. These might be NULL if the driver + * is not using GLSL */ + for (i = 0; i < G_N_ELEMENTS (pipelines); i++) + shader_states[i] = get_shader_state (pipelines[i]); + + /* If the first two pipelines are using GLSL then they should have + * the same shader unless there is no builtin uniform for the point + * size */ + if (shader_states[0]) + { + if ((test_ctx->private_feature_flags & + COGL_PRIVATE_FEATURE_BUILTIN_POINT_SIZE_UNIFORM)) + g_assert (shader_states[0] == shader_states[1]); + else + g_assert (shader_states[0] != shader_states[1]); + } + + /* The second and third pipelines should always have the same shader + * state because only toggling between zero and non-zero should + * change the shader */ + g_assert (shader_states[1] == shader_states[2]); + + /* The fourth pipeline should be exactly the same as the first */ + g_assert (shader_states[0] == shader_states[3]); +} + #endif /* COGL_PIPELINE_VERTEND_GLSL */