[Cogl] [PATCH] Don't generate GLSL for the point size for default pipelines
Neil Roberts
neil at linux.intel.com
Thu Jun 20 05:35:03 PDT 2013
Previously on GLES2 where there is no builtin point size uniform then
we would always add a line to the vertex shader to write to the
builtin point size output because when generating the shader it is not
possible to determine if the pipeline will be used to draw points or
not. This patch changes it so that the default point size is 0.0f
which is documented to have undefined results when drawing points.
That way we can avoid adding the point size code to the shader in that
case. The assumption is that any application that is drawing points
will probably have explicitly set the point size on the pipeline
anyway so it is not a big deal to change the default size from 1.0f.
This adds a new pipeline state flag to track whether the point size is
non-zero. This needs to be its own state because altering it needs to
cause a different shader to be added to the pipeline cache. The state
flags that affect the vertex shader have been changed from a constant
to a runtime function because they will be different depending on
whether there is a builtin point size uniform.
There is also a unit test to ensure that changing the point size does
or doesn't generate a new shader depending on the values.
---
cogl/cogl-pipeline-cache.c | 2 +-
cogl/cogl-pipeline-private.h | 15 +++--
cogl/cogl-pipeline-state-private.h | 8 +++
cogl/cogl-pipeline-state.c | 49 ++++++++++++++++
cogl/cogl-pipeline-state.h | 17 ++++--
cogl/cogl-pipeline.c | 33 ++++++++++-
cogl/driver/gl/cogl-pipeline-progend-glsl.c | 6 +-
cogl/driver/gl/cogl-pipeline-vertend-fixed.c | 3 +-
cogl/driver/gl/cogl-pipeline-vertend-glsl.c | 88 +++++++++++++++++++++++++---
9 files changed, 193 insertions(+), 28 deletions(-)
diff --git a/cogl/cogl-pipeline-cache.c b/cogl/cogl-pipeline-cache.c
index df4c433..3a96490 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 72917de..996ec3a 100644
--- a/cogl/cogl-pipeline-private.h
+++ b/cogl/cogl-pipeline-private.h
@@ -114,6 +114,7 @@ typedef enum
COGL_PIPELINE_STATE_ALPHA_FUNC_REFERENCE_INDEX,
COGL_PIPELINE_STATE_BLEND_INDEX,
COGL_PIPELINE_STATE_DEPTH_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,
@@ -157,6 +158,8 @@ typedef enum _CoglPipelineState
1L<<COGL_PIPELINE_STATE_BLEND_INDEX,
COGL_PIPELINE_STATE_DEPTH =
1L<<COGL_PIPELINE_STATE_DEPTH_INDEX,
+ COGL_PIPELINE_STATE_NON_ZERO_POINT_SIZE =
+ 1L<<COGL_PIPELINE_STATE_NON_ZERO_POINT_SIZE_INDEX,
COGL_PIPELINE_STATE_POINT_SIZE =
1L<<COGL_PIPELINE_STATE_POINT_SIZE_INDEX,
COGL_PIPELINE_STATE_PER_VERTEX_POINT_SIZE =
@@ -201,6 +204,7 @@ typedef enum _CoglPipelineState
COGL_PIPELINE_STATE_ALPHA_FUNC_REFERENCE | \
COGL_PIPELINE_STATE_BLEND | \
COGL_PIPELINE_STATE_DEPTH | \
+ COGL_PIPELINE_STATE_NON_ZERO_POINT_SIZE | \
COGL_PIPELINE_STATE_POINT_SIZE | \
COGL_PIPELINE_STATE_PER_VERTEX_POINT_SIZE | \
COGL_PIPELINE_STATE_LOGIC_OPS | \
@@ -219,11 +223,6 @@ typedef enum _CoglPipelineState
COGL_PIPELINE_STATE_VERTEX_SNIPPETS | \
COGL_PIPELINE_STATE_FRAGMENT_SNIPPETS)
-#define COGL_PIPELINE_STATE_AFFECTS_VERTEX_CODEGEN \
- (COGL_PIPELINE_STATE_LAYERS | \
- COGL_PIPELINE_STATE_PER_VERTEX_POINT_SIZE | \
- COGL_PIPELINE_STATE_VERTEX_SNIPPETS)
-
typedef struct
{
/* Determines what fragments are discarded based on their alpha */
@@ -285,7 +284,8 @@ typedef struct
CoglPipelineBlendState blend_state;
CoglDepthState depth_state;
float point_size;
- CoglBool per_vertex_point_size;
+ unsigned int non_zero_point_size : 1;
+ unsigned int per_vertex_point_size : 1;
CoglPipelineLogicOpsState logic_ops_state;
CoglPipelineCullFaceState cull_face_state;
CoglPipelineUniformsState uniforms_state;
@@ -905,6 +905,9 @@ _cogl_pipeline_init_state_hash_functions (void);
void
_cogl_pipeline_init_layer_state_hash_functions (void);
+CoglPipelineState
+_cogl_pipeline_get_state_for_vertex_codegen (CoglContext *context);
+
CoglPipelineLayerState
_cogl_pipeline_get_layer_state_for_fragment_codegen (CoglContext *context);
diff --git a/cogl/cogl-pipeline-state-private.h b/cogl/cogl-pipeline-state-private.h
index b22223d..cf14f8f 100644
--- a/cogl/cogl-pipeline-state-private.h
+++ b/cogl/cogl-pipeline-state-private.h
@@ -78,6 +78,10 @@ _cogl_pipeline_depth_state_equal (CoglPipeline *authority0,
CoglPipeline *authority1);
CoglBool
+_cogl_pipeline_non_zero_point_size_equal (CoglPipeline *authority0,
+ CoglPipeline *authority1);
+
+CoglBool
_cogl_pipeline_point_size_equal (CoglPipeline *authority0,
CoglPipeline *authority1);
CoglBool
@@ -141,6 +145,10 @@ _cogl_pipeline_hash_depth_state (CoglPipeline *authority,
CoglPipelineHashState *state);
void
+_cogl_pipeline_hash_non_zero_point_size_state (CoglPipeline *authority,
+ CoglPipelineHashState *state);
+
+void
_cogl_pipeline_hash_point_size_state (CoglPipeline *authority,
CoglPipelineHashState *state);
diff --git a/cogl/cogl-pipeline-state.c b/cogl/cogl-pipeline-state.c
index 3f4eb43..b5e391b 100644
--- a/cogl/cogl-pipeline-state.c
+++ b/cogl/cogl-pipeline-state.c
@@ -138,6 +138,14 @@ _cogl_pipeline_depth_state_equal (CoglPipeline *authority0,
}
CoglBool
+_cogl_pipeline_non_zero_point_size_equal (CoglPipeline *authority0,
+ CoglPipeline *authority1)
+{
+ return (authority0->big_state->non_zero_point_size ==
+ authority1->big_state->non_zero_point_size);
+}
+
+CoglBool
_cogl_pipeline_point_size_equal (CoglPipeline *authority0,
CoglPipeline *authority1)
{
@@ -940,6 +948,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)
@@ -954,6 +986,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
@@ -1413,6 +1451,17 @@ _cogl_pipeline_hash_depth_state (CoglPipeline *authority,
}
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 ab5d8da..be6f7c9 100644
--- a/cogl/cogl-pipeline-state.h
+++ b/cogl/cogl-pipeline-state.h
@@ -302,12 +302,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 19a8a19..e843cd6 100644
--- a/cogl/cogl-pipeline.c
+++ b/cogl/cogl-pipeline.c
@@ -190,7 +190,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;
@@ -919,6 +919,9 @@ _cogl_pipeline_copy_differences (CoglPipeline *dest,
sizeof (CoglDepthState));
}
+ 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;
@@ -1006,6 +1009,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_PER_VERTEX_POINT_SIZE:
case COGL_PIPELINE_STATE_REAL_BLEND_ENABLE:
@@ -2164,6 +2168,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]))
@@ -2526,6 +2535,8 @@ _cogl_pipeline_init_state_hash_functions (void)
_cogl_pipeline_hash_depth_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] =
@@ -2541,7 +2552,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 == 14,
+ _COGL_STATIC_ASSERT (COGL_PIPELINE_STATE_SPARSE_COUNT == 15,
"Make sure to install a hash function for "
"newly added pipeline state and update assert "
"in _cogl_pipeline_init_state_hash_functions");
@@ -2808,6 +2819,24 @@ _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_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 c4e4bc2..6051bb0 100644
--- a/cogl/driver/gl/cogl-pipeline-progend-glsl.c
+++ b/cogl/driver/gl/cogl-pipeline-progend-glsl.c
@@ -617,7 +617,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) |
@@ -756,8 +756,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 bc7d6c1..d6fa784 100644
--- a/cogl/driver/gl/cogl-pipeline-vertend-fixed.c
+++ b/cogl/driver/gl/cogl-pipeline-vertend-fixed.c
@@ -71,7 +71,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 c5a5684..035ad4a 100644
--- a/cogl/driver/gl/cogl-pipeline-vertend-glsl.c
+++ b/cogl/driver/gl/cogl-pipeline-vertend-glsl.c
@@ -31,6 +31,8 @@
#include <string.h>
+#include <test-fixtures/test-unit.h>
+
#include "cogl-context-private.h"
#include "cogl-util-gl-private.h"
#include "cogl-pipeline-private.h"
@@ -214,7 +216,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);
@@ -286,12 +288,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");
+ }
}
}
@@ -509,7 +517,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 */
@@ -521,7 +530,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);
}
@@ -565,4 +576,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 */
--
1.7.11.3.g3c3efa5
More information about the Cogl
mailing list