Don't generate GLSL for the point size for default pipelines

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.

Reviewed-by: Robert Bragg <robert@linux.intel.com>

(cherry picked from commit b2eba06e16b587acbf5c57944a70ceccecb4f175)

Conflicts:
	cogl/cogl-pipeline-private.h
	cogl/cogl-pipeline-state-private.h
	cogl/cogl-pipeline-state.c
	cogl/cogl-pipeline.c
This commit is contained in:
Neil Roberts 2013-06-20 13:25:49 +01:00
parent 1455561a20
commit 2933423ca7
9 changed files with 194 additions and 29 deletions

View File

@ -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 =

View File

@ -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<<COGL_PIPELINE_STATE_DEPTH_INDEX,
COGL_PIPELINE_STATE_FOG =
1L<<COGL_PIPELINE_STATE_FOG_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 =
@ -215,6 +218,7 @@ typedef enum _CoglPipelineState
COGL_PIPELINE_STATE_USER_SHADER | \
COGL_PIPELINE_STATE_DEPTH | \
COGL_PIPELINE_STATE_FOG | \
COGL_PIPELINE_STATE_NON_ZERO_POINT_SIZE | \
COGL_PIPELINE_STATE_POINT_SIZE | \
COGL_PIPELINE_STATE_PER_VERTEX_POINT_SIZE | \
COGL_PIPELINE_STATE_LOGIC_OPS | \
@ -235,12 +239,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_USER_SHADER | \
COGL_PIPELINE_STATE_PER_VERTEX_POINT_SIZE | \
COGL_PIPELINE_STATE_VERTEX_SNIPPETS)
typedef enum
{
COGL_PIPELINE_LIGHTING_STATE_PROPERTY_AMBIENT = 1,
@ -334,7 +332,8 @@ typedef struct
CoglDepthState depth_state;
CoglPipelineFogState fog_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;
@ -980,6 +979,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);

View File

@ -75,6 +75,10 @@ CoglBool
_cogl_pipeline_fog_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);
@ -146,6 +150,10 @@ void
_cogl_pipeline_hash_fog_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);

View File

@ -189,6 +189,14 @@ _cogl_pipeline_fog_state_equal (CoglPipeline *authority0,
return FALSE;
}
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)
@ -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)

View File

@ -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

View File

@ -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)
{

View File

@ -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);
}

View File

@ -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;

View File

@ -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"
@ -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 */