framebuffer: flush journal when switching framebuffers

There is currently a problem with per-framebuffer journals in that it's
possible to create a framebuffer from a texture which then gets rendered
too but the framebuffer (and corresponding journal) can be freed before
the texture gets used to draw with.

Conceptually we want to make sure when freeing a framebuffer that - if
it is associated with a texture - we flush the journal as the last thing
before really freeing the framebuffer's meta data. Technically though
this is awkward to implement since the obvious mechanism for us to be
notified about the framebuffer's destruction (by setting some user data
internally with a callback) notifies when the framebuffer has a
ref-count of 0. This means we'd have to be careful what we do with the
framebuffer to consider e.g. recursive destruction; anything that would
set more user data on the framebuffer while it is being destroyed and
ensuring nothing else gets notified of the framebuffer's destruction
before the journal has been flushed.

For simplicity, for now, this patch provides another solution which is
to flush framebuffer journals whenever we switch away from a given
framebuffer via cogl_set_framebuffer or cogl_push/pop_framebuffer. The
disadvantage of this approach is that we can't batch all the geometry of
a scene that involves intermediate renders to offscreen framebufers.
Clutter is doing this more and more with applications that use the
ClutterEffect APIs so this is a shame. Hopefully this will only be a
stop-gap solution while we consider how to reliably support journal
logging across framebuffer changes.

diff --git a/clutter/cogl/cogl/cogl-framebuffer.c b/clutter/cogl/cogl/cogl-framebuffer.c
index f14b80b..a788dd5 100644 (file)
--- a/clutter/cogl/cogl/cogl-framebuffer.c
+++ b/clutter/cogl/cogl/cogl-framebuffer.c
@@ -587,7 +587,8 @@ _cogl_framebuffer_remove_all_dependencies (CoglFramebuffer *framebuffer)
 void
 _cogl_framebuffer_flush_journal (CoglFramebuffer *framebuffer)
 {
-  _cogl_journal_flush (framebuffer->journal, framebuffer);
+  if (framebuffer)
+    _cogl_journal_flush (framebuffer->journal, framebuffer);
 }
 
 void
@@ -1045,10 +1046,20 @@ _cogl_set_framebuffer_real (CoglFramebuffer *framebuffer)
 void
 cogl_set_framebuffer (CoglFramebuffer *framebuffer)
 {
+  CoglFramebuffer *current;
+
   g_return_if_fail (_cogl_is_framebuffer (framebuffer));
 
-  if (_cogl_get_framebuffer () != framebuffer)
-    _cogl_set_framebuffer_real (framebuffer);
+  current = _cogl_get_framebuffer ();
+  if (current != framebuffer)
+    {
+      /* 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 (current);
+      _cogl_set_framebuffer_real (framebuffer);
+    }
 }
 
 /* XXX: deprecated API */
@@ -1108,7 +1119,14 @@ cogl_pop_framebuffer (void)
   to_pop = ctx->framebuffer_stack->data;
   to_restore = ctx->framebuffer_stack->next->data;
 
-  cogl_flush ();
+  if (to_pop != to_restore)
+    {
+      /* 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);
+    }
 
   cogl_object_unref (to_pop);
   ctx->framebuffer_stack =