From f15b47cc03b2f8f2388b785706d61e2f2c05707a Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Mon, 1 Nov 2010 19:52:45 +0000 Subject: [PATCH] cogl-clip-stack: Don't allocate a separate struct for CoglClipStack Instead of having a separate CoglHandle for CoglClipStack the code is now expected to directly hold a pointer to the top entry on the stack. The empty stack is then the NULL pointer. This saves an allocation when we want to copy the stack because we can just take a reference on a stack entry. The idea is that this will make it possible to store the clip stack in the journal without any extra allocations. The _cogl_get_clip_stack and set functions now take a CoglClipStack pointer instead of a handle so it would no longer make sense to make them public. However I think the only reason we would have wanted that in the first place would be to save the clip state between switching FBOs and that is no longer necessary. --- cogl/cogl-clip-stack.c | 193 ++++++++++++++++------------------------- cogl/cogl-clip-stack.h | 38 ++++---- cogl/cogl-clip-state.c | 55 +++++------- cogl/cogl-clip-state.h | 20 ++--- 4 files changed, 116 insertions(+), 190 deletions(-) diff --git a/cogl/cogl-clip-stack.c b/cogl/cogl-clip-stack.c index 0c36b34fd..0a2ccc88e 100644 --- a/cogl/cogl-clip-stack.c +++ b/cogl/cogl-clip-stack.c @@ -41,17 +41,16 @@ #include "cogl-path-private.h" #include "cogl-matrix-private.h" -typedef struct _CoglClipStackEntry CoglClipStackEntry; -typedef struct _CoglClipStackEntryRect CoglClipStackEntryRect; -typedef struct _CoglClipStackEntryWindowRect CoglClipStackEntryWindowRect; -typedef struct _CoglClipStackEntryPath CoglClipStackEntryPath; +typedef struct _CoglClipStackRect CoglClipStackRect; +typedef struct _CoglClipStackWindowRect CoglClipStackWindowRect; +typedef struct _CoglClipStackPath CoglClipStackPath; typedef enum { COGL_CLIP_STACK_RECT, COGL_CLIP_STACK_WINDOW_RECT, COGL_CLIP_STACK_PATH - } CoglClipStackEntryType; + } CoglClipStackType; /* A clip stack consists a list of entries. Each entry has a reference * count and a link to its parent node. The child takes a reference on @@ -63,14 +62,12 @@ typedef enum * For example, the following sequence of operations would generate * the tree below: * - * CoglClipStack *stack_a = _cogl_clip_stack_new (); - * _cogl_set_clip_stack (stack_a); - * cogl_clip_stack_push_rectangle (...); - * cogl_clip_stack_push_rectangle (...); - * CoglClipStack *stack_b = _cogl_clip_stack_copy (stack_a); - * cogl_clip_stack_push_from_path (); - * cogl_set_clip_stack (stack_b); - * cogl_clip_stack_push_window_rectangle (...); + * CoglClipStack *stack_a = NULL; + * stack_a = _cogl_clip_stack_push_rectangle (stack_a, ...); + * stack_a = _cogl_clip_stack_push_rectangle (stack_a, ...); + * stack_a = _cogl_clip_stack_push_from_path (stack_a, ...); + * CoglClipStack *stack_b = NULL; + * stack_b = cogl_clip_stack_push_window_rectangle (stack_b, ...); * * stack_a * \ holds a ref to @@ -95,18 +92,11 @@ typedef enum struct _CoglClipStack { - CoglObject _parent; - - CoglClipStackEntry *stack_top; -}; - -struct _CoglClipStackEntry -{ - CoglClipStackEntryType type; + 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 */ - CoglClipStackEntry *parent; + CoglClipStack *parent; /* All clip entries have a window-space bounding box which we can use to calculate a scissor. The scissor limits the clip so that @@ -121,9 +111,9 @@ struct _CoglClipStackEntry unsigned int ref_count; }; -struct _CoglClipStackEntryRect +struct _CoglClipStackRect { - CoglClipStackEntry _parent_data; + CoglClipStack _parent_data; /* The rectangle for this clip */ float x0; @@ -135,17 +125,17 @@ struct _CoglClipStackEntryRect CoglMatrix matrix; }; -struct _CoglClipStackEntryWindowRect +struct _CoglClipStackWindowRect { - CoglClipStackEntry _parent_data; + CoglClipStack _parent_data; /* The window rect clip doesn't need any specific data because it just adds to the scissor clip */ }; -struct _CoglClipStackEntryPath +struct _CoglClipStackPath { - CoglClipStackEntry _parent_data; + CoglClipStack _parent_data; /* The matrix that was current when the clip was set */ CoglMatrix matrix; @@ -153,12 +143,6 @@ struct _CoglClipStackEntryPath CoglPath *path; }; -static void _cogl_clip_stack_free (CoglClipStack *stack); - -COGL_OBJECT_INTERNAL_DEFINE (ClipStack, clip_stack); - -#define COGL_CLIP_STACK(stack) ((CoglClipStack *) (stack)) - static void project_vertex (const CoglMatrix *modelview_matrix, const CoglMatrix *projection_matrix, @@ -398,20 +382,18 @@ disable_clip_planes (void) static gpointer _cogl_clip_stack_push_entry (CoglClipStack *clip_stack, size_t size, - CoglClipStackEntryType type) + CoglClipStackType type) { - CoglClipStackEntry *entry = g_slice_alloc (size); + CoglClipStack *entry = g_slice_alloc (size); /* The new entry starts with a ref count of 1 because the stack holds a reference to it as it is the top entry */ entry->ref_count = 1; entry->type = type; - entry->parent = clip_stack->stack_top; - clip_stack->stack_top = entry; + entry->parent = clip_stack; /* We don't need to take a reference to the parent from the entry - because the clip_stack would have had to reference the top of - the stack and we can just steal that */ + because the we are stealing the ref in the new stack top */ return entry; } @@ -419,7 +401,7 @@ _cogl_clip_stack_push_entry (CoglClipStack *clip_stack, /* Sets the window-space bounds of the entry based on the projected coordinates of the given rectangle */ static void -_cogl_clip_stack_entry_set_bounds (CoglClipStackEntry *entry, +_cogl_clip_stack_entry_set_bounds (CoglClipStack *entry, float x_1, float y_1, float x_2, @@ -459,28 +441,28 @@ _cogl_clip_stack_entry_set_bounds (CoglClipStackEntry *entry, entry->bounds_y1 = ceilf (max_y); } -void +CoglClipStack * _cogl_clip_stack_push_window_rectangle (CoglClipStack *stack, int x_offset, int y_offset, int width, int height) { - CoglClipStackEntry *entry; - - _COGL_GET_CONTEXT (ctx, NO_RETVAL); + CoglClipStack *entry; entry = _cogl_clip_stack_push_entry (stack, - sizeof (CoglClipStackEntryWindowRect), + sizeof (CoglClipStackWindowRect), COGL_CLIP_STACK_WINDOW_RECT); entry->bounds_x0 = x_offset; entry->bounds_x1 = x_offset + width; entry->bounds_y0 = y_offset; entry->bounds_y1 = y_offset + height; + + return entry; } -void +CoglClipStack * _cogl_clip_stack_push_rectangle (CoglClipStack *stack, float x_1, float y_1, @@ -488,11 +470,11 @@ _cogl_clip_stack_push_rectangle (CoglClipStack *stack, float y_2, const CoglMatrix *modelview_matrix) { - CoglClipStackEntryRect *entry; + CoglClipStackRect *entry; /* Make a new entry */ entry = _cogl_clip_stack_push_entry (stack, - sizeof (CoglClipStackEntryRect), + sizeof (CoglClipStackRect), COGL_CLIP_STACK_RECT); entry->x0 = x_1; @@ -502,20 +484,22 @@ _cogl_clip_stack_push_rectangle (CoglClipStack *stack, entry->matrix = *modelview_matrix; - _cogl_clip_stack_entry_set_bounds ((CoglClipStackEntry *) entry, + _cogl_clip_stack_entry_set_bounds ((CoglClipStack *) entry, x_1, y_1, x_2, y_2, modelview_matrix); + + return (CoglClipStack *) entry; } -void +CoglClipStack * _cogl_clip_stack_push_from_path (CoglClipStack *stack, CoglPath *path, const CoglMatrix *modelview_matrix) { - CoglClipStackEntryPath *entry; + CoglClipStackPath *entry; float x_1, y_1, x_2, y_2; entry = _cogl_clip_stack_push_entry (stack, - sizeof (CoglClipStackEntryPath), + sizeof (CoglClipStackPath), COGL_CLIP_STACK_PATH); entry->path = cogl_path_copy (path); @@ -524,32 +508,45 @@ _cogl_clip_stack_push_from_path (CoglClipStack *stack, _cogl_path_get_bounds (path, &x_1, &y_1, &x_2, &y_2); - _cogl_clip_stack_entry_set_bounds ((CoglClipStackEntry *) entry, + _cogl_clip_stack_entry_set_bounds ((CoglClipStack *) entry, x_1, y_1, x_2, y_2, modelview_matrix); + + return (CoglClipStack *) entry; } -static void -_cogl_clip_stack_entry_unref (CoglClipStackEntry *entry) +CoglClipStack * +_cogl_clip_stack_ref (CoglClipStack *entry) +{ + /* A NULL pointer is considered a valid stack so we should accept + that as an argument */ + if (entry) + entry->ref_count++; + + return entry; +} + +void +_cogl_clip_stack_unref (CoglClipStack *entry) { /* Unref all of the entries until we hit the root of the list or the entry still has a remaining reference */ while (entry && --entry->ref_count <= 0) { - CoglClipStackEntry *parent = entry->parent; + CoglClipStack *parent = entry->parent; switch (entry->type) { case COGL_CLIP_STACK_RECT: - g_slice_free1 (sizeof (CoglClipStackEntryRect), entry); + g_slice_free1 (sizeof (CoglClipStackRect), entry); break; case COGL_CLIP_STACK_WINDOW_RECT: - g_slice_free1 (sizeof (CoglClipStackEntryWindowRect), entry); + g_slice_free1 (sizeof (CoglClipStackWindowRect), entry); break; case COGL_CLIP_STACK_PATH: - cogl_object_unref (((CoglClipStackEntryPath *) entry)->path); - g_slice_free1 (sizeof (CoglClipStackEntryPath), entry); + cogl_object_unref (((CoglClipStackPath *) entry)->path); + g_slice_free1 (sizeof (CoglClipStackPath), entry); break; default: @@ -560,12 +557,12 @@ _cogl_clip_stack_entry_unref (CoglClipStackEntry *entry) } } -void +CoglClipStack * _cogl_clip_stack_pop (CoglClipStack *stack) { - CoglClipStackEntry *entry; + CoglClipStack *new_top; - g_return_if_fail (stack->stack_top != NULL); + g_return_val_if_fail (stack != NULL, NULL); /* To pop we are moving the top of the stack to the old top's parent node. The stack always needs to have a reference to the top entry @@ -575,11 +572,13 @@ _cogl_clip_stack_pop (CoglClipStack *stack) this stack was the only thing referencing the old top. In that case the call to _cogl_clip_stack_entry_unref will unref the parent. */ - entry = stack->stack_top; - stack->stack_top = entry->parent; - if (stack->stack_top) - stack->stack_top->ref_count++; - _cogl_clip_stack_entry_unref (entry); + new_top = stack->parent; + + _cogl_clip_stack_ref (new_top); + + _cogl_clip_stack_unref (stack); + + return new_top; } void @@ -595,7 +594,7 @@ _cogl_clip_stack_flush (CoglClipStack *stack, int scissor_y1 = G_MAXINT; CoglMatrixStack *modelview_stack = _cogl_framebuffer_get_modelview_stack (_cogl_get_framebuffer ()); - CoglClipStackEntry *entry; + CoglClipStack *entry; int scissor_y_start; has_clip_planes = cogl_features_available (COGL_FEATURE_FOUR_CLIP_PLANES); @@ -605,7 +604,7 @@ _cogl_clip_stack_flush (CoglClipStack *stack, disable_stencil_buffer (); /* If the stack is empty then there's nothing else to do */ - if (stack->stack_top == NULL) + if (stack == NULL) { *stencil_used_p = FALSE; GE (glDisable (GL_SCISSOR_TEST)); @@ -616,7 +615,7 @@ _cogl_clip_stack_flush (CoglClipStack *stack, clear the stencil buffer then the clear will be clipped to the intersection of all of the bounding boxes. This saves having to clear the whole stencil buffer */ - for (entry = stack->stack_top; entry; entry = entry->parent) + for (entry = stack; entry; entry = entry->parent) { /* Get the intersection of the current scissor and the bounding box of this clip */ @@ -661,11 +660,11 @@ _cogl_clip_stack_flush (CoglClipStack *stack, reverse order that they were specified but as all of the clips are intersecting it should work out the same regardless of the order */ - for (entry = stack->stack_top; entry; entry = entry->parent) + for (entry = stack; entry; entry = entry->parent) { if (entry->type == COGL_CLIP_STACK_PATH) { - CoglClipStackEntryPath *path_entry = (CoglClipStackEntryPath *) entry; + CoglClipStackPath *path_entry = (CoglClipStackPath *) entry; _cogl_matrix_stack_push (modelview_stack); _cogl_matrix_stack_set (modelview_stack, &path_entry->matrix); @@ -680,7 +679,7 @@ _cogl_clip_stack_flush (CoglClipStack *stack, } else if (entry->type == COGL_CLIP_STACK_RECT) { - CoglClipStackEntryRect *rect = (CoglClipStackEntryRect *) entry; + CoglClipStackRect *rect = (CoglClipStackRect *) entry; _cogl_matrix_stack_push (modelview_stack); _cogl_matrix_stack_set (modelview_stack, &rect->matrix); @@ -721,47 +720,3 @@ _cogl_clip_stack_flush (CoglClipStack *stack, *stencil_used_p = using_stencil_buffer; } - -CoglClipStack * -_cogl_clip_stack_new (void) -{ - CoglClipStack *stack; - - stack = g_slice_new (CoglClipStack); - stack->stack_top = NULL; - - return _cogl_clip_stack_object_new (stack); -} - -void -_cogl_clip_stack_free (CoglClipStack *stack) -{ - /* We only need to unref the top node and this - should end up freeing all of the parents if need be */ - if (stack->stack_top) - _cogl_clip_stack_entry_unref (stack->stack_top); - - g_slice_free (CoglClipStack, stack); -} - -CoglClipStack * -_cogl_clip_stack_copy (CoglClipStack *old_stack) -{ - CoglClipStack *new_stack; - - if (!_cogl_is_clip_stack (old_stack)) - return NULL; - - new_stack = _cogl_clip_stack_new (); - - /* We can copy the stack by just referencing the other stack's - data. There's no need to implement copy-on-write because the - entries of the stack can't be modified. If the other stack pops - some entries off they will still be kept alive because this stack - holds a reference. */ - new_stack->stack_top = old_stack->stack_top; - if (new_stack->stack_top) - new_stack->stack_top->ref_count++; - - return new_stack; -} diff --git a/cogl/cogl-clip-stack.h b/cogl/cogl-clip-stack.h index 90625fdca..a52512e97 100644 --- a/cogl/cogl-clip-stack.h +++ b/cogl/cogl-clip-stack.h @@ -24,19 +24,24 @@ #ifndef __COGL_CLIP_STACK_H #define __COGL_CLIP_STACK_H +/* The clip stack works like a GSList where only a pointer to the top + of the stack is stored. The empty clip stack is represented simply + by the NULL pointer. When an entry is added to or removed from the + stack the new top of the stack is returned. When an entry is pushed + a new clip stack entry is created which effectively takes ownership + of the reference on the old entry. Therefore unrefing the top entry + effectively loses ownership of all entries in the stack */ + typedef struct _CoglClipStack CoglClipStack; CoglClipStack * -_cogl_clip_stack_new (void); - -void _cogl_clip_stack_push_window_rectangle (CoglClipStack *stack, int x_offset, int y_offset, int width, int height); -void +CoglClipStack * _cogl_clip_stack_push_rectangle (CoglClipStack *stack, float x_1, float y_1, @@ -44,34 +49,21 @@ _cogl_clip_stack_push_rectangle (CoglClipStack *stack, float y_2, const CoglMatrix *modelview_matrix); -void +CoglClipStack * _cogl_clip_stack_push_from_path (CoglClipStack *stack, CoglPath *path, const CoglMatrix *modelview_matrix); -void +CoglClipStack * _cogl_clip_stack_pop (CoglClipStack *stack); void _cogl_clip_stack_flush (CoglClipStack *stack, gboolean *stencil_used_p); - -/* TODO: we may want to make this function public because it can be - * used to implement a better API than cogl_clip_stack_save() and - * cogl_clip_stack_restore(). - */ -/* - * _cogl_clip_stack_copy: - * @stack: A #CoglClipStack - * - * Creates a copy of the given clip stack and returns a new pointer to - * it. The data from the original stack is shared with the new stack - * so making copies is relatively cheap. Modifying the original stack - * does not affect the new stack. - * - * Return value: a new clip stack with the same data as @stack - */ CoglClipStack * -_cogl_clip_stack_copy (CoglClipStack *stack); +_cogl_clip_stack_ref (CoglClipStack *stack); + +void +_cogl_clip_stack_unref (CoglClipStack *stack); #endif /* __COGL_CLIP_STACK_H */ diff --git a/cogl/cogl-clip-state.c b/cogl/cogl-clip-state.c index 317089ef4..18850e09a 100644 --- a/cogl/cogl-clip-state.c +++ b/cogl/cogl-clip-state.c @@ -48,7 +48,6 @@ cogl_clip_push_window_rectangle (int x_offset, { CoglFramebuffer *framebuffer; CoglClipState *clip_state; - CoglHandle stack; _COGL_GET_CONTEXT (ctx, NO_RETVAL); @@ -59,10 +58,10 @@ cogl_clip_push_window_rectangle (int x_offset, framebuffer = _cogl_get_framebuffer (); clip_state = _cogl_framebuffer_get_clip_state (framebuffer); - stack = clip_state->stacks->data; - - _cogl_clip_stack_push_window_rectangle (stack, x_offset, y_offset, - width, height); + clip_state->stacks->data = + _cogl_clip_stack_push_window_rectangle (clip_state->stacks->data, + x_offset, y_offset, + width, height); clip_state->stack_dirty = TRUE; } @@ -135,7 +134,6 @@ cogl_clip_push_rectangle (float x_1, { CoglFramebuffer *framebuffer; CoglClipState *clip_state; - CoglHandle stack; CoglMatrix modelview_matrix; _COGL_GET_CONTEXT (ctx, NO_RETVAL); @@ -152,12 +150,12 @@ cogl_clip_push_rectangle (float x_1, framebuffer = _cogl_get_framebuffer (); clip_state = _cogl_framebuffer_get_clip_state (framebuffer); - stack = clip_state->stacks->data; - cogl_get_modelview_matrix (&modelview_matrix); - _cogl_clip_stack_push_rectangle (stack, x_1, y_1, x_2, y_2, - &modelview_matrix); + clip_state->stacks->data = + _cogl_clip_stack_push_rectangle (clip_state->stacks->data, + x_1, y_1, x_2, y_2, + &modelview_matrix); clip_state->stack_dirty = TRUE; } @@ -180,7 +178,6 @@ cogl_clip_push_from_path_preserve (void) { CoglFramebuffer *framebuffer; CoglClipState *clip_state; - CoglHandle stack; CoglMatrix modelview_matrix; _COGL_GET_CONTEXT (ctx, NO_RETVAL); @@ -192,12 +189,11 @@ cogl_clip_push_from_path_preserve (void) framebuffer = _cogl_get_framebuffer (); clip_state = _cogl_framebuffer_get_clip_state (framebuffer); - stack = clip_state->stacks->data; - cogl_get_modelview_matrix (&modelview_matrix); - _cogl_clip_stack_push_from_path (stack, cogl_get_path (), - &modelview_matrix); + clip_state->stacks->data = + _cogl_clip_stack_push_from_path (clip_state->stacks->data, cogl_get_path (), + &modelview_matrix); clip_state->stack_dirty = TRUE; } @@ -213,15 +209,11 @@ cogl_clip_push_from_path (void) static void _cogl_clip_pop_real (CoglClipState *clip_state) { - CoglHandle stack; - /* We don't log clip stack changes in the journal so we must flush * it before making modifications */ _cogl_journal_flush (); - stack = clip_state->stacks->data; - - _cogl_clip_stack_pop (stack); + clip_state->stacks->data = _cogl_clip_stack_pop (clip_state->stacks->data); clip_state->stack_dirty = TRUE; } @@ -243,7 +235,7 @@ cogl_clip_pop (void) void _cogl_clip_state_flush (CoglClipState *clip_state) { - CoglHandle stack; + CoglClipStack *stack; if (!clip_state->stack_dirty) return; @@ -277,15 +269,11 @@ cogl_clip_ensure (void) static void _cogl_clip_stack_save_real (CoglClipState *clip_state) { - CoglHandle stack; - /* We don't log clip stack changes in the journal so we must flush * it before making modifications */ _cogl_journal_flush (); - stack = _cogl_clip_stack_new (); - - clip_state->stacks = g_slist_prepend (clip_state->stacks, stack); + clip_state->stacks = g_slist_prepend (clip_state->stacks, NULL); clip_state->stack_dirty = TRUE; } @@ -316,7 +304,7 @@ _cogl_clip_stack_restore_real (CoglClipState *clip_state) stack = clip_state->stacks->data; - cogl_handle_unref (stack); + _cogl_clip_stack_unref (stack); /* Revert to an old stack */ clip_state->stacks = g_slist_delete_link (clip_state->stacks, @@ -365,7 +353,7 @@ _cogl_clip_state_dirty (CoglClipState *clip_state) clip_state->stack_dirty = TRUE; } -CoglHandle +CoglClipStack * _cogl_get_clip_stack (void) { CoglFramebuffer *framebuffer; @@ -380,21 +368,18 @@ _cogl_get_clip_stack (void) } void -_cogl_set_clip_stack (CoglHandle handle) +_cogl_set_clip_stack (CoglClipStack *stack) { CoglFramebuffer *framebuffer; CoglClipState *clip_state; _COGL_GET_CONTEXT (ctx, NO_RETVAL); - if (handle == COGL_INVALID_HANDLE) - return; - framebuffer = _cogl_get_framebuffer (); clip_state = _cogl_framebuffer_get_clip_state (framebuffer); /* Replace the top of the stack of stacks */ - cogl_handle_ref (handle); - cogl_handle_unref (clip_state->stacks->data); - clip_state->stacks->data = handle; + _cogl_clip_stack_ref (stack); + _cogl_clip_stack_unref (clip_state->stacks->data); + clip_state->stacks->data = stack; } diff --git a/cogl/cogl-clip-state.h b/cogl/cogl-clip-state.h index dc5e80b76..dd09449fa 100644 --- a/cogl/cogl-clip-state.h +++ b/cogl/cogl-clip-state.h @@ -47,32 +47,26 @@ _cogl_clip_state_dirty (CoglClipState *state); void _cogl_clip_state_flush (CoglClipState *clip_state); -/* TODO: we may want to make these two functions public because they - * can be used to implement a better API than cogl_clip_stack_save() - * and cogl_clip_stack_restore(). - */ /* * _cogl_get_clip_stack: * - * Gets a handle to the current clip stack. This can be used to later + * Gets a pointer to the current clip stack. This can be used to later * return to the same clip stack state with _cogl_set_clip_stack(). A * reference is not taken on the stack so if you want to keep it you - * should call cogl_handle_ref() or _cogl_clip_stack_copy(). + * should call _cogl_clip_stack_ref(). * - * Return value: a handle to the current clip stack. + * Return value: a pointer to the current clip stack. */ -CoglHandle +CoglClipStack * _cogl_get_clip_stack (void); /* * _cogl_set_clip_stack: - * @handle: a handle to the replacement clip stack + * @stack: a pointer to the replacement clip stack * - * Replaces the current clip stack with @handle. - * - * Return value: a handle to the current clip stack. + * Replaces the current clip stack with @stack. */ void -_cogl_set_clip_stack (CoglHandle handle); +_cogl_set_clip_stack (CoglClipStack *stack); #endif /* __COGL_CLIP_STATE_H */