Move tracking of renderables from FlRenderer to FlEngine (#165848)

Motivated by work on refactoring FlRenderer to FlCompositor.

Author: robert-ancell

engine/src/flutter/shell/platform/linux/fl_engine.cc

@@ -8,6 +8,7 @@
 
 #include <cstring>
 
+#include "flutter/common/constants.h"
 #include "flutter/shell/platform/common/engine_switches.h"
 #include "flutter/shell/platform/embedder/embedder.h"
 #include "flutter/shell/platform/linux/fl_binary_messenger_private.h"
@@ -93,6 +94,9 @@ struct _FlEngine {
   // Next ID to use for a view.
   FlutterViewId next_view_id;
 
+  // Objects rendering the views.
+  GHashTable* renderables_by_view_id;
+
   // Function to call when a platform message is received.
   FlEnginePlatformMessageHandler platform_message_handler;
   gpointer platform_message_handler_data;
@@ -489,6 +493,7 @@ static void fl_engine_dispose(GObject* object) {
   g_clear_object(&self->keyboard_handler);
   g_clear_object(&self->mouse_cursor_handler);
   g_clear_object(&self->task_runner);
+  g_clear_pointer(&self->renderables_by_view_id, g_hash_table_unref);
 
   if (self->platform_message_handler_destroy_notify) {
     self->platform_message_handler_destroy_notify(
@@ -534,6 +539,12 @@ static void fl_engine_init(FlEngine* self) {
 
   // Implicit view is 0, so start at 1.
   self->next_view_id = 1;
+  self->renderables_by_view_id = g_hash_table_new_full(
+      g_direct_hash, g_direct_equal, nullptr, [](gpointer value) {
+        GWeakRef* ref = static_cast<GWeakRef*>(value);
+        g_weak_ref_clear(ref);
+        free(ref);
+      });
 
   self->texture_registrar = fl_texture_registrar_new(self);
 }
@@ -734,7 +745,15 @@ void fl_engine_notify_display_update(FlEngine* self,
   }
 }
 
+void fl_engine_set_implicit_view(FlEngine* self, FlRenderable* renderable) {
+  GWeakRef* ref = g_new(GWeakRef, 1);
+  g_weak_ref_init(ref, G_OBJECT(renderable));
+  g_hash_table_insert(self->renderables_by_view_id,
+                      GINT_TO_POINTER(flutter::kFlutterImplicitViewId), ref);
+}
+
 FlutterViewId fl_engine_add_view(FlEngine* self,
+                                 FlRenderable* renderable,
                                  size_t width,
                                  size_t height,
                                  double pixel_ratio,
@@ -748,6 +767,11 @@ FlutterViewId fl_engine_add_view(FlEngine* self,
   FlutterViewId view_id = self->next_view_id;
   self->next_view_id++;
 
+  GWeakRef* ref = g_new(GWeakRef, 1);
+  g_weak_ref_init(ref, G_OBJECT(renderable));
+  g_hash_table_insert(self->renderables_by_view_id, GINT_TO_POINTER(view_id),
+                      ref);
+
   // We don't know which display this view will open on, so set to zero and this
   // will be updated in a following FlutterWindowMetricsEvent
   FlutterEngineDisplayId display_id = 0;
@@ -784,13 +808,23 @@ gboolean fl_engine_add_view_finish(FlEngine* self,
   return g_task_propagate_boolean(G_TASK(result), error);
 }
 
+FlRenderable* fl_engine_get_renderable(FlEngine* self, FlutterViewId view_id) {
+  g_return_val_if_fail(FL_IS_ENGINE(self), nullptr);
+
+  GWeakRef* ref = static_cast<GWeakRef*>(g_hash_table_lookup(
+      self->renderables_by_view_id, GINT_TO_POINTER(view_id)));
+  return FL_RENDERABLE(g_weak_ref_get(ref));
+}
+
 void fl_engine_remove_view(FlEngine* self,
                            FlutterViewId view_id,
                            GCancellable* cancellable,
                            GAsyncReadyCallback callback,
                            gpointer user_data) {
   g_return_if_fail(FL_IS_ENGINE(self));
 
+  g_hash_table_remove(self->renderables_by_view_id, GINT_TO_POINTER(view_id));
+
   g_autoptr(GTask) task = g_task_new(self, cancellable, callback, user_data);
 
   FlutterRemoveViewInfo info;

engine/src/flutter/shell/platform/linux/fl_engine_private.h

@@ -11,6 +11,7 @@
 #include "flutter/shell/platform/linux/fl_display_monitor.h"
 #include "flutter/shell/platform/linux/fl_keyboard_manager.h"
 #include "flutter/shell/platform/linux/fl_mouse_cursor_handler.h"
+#include "flutter/shell/platform/linux/fl_renderable.h"
 #include "flutter/shell/platform/linux/fl_renderer.h"
 #include "flutter/shell/platform/linux/fl_task_runner.h"
 #include "flutter/shell/platform/linux/fl_text_input_handler.h"
@@ -127,9 +128,19 @@ void fl_engine_notify_display_update(FlEngine* engine,
                                      const FlutterEngineDisplay* displays,
                                      size_t displays_length);
 
+/**
+ * fl_engine_set_implicit_view:
+ * @engine: an #FlEngine.
+ * @renderable: the object that will render the implicit view.
+ *
+ * Sets the object to render the implicit view.
+ */
+void fl_engine_set_implicit_view(FlEngine* engine, FlRenderable* renderable);
+
 /**
  * fl_engine_add_view:
  * @engine: an #FlEngine.
+ * @renderable: the object that will render this view.
  * @width: width of view in pixels.
  * @height: height of view in pixels.
  * @pixel_ratio: scale factor for view.
@@ -144,6 +155,7 @@ void fl_engine_notify_display_update(FlEngine* engine,
  * Returns: the ID for the view.
  */
 FlutterViewId fl_engine_add_view(FlEngine* engine,
+                                 FlRenderable* renderable,
                                  size_t width,
                                  size_t height,
                                  double pixel_ratio,
@@ -166,6 +178,18 @@ gboolean fl_engine_add_view_finish(FlEngine* engine,
                                    GAsyncResult* result,
                                    GError** error);
 
+/**
+ * fl_engine_get_renderable:
+ * @engine: an #FlEngine.
+ * @view_id: ID to check.
+ *
+ * Gets the renderable associated with the give view ID.
+ *
+ * Returns: (transfer full): a reference to an #FlRenderable or %NULL if none
+ * for this ID.
+ */
+FlRenderable* fl_engine_get_renderable(FlEngine* engine, FlutterViewId view_id);
+
 /**
  * fl_engine_remove_view:
  * @engine: an #FlEngine.

engine/src/flutter/shell/platform/linux/fl_engine_test.cc

@@ -10,6 +10,7 @@
 #include "flutter/shell/platform/linux/public/flutter_linux/fl_engine.h"
 #include "flutter/shell/platform/linux/public/flutter_linux/fl_json_message_codec.h"
 #include "flutter/shell/platform/linux/public/flutter_linux/fl_string_codec.h"
+#include "flutter/shell/platform/linux/testing/mock_renderer.h"
 
 // MOCK_ENGINE_PROC is leaky by design
 // NOLINTBEGIN(clang-analyzer-core.StackAddressEscape)
@@ -651,8 +652,10 @@ TEST(FlEngineTest, AddView) {
         return kSuccess;
       }));
 
+  g_autoptr(FlMockRenderable) renderable = fl_mock_renderable_new();
   FlutterViewId view_id =
-      fl_engine_add_view(engine, 123, 456, 2.0, nullptr, add_view_cb, loop);
+      fl_engine_add_view(engine, FL_RENDERABLE(renderable), 123, 456, 2.0,
+                         nullptr, add_view_cb, loop);
   EXPECT_GT(view_id, 0);
   EXPECT_TRUE(called);
 
@@ -688,8 +691,10 @@ TEST(FlEngineTest, AddViewError) {
         return kSuccess;
       }));
 
-  FlutterViewId view_id = fl_engine_add_view(engine, 123, 456, 2.0, nullptr,
-                                             add_view_error_cb, loop);
+  g_autoptr(FlMockRenderable) renderable = fl_mock_renderable_new();
+  FlutterViewId view_id =
+      fl_engine_add_view(engine, FL_RENDERABLE(renderable), 123, 456, 2.0,
+                         nullptr, add_view_error_cb, loop);
   EXPECT_GT(view_id, 0);
 
   // Blocks here until add_view_error_cb is called.
@@ -718,8 +723,10 @@ TEST(FlEngineTest, AddViewEngineError) {
         return kInvalidArguments;
       }));
 
-  FlutterViewId view_id = fl_engine_add_view(engine, 123, 456, 2.0, nullptr,
-                                             add_view_engine_error_cb, loop);
+  g_autoptr(FlMockRenderable) renderable = fl_mock_renderable_new();
+  FlutterViewId view_id =
+      fl_engine_add_view(engine, FL_RENDERABLE(renderable), 123, 456, 2.0,
+                         nullptr, add_view_engine_error_cb, loop);
   EXPECT_GT(view_id, 0);
 
   // Blocks here until remove_view_engine_error_cb is called.

engine/src/flutter/shell/platform/linux/fl_renderer.cc

@@ -51,9 +51,6 @@ typedef struct {
   // The format used to create textures.
   GLint general_format;
 
-  // Views being rendered.
-  GHashTable* views;
-
   // target dimension for resizing
   int target_width;
   int target_height;
@@ -85,12 +82,6 @@ typedef struct {
 
 G_DEFINE_TYPE_WITH_PRIVATE(FlRenderer, fl_renderer, G_TYPE_OBJECT)
 
-static void free_weak_ref(gpointer value) {
-  GWeakRef* ref = static_cast<GWeakRef*>(value);
-  g_weak_ref_clear(ref);
-  free(ref);
-}
-
 // Check if running on an NVIDIA driver.
 static gboolean is_nvidia() {
   const gchar* vendor = reinterpret_cast<const gchar*>(glGetString(GL_VENDOR));
@@ -349,10 +340,12 @@ static gboolean present_layers(FlRenderer* self,
     }
   }
 
-  GWeakRef* ref = static_cast<GWeakRef*>(
-      g_hash_table_lookup(priv->views, GINT_TO_POINTER(view_id)));
+  g_autoptr(FlEngine) engine = FL_ENGINE(g_weak_ref_get(&priv->engine));
+  if (engine == nullptr) {
+    return TRUE;
+  }
   g_autoptr(FlRenderable) renderable =
-      ref != nullptr ? FL_RENDERABLE(g_weak_ref_get(ref)) : nullptr;
+      fl_engine_get_renderable(engine, view_id);
   if (renderable == nullptr) {
     return TRUE;
   }
@@ -462,7 +455,6 @@ static void fl_renderer_dispose(GObject* object) {
   fl_renderer_unblock_main_thread(self);
 
   g_weak_ref_clear(&priv->engine);
-  g_clear_pointer(&priv->views, g_hash_table_unref);
   g_clear_pointer(&priv->framebuffers_by_view_id, g_hash_table_unref);
   g_mutex_clear(&priv->present_mutex);
   g_cond_clear(&priv->present_condition);
@@ -477,8 +469,6 @@ static void fl_renderer_class_init(FlRendererClass* klass) {
 static void fl_renderer_init(FlRenderer* self) {
   FlRendererPrivate* priv = reinterpret_cast<FlRendererPrivate*>(
       fl_renderer_get_instance_private(self));
-  priv->views = g_hash_table_new_full(g_direct_hash, g_direct_equal, nullptr,
-                                      free_weak_ref);
   priv->framebuffers_by_view_id = g_hash_table_new_full(
       g_direct_hash, g_direct_equal, nullptr,
       reinterpret_cast<GDestroyNotify>(g_ptr_array_unref));
@@ -495,28 +485,6 @@ void fl_renderer_set_engine(FlRenderer* self, FlEngine* engine) {
   g_weak_ref_init(&priv->engine, engine);
 }
 
-void fl_renderer_add_renderable(FlRenderer* self,
-                                FlutterViewId view_id,
-                                FlRenderable* renderable) {
-  FlRendererPrivate* priv = reinterpret_cast<FlRendererPrivate*>(
-      fl_renderer_get_instance_private(self));
-
-  g_return_if_fail(FL_IS_RENDERER(self));
-
-  GWeakRef* ref = g_new(GWeakRef, 1);
-  g_weak_ref_init(ref, G_OBJECT(renderable));
-  g_hash_table_insert(priv->views, GINT_TO_POINTER(view_id), ref);
-}
-
-void fl_renderer_remove_view(FlRenderer* self, FlutterViewId view_id) {
-  FlRendererPrivate* priv = reinterpret_cast<FlRendererPrivate*>(
-      fl_renderer_get_instance_private(self));
-
-  g_return_if_fail(FL_IS_RENDERER(self));
-
-  g_hash_table_remove(priv->views, GINT_TO_POINTER(view_id));
-}
-
 void* fl_renderer_get_proc_address(FlRenderer* self, const char* name) {
   g_return_val_if_fail(FL_IS_RENDERER(self), NULL);
 

engine/src/flutter/shell/platform/linux/fl_renderer.h

@@ -66,27 +66,6 @@ struct _FlRendererClass {
  */
 void fl_renderer_set_engine(FlRenderer* renderer, FlEngine* engine);
 
-/**
- * fl_renderer_add_renderable:
- * @renderer: an #FlRenderer.
- * @view_id: the ID of the view.
- * @renderable: object that is to be rendered on.
- *
- * Add a view to render on.
- */
-void fl_renderer_add_renderable(FlRenderer* renderer,
-                                FlutterViewId view_id,
-                                FlRenderable* renderable);
-
-/**
- * fl_renderer_remove_view:
- * @renderer: an #FlRenderer.
- * @view_id: the ID of the view.
- *
- * Remove a view from the renderer.
- */
-void fl_renderer_remove_view(FlRenderer* renderer, FlutterViewId view_id);
-
 /**
  * fl_renderer_get_proc_address:
  * @renderer: an #FlRenderer.