From d42f1873fcd0876244eb8468d72ce35459ba94ca Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Tue, 24 Jan 2012 18:16:03 +0000 Subject: [PATCH] framebuffer: Flush the journal on destruction Instead of flushing the journal whenever the current framebuffer on a context is changed it is now flushed whenever the framebuffer is about to be destroyed instead. To do this it implements a custom unref function which detects when there is going to be exactly one reference on the framebuffer and then flushes its journal. The journal now always has a reference on the framebuffer whenever it is non-empty. That means the unref will only cause a flush if the only thing keeping the framebuffer alive is the entries in the journal. Reviewed-by: Robert Bragg --- cogl/cogl-framebuffer-private.h | 3 ++ cogl/cogl-framebuffer.c | 63 +++++++++++++++++++-------------- cogl/cogl-journal-private.h | 8 +++++ cogl/cogl-journal.c | 51 ++++++++++++++++++-------- cogl/cogl-onscreen.c | 4 ++- 5 files changed, 86 insertions(+), 43 deletions(-) diff --git a/cogl/cogl-framebuffer-private.h b/cogl/cogl-framebuffer-private.h index 8ab782e8d..7e1f97d7a 100644 --- a/cogl/cogl-framebuffer-private.h +++ b/cogl/cogl-framebuffer-private.h @@ -364,4 +364,7 @@ _cogl_framebuffer_save_clip_stack (CoglFramebuffer *framebuffer); void _cogl_framebuffer_restore_clip_stack (CoglFramebuffer *framebuffer); +void +_cogl_framebuffer_unref (CoglFramebuffer *framebuffer); + #endif /* __COGL_FRAMEBUFFER_PRIVATE_H */ diff --git a/cogl/cogl-framebuffer.c b/cogl/cogl-framebuffer.c index 2e63681a7..c82ad4f15 100644 --- a/cogl/cogl-framebuffer.c +++ b/cogl/cogl-framebuffer.c @@ -113,7 +113,9 @@ extern CoglObjectClass _cogl_onscreen_class; static void _cogl_offscreen_free (CoglOffscreen *offscreen); -COGL_OBJECT_DEFINE (Offscreen, offscreen); +COGL_OBJECT_DEFINE_WITH_CODE (Offscreen, offscreen, + _cogl_offscreen_class.virt_unref = + _cogl_framebuffer_unref); COGL_OBJECT_DEFINE_DEPRECATED_REF_COUNTING (offscreen); /* XXX: @@ -1167,19 +1169,7 @@ _cogl_set_framebuffers (CoglFramebuffer *draw_buffer, if (current_draw_buffer != draw_buffer || current_read_buffer != read_buffer) - { - /* XXX: eventually we want to remove this implicit journal flush - * so we can log into the journal beyond framebuffer changes to - * support batching scenes that depend on the results of - * mid-scene renders to textures. Current will be NULL when the - * framebuffer stack is first created so we need to guard - * against that here */ - if (current_draw_buffer) - _cogl_framebuffer_flush_journal (current_draw_buffer); - if (current_read_buffer) - _cogl_framebuffer_flush_journal (current_read_buffer); - _cogl_set_framebuffers_real (draw_buffer, read_buffer); - } + _cogl_set_framebuffers_real (draw_buffer, read_buffer); } void @@ -1292,19 +1282,10 @@ cogl_pop_framebuffer (void) if (to_pop->draw_buffer != to_restore->draw_buffer || to_pop->read_buffer != to_restore->read_buffer) - { - /* XXX: eventually we want to remove this implicit journal flush - * so we can log into the journal beyond framebuffer changes to - * support batching scenes that depend on the results of - * mid-scene renders to textures. */ - _cogl_framebuffer_flush_journal (to_pop->draw_buffer); - _cogl_framebuffer_flush_journal (to_pop->read_buffer); - - notify_buffers_changed (to_pop->draw_buffer, - to_restore->draw_buffer, - to_pop->read_buffer, - to_restore->read_buffer); - } + notify_buffers_changed (to_pop->draw_buffer, + to_restore->draw_buffer, + to_pop->read_buffer, + to_restore->read_buffer); cogl_object_unref (to_pop->draw_buffer); cogl_object_unref (to_pop->read_buffer); @@ -2407,3 +2388,31 @@ _cogl_framebuffer_restore_clip_stack (CoglFramebuffer *framebuffer) framebuffer->context->current_draw_buffer_changes |= COGL_FRAMEBUFFER_STATE_CLIP; } + +void +_cogl_framebuffer_unref (CoglFramebuffer *framebuffer) +{ + /* The journal holds a reference to the framebuffer whenever it is + non-empty. Therefore if the journal is non-empty and we will have + exactly one reference then we know the journal is the only thing + keeping the framebuffer alive. In that case we want to flush the + journal and let the framebuffer die. It is fine at this point if + flushing the journal causes something else to take a reference to + it and it comes back to life */ + if (framebuffer->journal->entries->len > 0) + { + unsigned int ref_count = ((CoglObject *) framebuffer)->ref_count; + + /* There should be at least two references - the one we are + about to drop and the one held by the journal */ + if (ref_count < 2) + g_warning ("Inconsistent ref count on a framebuffer with journal " + "entries."); + + if (ref_count == 2) + _cogl_framebuffer_flush_journal (framebuffer); + } + + /* Chain-up */ + _cogl_object_default_unref (framebuffer); +} diff --git a/cogl/cogl-journal-private.h b/cogl/cogl-journal-private.h index 9befcd4a5..d81ff75a0 100644 --- a/cogl/cogl-journal-private.h +++ b/cogl/cogl-journal-private.h @@ -34,6 +34,14 @@ typedef struct _CoglJournal { CoglObject _parent; + /* A pointer the framebuffer that is using this journal. This is + only valid when the journal is not empty. It *does* take a + reference on the framebuffer. Although this creates a circular + reference, the framebuffer has special code to handle the case + where the journal is the only thing holding a reference and it + will cause the journal to flush */ + CoglFramebuffer *framebuffer; + GArray *entries; GArray *vertices; size_t needed_vbo_len; diff --git a/cogl/cogl-journal.c b/cogl/cogl-journal.c index 63cf90ffa..5a67aa1de 100644 --- a/cogl/cogl-journal.c +++ b/cogl/cogl-journal.c @@ -1261,6 +1261,14 @@ _cogl_journal_discard (CoglJournal *journal) g_array_set_size (journal->vertices, 0); journal->needed_vbo_len = 0; journal->fast_read_pixel_count = 0; + + /* The journal only holds a reference to the framebuffer while the + journal is not empty */ + if (journal->framebuffer) + { + cogl_object_unref (journal->framebuffer); + journal->framebuffer = NULL; + } } /* Note: A return value of FALSE doesn't mean 'no' it means @@ -1353,6 +1361,11 @@ _cogl_journal_flush (CoglJournal *journal, if (journal->entries->len == 0) return; + /* Something has gone wrong if we're using a different framebuffer + to flush than the one that was current when the entries were + added */ + _COGL_RETURN_IF_FAIL (framebuffer == journal->framebuffer); + /* The entries in this journal may depend on images in other * framebuffers which may require that we flush the journals * associated with those framebuffers before we can flush @@ -1496,6 +1509,14 @@ _cogl_journal_log_quad (CoglJournal *journal, COGL_TIMER_START (_cogl_uprof_context, log_timer); + /* If the framebuffer was previously empty then we'll take a + reference to the current framebuffer. This reference will be + removed when the journal is flushed. FIXME: This should probably + be being passed a pointer to the framebuffer from further up so + that we don't have to rely on the global framebuffer stack */ + if (journal->vertices->len == 0) + journal->framebuffer = cogl_object_ref (cogl_get_draw_framebuffer ()); + /* The vertex data is logged into a separate array. The data needs to be copied into a vertex array before it's given to GL so we only store two vertices per quad and expand it to four while @@ -1573,7 +1594,7 @@ _cogl_journal_log_quad (CoglJournal *journal, entry->pipeline = _cogl_pipeline_journal_ref (source); - clip_stack = _cogl_framebuffer_get_clip_stack (cogl_get_draw_framebuffer ()); + clip_stack = _cogl_framebuffer_get_clip_stack (journal->framebuffer); entry->clip_stack = _cogl_clip_stack_ref (clip_stack); if (G_UNLIKELY (source != pipeline)) @@ -1583,7 +1604,7 @@ _cogl_journal_log_quad (CoglJournal *journal, _cogl_pipeline_foreach_layer_internal (pipeline, add_framebuffer_deps_cb, - cogl_get_draw_framebuffer ()); + journal->framebuffer); /* XXX: It doesn't feel very nice that in this case we just assume * that the journal is associated with the current framebuffer. I @@ -1591,13 +1612,14 @@ _cogl_journal_log_quad (CoglJournal *journal, * the reason we don't have that currently is that it would * introduce a circular reference. */ if (G_UNLIKELY (COGL_DEBUG_ENABLED (COGL_DEBUG_DISABLE_BATCHING))) - _cogl_framebuffer_flush_journal (cogl_get_draw_framebuffer ()); + _cogl_framebuffer_flush_journal (journal->framebuffer); COGL_TIMER_STOP (_cogl_uprof_context, log_timer); } static void -entry_to_screen_polygon (const CoglJournalEntry *entry, +entry_to_screen_polygon (CoglFramebuffer *framebuffer, + const CoglJournalEntry *entry, float *vertices, float *poly) { @@ -1642,7 +1664,7 @@ entry_to_screen_polygon (const CoglJournalEntry *entry, 4 /* n_points */); projection_stack = - _cogl_framebuffer_get_projection_stack (cogl_get_draw_framebuffer ()); + _cogl_framebuffer_get_projection_stack (framebuffer); _cogl_matrix_stack_get (projection_stack, &projection); cogl_matrix_project_points (&projection, @@ -1654,7 +1676,7 @@ entry_to_screen_polygon (const CoglJournalEntry *entry, poly, /* points_out */ 4 /* n_points */); - cogl_framebuffer_get_viewport4fv (cogl_get_draw_framebuffer (), viewport); + cogl_framebuffer_get_viewport4fv (framebuffer, viewport); /* Scale from OpenGL normalized device coordinates (ranging from -1 to 1) * to Cogl window/framebuffer coordinates (ranging from 0 to buffer-size) with @@ -1688,7 +1710,8 @@ entry_to_screen_polygon (const CoglJournalEntry *entry, } static gboolean -try_checking_point_hits_entry_after_clipping (CoglJournalEntry *entry, +try_checking_point_hits_entry_after_clipping (CoglFramebuffer *framebuffer, + CoglJournalEntry *entry, float *vertices, float x, float y, @@ -1750,7 +1773,7 @@ try_checking_point_hits_entry_after_clipping (CoglJournalEntry *entry, return FALSE; software_clip_entry (entry, vertices, &clip_bounds); - entry_to_screen_polygon (entry, vertices, poly); + entry_to_screen_polygon (framebuffer, entry, vertices, poly); *hit = _cogl_util_point_in_screen_poly (x, y, poly, sizeof (float) * 4, 4); return TRUE; @@ -1803,22 +1826,20 @@ _cogl_journal_try_read_pixel (CoglJournal *journal, entry->array_offset); float *vertices = (float *)color + 1; float poly[16]; + CoglFramebuffer *framebuffer = journal->framebuffer; - entry_to_screen_polygon (entry, vertices, poly); + entry_to_screen_polygon (framebuffer, entry, vertices, poly); if (!_cogl_util_point_in_screen_poly (x, y, poly, sizeof (float) * 4, 4)) continue; - /* FIXME: the journal should have a back pointer to the - * associated framebuffer, because it should be possible to read - * a pixel from arbitrary framebuffers without needing to - * internally call _cogl_push/pop_framebuffer. - */ if (entry->clip_stack) { gboolean hit; - if (!try_checking_point_hits_entry_after_clipping (entry, vertices, + if (!try_checking_point_hits_entry_after_clipping (framebuffer, + entry, + vertices, x, y, &hit)) return FALSE; /* hit couldn't be determined */ diff --git a/cogl/cogl-onscreen.c b/cogl/cogl-onscreen.c index 686b9e973..11b14927d 100644 --- a/cogl/cogl-onscreen.c +++ b/cogl/cogl-onscreen.c @@ -34,7 +34,9 @@ static void _cogl_onscreen_free (CoglOnscreen *onscreen); -COGL_OBJECT_INTERNAL_DEFINE (Onscreen, onscreen); +COGL_OBJECT_INTERNAL_DEFINE_WITH_CODE (Onscreen, onscreen, + _cogl_onscreen_class.virt_unref = + _cogl_framebuffer_unref); static void _cogl_onscreen_init_from_template (CoglOnscreen *onscreen,