[Cogl] [PATCH] Reorder some struct members to avoid padding due to alignment
Robert Bragg
robert at sixbynine.org
Wed Jan 16 06:41:38 PST 2013
One initial concern I had was that the differences members for
CoglPipeline and CoglPipelineLayer might be used with the COGL_FLAGS_
macros somewhere and those expect to deal with longs. A quick grep
makes me think that actually we don't, although I was a bit surprised
by that so maybe it's also worth double checking that's the case.
Assuming that's fine then this patch looks good to land to me:
Reviewed-by: Robert Bragg <robert at linux.intel.com>
thanks,
- Robert
On Thu, Jan 10, 2013 at 2:54 PM, Neil Roberts <neil at linux.intel.com> wrote:
> This tweaks the ordering of some struct members in some of the more
> important structs so that the compiler won't insert wasted padding to
> avoid breaking the alignment. Some members that were previously
> unsigned long have been changed to unsigned int. These members need to
> be able to fit in 32-bits to run on 32-bit machines anyway so there's
> no point in having them extend to 64-bit on 64-bit machines. This
> doesn't affect the public API.
> ---
> cogl/cogl-clip-stack.h | 10 +++++-----
> cogl/cogl-journal-private.h | 2 +-
> cogl/cogl-matrix-stack-private.h | 2 +-
> cogl/cogl-pipeline-layer-private.h | 12 ++++++------
> cogl/cogl-pipeline-private.h | 18 +++++++++---------
> cogl/cogl-primitive-private.h | 2 +-
> 6 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/cogl/cogl-clip-stack.h b/cogl/cogl-clip-stack.h
> index 48ab800..7c2ee98 100644
> --- a/cogl/cogl-clip-stack.h
> +++ b/cogl/cogl-clip-stack.h
> @@ -92,12 +92,12 @@ typedef enum
>
> struct _CoglClipStack
> {
> - CoglClipStackType type;
> -
> /* This will be null if there is no parent. If it is not null then
> this node must be holding a reference to the parent */
> CoglClipStack *parent;
>
> + CoglClipStackType type;
> +
> /* All clip entries have a window-space bounding box which we can
> use to calculate a scissor. The scissor limits the clip so that
> we don't need to do a full stencil clear if the stencil buffer is
> @@ -121,6 +121,9 @@ struct _CoglClipStackRect
> float x1;
> float y1;
>
> + /* The matrix that was current when the clip was set */
> + CoglMatrixEntry *matrix_entry;
> +
> /* If this is true then the clip for this rectangle is entirely
> described by the scissor bounds. This implies that the rectangle
> is screen aligned and we don't need to use the stencil buffer to
> @@ -130,9 +133,6 @@ struct _CoglClipStackRect
> journal. In that case we can use the original clip coordinates
> and modify the rectangle instead. */
> CoglBool can_be_scissor;
> -
> - /* The matrix that was current when the clip was set */
> - CoglMatrixEntry *matrix_entry;
> };
>
> struct _CoglClipStackWindowRect
> diff --git a/cogl/cogl-journal-private.h b/cogl/cogl-journal-private.h
> index f5190f6..6f839d3 100644
> --- a/cogl/cogl-journal-private.h
> +++ b/cogl/cogl-journal-private.h
> @@ -66,11 +66,11 @@ typedef struct _CoglJournal
> typedef struct _CoglJournalEntry
> {
> CoglPipeline *pipeline;
> - int n_layers;
> CoglMatrixEntry *modelview_entry;
> CoglClipStack *clip_stack;
> /* Offset into ctx->logged_vertices */
> size_t array_offset;
> + int n_layers;
> } CoglJournalEntry;
>
> CoglJournal *
> diff --git a/cogl/cogl-matrix-stack-private.h b/cogl/cogl-matrix-stack-private.h
> index 5d0e911..33a32d1 100644
> --- a/cogl/cogl-matrix-stack-private.h
> +++ b/cogl/cogl-matrix-stack-private.h
> @@ -130,8 +130,8 @@ typedef struct _CoglMatrixEntrySave
> {
> CoglMatrixEntry _parent_data;
>
> - CoglBool cache_valid;
> CoglMatrix *cache;
> + CoglBool cache_valid;
>
> } CoglMatrixEntrySave;
>
> diff --git a/cogl/cogl-pipeline-layer-private.h b/cogl/cogl-pipeline-layer-private.h
> index d56e3f4..ca69168 100644
> --- a/cogl/cogl-pipeline-layer-private.h
> +++ b/cogl/cogl-pipeline-layer-private.h
> @@ -172,10 +172,10 @@ typedef struct
>
> float texture_combine_constant[4];
>
> - CoglBool point_sprite_coords;
> -
> CoglPipelineSnippetList vertex_snippets;
> CoglPipelineSnippetList fragment_snippets;
> +
> + CoglBool point_sprite_coords;
> } CoglPipelineLayerBigState;
>
> struct _CoglPipelineLayer
> @@ -211,7 +211,7 @@ struct _CoglPipelineLayer
>
> /* A mask of which state groups are different in this layer
> * in comparison to its parent. */
> - unsigned long differences;
> + unsigned int differences;
>
> /* Common differences
> *
> @@ -224,14 +224,14 @@ struct _CoglPipelineLayer
> /* Each layer is directly associated with a single texture unit */
> int unit_index;
>
> - /* The texture for this layer, or NULL for an empty
> - * layer */
> - CoglTexture *texture;
> /* The type of the texture. This is always set even if the texture
> is NULL and it will be used to determine what type of texture
> lookups to use in any shaders generated by the pipeline
> backends. */
> CoglTextureType texture_type;
> + /* The texture for this layer, or NULL for an empty
> + * layer */
> + CoglTexture *texture;
>
> const CoglSamplerCacheEntry *sampler_cache_entry;
>
> diff --git a/cogl/cogl-pipeline-private.h b/cogl/cogl-pipeline-private.h
> index 6cf3531..32f6d51 100644
> --- a/cogl/cogl-pipeline-private.h
> +++ b/cogl/cogl-pipeline-private.h
> @@ -333,11 +333,6 @@ struct _CoglPipeline
> * type to track the tree heirachy so we can share code... */
> CoglNode _parent;
>
> - /* We need to track if a pipeline is referenced in the journal
> - * because we can't allow modification to these pipelines without
> - * flushing the journal first */
> - unsigned long journal_ref_count;
> -
> /* When weak pipelines are destroyed the user is notified via this
> * callback */
> CoglPipelineDestroyCallback destroy_callback;
> @@ -346,28 +341,33 @@ struct _CoglPipeline
> * private data is passed to the above callback */
> void *destroy_data;
>
> + /* We need to track if a pipeline is referenced in the journal
> + * because we can't allow modification to these pipelines without
> + * flushing the journal first */
> + unsigned int journal_ref_count;
> +
> /* A mask of which sparse state groups are different in this
> * pipeline in comparison to its parent. */
> - unsigned long differences;
> + unsigned int differences;
>
> /* Whenever a pipeline is modified we increment the age. There's no
> * guarantee that it won't wrap but it can nevertheless be a
> * convenient mechanism to determine when a pipeline has been
> * changed to you can invalidate some some associated cache that
> * depends on the old state. */
> - unsigned long age;
> + unsigned int age;
>
> /* This is the primary color of the pipeline.
> *
> * This is a sparse property, ref COGL_PIPELINE_STATE_COLOR */
> - CoglColor color;
> + CoglColor color;
>
> /* A pipeline may be made up with multiple layers used to combine
> * textures together.
> *
> * This is sparse state, ref COGL_PIPELINE_STATE_LAYERS */
> - GList *layer_differences;
> unsigned int n_layers;
> + GList *layer_differences;
>
> /* As a basic way to reduce memory usage we divide the pipeline
> * state into two groups; the minimal state modified in 90% of
> diff --git a/cogl/cogl-primitive-private.h b/cogl/cogl-primitive-private.h
> index c3fd991..9d03a6b 100644
> --- a/cogl/cogl-primitive-private.h
> +++ b/cogl/cogl-primitive-private.h
> @@ -36,10 +36,10 @@ struct _CoglPrimitive
> {
> CoglObject _parent;
>
> + CoglIndices *indices;
> CoglVerticesMode mode;
> int first_vertex;
> int n_vertices;
> - CoglIndices *indices;
>
> int immutable_ref;
>
> --
> 1.7.11.3.g3c3efa5
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl
More information about the Cogl
mailing list