Preserve order when regenerating GPU images (flutter#40268)

Preserve order when regenerating GPU images

common/graphics/texture.cc

@@ -14,7 +14,7 @@ Texture::Texture(int64_t id) : id_(id) {}
 
 Texture::~Texture() = default;
 
-TextureRegistry::TextureRegistry() = default;
+TextureRegistry::TextureRegistry() : image_counter_(0) {}
 
 void TextureRegistry::RegisterTexture(const std::shared_ptr<Texture>& texture) {
   if (!texture) {
@@ -26,7 +26,13 @@ void TextureRegistry::RegisterTexture(const std::shared_ptr<Texture>& texture) {
 void TextureRegistry::RegisterContextListener(
     uintptr_t id,
     std::weak_ptr<ContextListener> image) {
-  images_[id] = std::move(image);
+  size_t next_id = image_counter_++;
+  auto const result = image_indices_.insert({id, next_id});
+  if (!result.second) {
+    ordered_images_.erase(result.first->second);
+    result.first->second = next_id;
+  }
+  ordered_images_[next_id] = {next_id, std::move(image)};
 }
 
 void TextureRegistry::UnregisterTexture(int64_t id) {
@@ -39,19 +45,28 @@ void TextureRegistry::UnregisterTexture(int64_t id) {
 }
 
 void TextureRegistry::UnregisterContextListener(uintptr_t id) {
-  images_.erase(id);
+  ordered_images_.erase(image_indices_[id]);
+  image_indices_.erase(id);
 }
 
 void TextureRegistry::OnGrContextCreated() {
   for (auto& it : mapping_) {
     it.second->OnGrContextCreated();
   }
 
-  for (const auto& [id, weak_image] : images_) {
+  // Calling OnGrContextCreated may result in a subsequent call to
+  // RegisterContextListener from the listener, which may modify the map.
+  std::vector<InsertionOrderMap::value_type> ordered_images(
+      ordered_images_.begin(), ordered_images_.end());
+
+  for (const auto& [id, pair] : ordered_images) {
+    auto index_id = pair.first;
+    auto weak_image = pair.second;
     if (auto image = weak_image.lock()) {
       image->OnGrContextCreated();
     } else {
-      images_.erase(id);
+      image_indices_.erase(index_id);
+      ordered_images_.erase(id);
     }
   }
 }
@@ -61,11 +76,16 @@ void TextureRegistry::OnGrContextDestroyed() {
     it.second->OnGrContextDestroyed();
   }
 
-  for (const auto& [id, weak_image] : images_) {
+  auto it = ordered_images_.begin();
+  while (it != ordered_images_.end()) {
+    auto index_id = it->second.first;
+    auto weak_image = it->second.second;
     if (auto image = weak_image.lock()) {
       image->OnGrContextDestroyed();
+      it++;
     } else {
-      images_.erase(id);
+      image_indices_.erase(index_id);
+      it = ordered_images_.erase(it);
     }
   }
 }

common/graphics/texture.h

@@ -95,7 +95,16 @@ class TextureRegistry {
 
  private:
   std::map<int64_t, std::shared_ptr<Texture>> mapping_;
-  std::map<uintptr_t, std::weak_ptr<ContextListener>> images_;
+  size_t image_counter_;
+  // This map keeps track of registered context listeners by their own
+  // externally provided id. It indexes into ordered_images_.
+  std::map<uintptr_t, size_t> image_indices_;
+  // This map makes sure that iteration of images happens in insertion order
+  // (managed by image_counter_) so that images which depend on other images get
+  // re-created in the right order.
+  using InsertionOrderMap =
+      std::map<size_t, std::pair<uintptr_t, std::weak_ptr<ContextListener>>>;
+  InsertionOrderMap ordered_images_;
 
   FML_DISALLOW_COPY_AND_ASSIGN(TextureRegistry);
 };

flow/texture_unittests.cc

@@ -4,12 +4,36 @@
 
 #include "flutter/common/graphics/texture.h"
 
+#include <functional>
+
 #include "flutter/flow/testing/mock_texture.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace flutter {
 namespace testing {
 
+namespace {
+using int_closure = std::function<void(int)>;
+
+struct TestContextListener : public ContextListener {
+  TestContextListener(uintptr_t p_id,
+                      int_closure p_create,
+                      int_closure p_destroy)
+      : id(p_id), create(std::move(p_create)), destroy(std::move(p_destroy)) {}
+
+  virtual ~TestContextListener() = default;
+
+  const uintptr_t id;
+  int_closure create;
+  int_closure destroy;
+
+  void OnGrContextCreated() override { create(id); }
+
+  void OnGrContextDestroyed() override { destroy(id); }
+};
+}  // namespace
+
 TEST(TextureRegistryTest, UnregisterTextureCallbackTriggered) {
   TextureRegistry registry;
   auto mock_texture1 = std::make_shared<MockTexture>(0);
@@ -95,5 +119,24 @@ TEST(TextureRegistryTest, ReuseSameTextureSlot) {
   ASSERT_TRUE(mock_texture2->unregistered());
 }
 
+TEST(TextureRegistryTest, CallsOnGrContextCreatedInInsertionOrder) {
+  TextureRegistry registry;
+  std::vector<int> create_order;
+  std::vector<int> destroy_order;
+  auto create = [&](int id) { create_order.push_back(id); };
+  auto destroy = [&](int id) { destroy_order.push_back(id); };
+  auto a = std::make_shared<TestContextListener>(5, create, destroy);
+  auto b = std::make_shared<TestContextListener>(4, create, destroy);
+  auto c = std::make_shared<TestContextListener>(3, create, destroy);
+  registry.RegisterContextListener(a->id, a);
+  registry.RegisterContextListener(b->id, b);
+  registry.RegisterContextListener(c->id, c);
+  registry.OnGrContextDestroyed();
+  registry.OnGrContextCreated();
+
+  EXPECT_THAT(create_order, ::testing::ElementsAre(5, 4, 3));
+  EXPECT_THAT(destroy_order, ::testing::ElementsAre(5, 4, 3));
+}
+
 }  // namespace testing
 }  // namespace flutter

lib/ui/painting/display_list_deferred_image_gpu_skia.cc

@@ -137,7 +137,6 @@ void DlDeferredImageGPUSkia::ImageWrapper::OnGrContextCreated() {
 
 void DlDeferredImageGPUSkia::ImageWrapper::OnGrContextDestroyed() {
   FML_DCHECK(raster_task_runner_->RunsTasksOnCurrentThread());
-
   DeleteTexture();
 }