fuchsia: Change flatland present's release fence logic (flutter/engine#28850)

* fuchsia: Change flatland present's release fence logic

(cherry picked from commit a9a370252276df6c379c25bc809467d64b11beac)
(cherry picked from commit 74c559d424d35e45e619a7da1d87e3d3dec57ec1)

* fuchsia: Use runner_services to connect to Flatland

(cherry picked from commit 685493e7f01aaf9cc03f6e8ca66946203b20f880)

engine/src/flutter/shell/platform/fuchsia/flutter/BUILD.gn

@@ -472,6 +472,7 @@ executable("flutter_runner_unittests") {
     "tests/engine_unittests.cc",
     "tests/fake_flatland_unittests.cc",
     "tests/fake_session_unittests.cc",
+    "tests/flatland_connection_unittests.cc",
     "tests/flutter_runner_product_configuration_unittests.cc",
     "tests/gfx_external_view_embedder_unittests.cc",
     "tests/gfx_session_connection_unittests.cc",

engine/src/flutter/shell/platform/fuchsia/flutter/engine.cc

@@ -160,6 +160,16 @@ void Engine::Initialize(
   }
   scenic->CreateSessionT(std::move(gfx_protocols), [] {});
 
+  // Connect to Flatland.
+  fidl::InterfaceHandle<fuchsia::ui::composition::Flatland> flatland;
+  zx_status_t flatland_status =
+      runner_services->Connect<fuchsia::ui::composition::Flatland>(
+          flatland.NewRequest());
+  if (flatland_status != ZX_OK && use_flatland) {
+    FML_LOG(WARNING) << "fuchsia::ui::composition::Flatland connection failed: "
+                     << zx_status_get_string(flatland_status);
+  }
+
   // Connect to SemanticsManager service.
   fidl::InterfaceHandle<fuchsia::accessibility::semantics::SemanticsManager>
       semantics_manager;
@@ -226,7 +236,7 @@ void Engine::Initialize(
   task_runners.GetRasterTaskRunner()->PostTask(fml::MakeCopyable(
       [this, &view_embedder_latch,
        session_inspect_node = std::move(session_inspect_node),
-       session = std::move(session),
+       session = std::move(session), flatland = std::move(flatland),
        session_error_callback = std::move(session_error_callback), use_flatland,
        view_token = std::move(view_token_),
        view_creation_token = std::move(view_creation_token_),
@@ -237,7 +247,8 @@ void Engine::Initialize(
        vsync_offset = product_config.get_vsync_offset()]() mutable {
         if (use_flatland) {
           flatland_connection_ = std::make_shared<FlatlandConnection>(
-              thread_label_, std::move(session_error_callback), [](auto) {},
+              thread_label_, std::move(flatland),
+              std::move(session_error_callback), [](auto) {},
               max_frames_in_flight, vsync_offset);
           surface_producer_.emplace(/*scenic_session=*/nullptr);
           fuchsia::ui::views::ViewIdentityOnCreation view_identity = {

engine/src/flutter/shell/platform/fuchsia/flutter/flatland_connection.cc

@@ -4,7 +4,7 @@
 
 #include "flatland_connection.h"
 
-#include <lib/fdio/directory.h>
+#include <zircon/status.h>
 
 #include "flutter/fml/logging.h"
 
@@ -12,17 +12,18 @@ namespace flutter_runner {
 
 FlatlandConnection::FlatlandConnection(
     std::string debug_label,
+    fidl::InterfaceHandle<fuchsia::ui::composition::Flatland> flatland,
     fml::closure error_callback,
     on_frame_presented_event on_frame_presented_callback,
     uint64_t max_frames_in_flight,
     fml::TimeDelta vsync_offset)
-    : error_callback_(error_callback),
+    : flatland_(flatland.Bind()),
+      error_callback_(error_callback),
       on_frame_presented_callback_(std::move(on_frame_presented_callback)) {
-  zx_status_t status =
-      fdio_service_connect("/svc/fuchsia.ui.composition.Flatland",
-                           flatland_.NewRequest().TakeChannel().release());
-  FML_DCHECK(status == ZX_OK);
-
+  flatland_.set_error_handler([callback = error_callback_](zx_status_t status) {
+    FML_LOG(ERROR) << "Flatland disconnected" << zx_status_get_string(status);
+    callback();
+  });
   flatland_->SetDebugName(debug_label);
   flatland_.events().OnError =
       fit::bind_member(this, &FlatlandConnection::OnError);
@@ -44,18 +45,22 @@ void FlatlandConnection::Present() {
   fuchsia::ui::composition::PresentArgs present_args;
   present_args.set_requested_presentation_time(0);
   present_args.set_acquire_fences(std::move(acquire_fences_));
-  present_args.set_release_fences(std::move(release_fences_));
+  present_args.set_release_fences(std::move(previous_present_release_fences_));
   present_args.set_unsquashable(false);
   flatland_->Present(std::move(present_args));
 
+  // In Flatland, release fences apply to the content of the previous present.
+  // Keeping track of the old frame's release fences and swapping ensure we set
+  // the correct ones for VulkanSurface's interpretation.
+  previous_present_release_fences_.clear();
+  previous_present_release_fences_.swap(current_present_release_fences_);
   acquire_fences_.clear();
-  release_fences_.clear();
 }
 
 void FlatlandConnection::AwaitVsync(FireCallbackCallback callback) {
   if (first_call) {
     fml::TimePoint now = fml::TimePoint::Now();
-    callback(now, now + fml::TimeDelta::FromMilliseconds(5));
+    callback(now, now + kDefaultFlatlandPresentationInterval);
     first_call = false;
     return;
   }
@@ -78,7 +83,7 @@ void FlatlandConnection::OnNextFrameBegin(
   if (fire_callback_) {
     fml::TimePoint now = fml::TimePoint::Now();
     // TODO(fxbug.dev/64201): Calculate correct frame times.
-    fire_callback_(now, now + fml::TimeDelta::FromMilliseconds(5));
+    fire_callback_(now, now + kDefaultFlatlandPresentationInterval);
   }
 }
 
@@ -92,7 +97,7 @@ void FlatlandConnection::EnqueueAcquireFence(zx::event fence) {
 }
 
 void FlatlandConnection::EnqueueReleaseFence(zx::event fence) {
-  release_fences_.push_back(std::move(fence));
+  current_present_release_fences_.push_back(std::move(fence));
 }
 
 }  // namespace flutter_runner

engine/src/flutter/shell/platform/fuchsia/flutter/flatland_connection.h

@@ -21,15 +21,21 @@ namespace flutter_runner {
 using on_frame_presented_event =
     std::function<void(fuchsia::scenic::scheduling::FramePresentedInfo)>;
 
+// Assume a 60hz refresh rate.
+static constexpr fml::TimeDelta kDefaultFlatlandPresentationInterval =
+    fml::TimeDelta::FromSecondsF(1.0 / 60.0);
+
 // The component residing on the raster thread that is responsible for
 // maintaining the Flatland instance connection and presenting updates.
 class FlatlandConnection final {
  public:
-  FlatlandConnection(std::string debug_label,
-                     fml::closure error_callback,
-                     on_frame_presented_event on_frame_presented_callback,
-                     uint64_t max_frames_in_flight,
-                     fml::TimeDelta vsync_offset);
+  FlatlandConnection(
+      std::string debug_label,
+      fidl::InterfaceHandle<fuchsia::ui::composition::Flatland> flatland,
+      fml::closure error_callback,
+      on_frame_presented_event on_frame_presented_callback,
+      uint64_t max_frames_in_flight,
+      fml::TimeDelta vsync_offset);
 
   ~FlatlandConnection();
 
@@ -73,7 +79,8 @@ class FlatlandConnection final {
   bool first_call = true;
 
   std::vector<zx::event> acquire_fences_;
-  std::vector<zx::event> release_fences_;
+  std::vector<zx::event> current_present_release_fences_;
+  std::vector<zx::event> previous_present_release_fences_;
 
   FML_DISALLOW_COPY_AND_ASSIGN(FlatlandConnection);
 };

engine/src/flutter/shell/platform/fuchsia/flutter/flatland_external_view_embedder.cc

@@ -246,19 +246,20 @@ void FlatlandExternalViewEmbedder::SubmitFrame(
         // Create a new layer if needed for the surface.
         FML_CHECK(flatland_layer_index <= flatland_layers_.size());
         if (flatland_layer_index == flatland_layers_.size()) {
-          FlatlandLayer new_layer{
-              .transform_id = flatland_.NextTransformId(),
-              .image_id = {surface_for_layer->GetImageId()}};
+          FlatlandLayer new_layer{.transform_id = flatland_.NextTransformId()};
           flatland_.flatland()->CreateTransform(new_layer.transform_id);
-          flatland_.flatland()->SetContent(new_layer.transform_id,
-                                           new_layer.image_id);
-          const auto& size = surface_for_layer->GetSize();
-          flatland_.flatland()->SetImageDestinationSize(
-              new_layer.image_id, {static_cast<uint32_t>(size.width()),
-                                   static_cast<uint32_t>(size.height())});
           flatland_layers_.emplace_back(std::move(new_layer));
         }
 
+        // Update the image content and set size.
+        flatland_.flatland()->SetContent(
+            flatland_layers_[flatland_layer_index].transform_id,
+            {surface_for_layer->GetImageId()});
+        flatland_.flatland()->SetImageDestinationSize(
+            {surface_for_layer->GetImageId()},
+            {static_cast<uint32_t>(surface_for_layer->GetSize().width()),
+             static_cast<uint32_t>(surface_for_layer->GetSize().height())});
+
         // Attach the FlatlandLayer to the main scene graph.
         flatland_.flatland()->AddChild(
             root_transform_id_,

engine/src/flutter/shell/platform/fuchsia/flutter/flatland_external_view_embedder.h

@@ -164,7 +164,6 @@ class FlatlandExternalViewEmbedder final
 
   struct FlatlandLayer {
     fuchsia::ui::composition::TransformId transform_id;
-    fuchsia::ui::composition::ContentId image_id;
   };
 
   FlatlandConnection& flatland_;

engine/src/flutter/shell/platform/fuchsia/flutter/tests/flatland_connection_unittests.cc

@@ -0,0 +1,188 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/shell/platform/fuchsia/flutter/flatland_connection.h"
+
+#include <fuchsia/scenic/scheduling/cpp/fidl.h>
+#include <fuchsia/ui/composition/cpp/fidl.h>
+#include <lib/async-testing/test_loop.h>
+
+#include <string>
+#include <vector>
+
+#include "flutter/fml/logging.h"
+#include "flutter/fml/time/time_delta.h"
+#include "flutter/fml/time/time_point.h"
+#include "gtest/gtest.h"
+
+#include "fakes/scenic/fake_flatland.h"
+
+namespace flutter_runner::testing {
+
+namespace {
+
+std::string GetCurrentTestName() {
+  return ::testing::UnitTest::GetInstance()->current_test_info()->name();
+}
+
+void AwaitVsyncChecked(FlatlandConnection& flatland_connection,
+                       bool& condition_variable,
+                       fml::TimeDelta expected_frame_delta) {
+  flatland_connection.AwaitVsync(
+      [&condition_variable,
+       expected_frame_delta = std::move(expected_frame_delta)](
+          fml::TimePoint frame_start, fml::TimePoint frame_end) {
+        EXPECT_EQ(frame_end.ToEpochDelta() - frame_start.ToEpochDelta(),
+                  expected_frame_delta);
+        condition_variable = true;
+      });
+}
+
+}  // namespace
+
+class FlatlandConnectionTest : public ::testing::Test {
+ protected:
+  FlatlandConnectionTest()
+      : session_subloop_(loop_.StartNewLoop()),
+        flatland_handle_(
+            fake_flatland_.Connect(session_subloop_->dispatcher())) {}
+  ~FlatlandConnectionTest() override = default;
+
+  async::TestLoop& loop() { return loop_; }
+
+  FakeFlatland& fake_flatland() { return fake_flatland_; }
+
+  fidl::InterfaceHandle<fuchsia::ui::composition::Flatland>
+  TakeFlatlandHandle() {
+    FML_CHECK(flatland_handle_.is_valid());
+    return std::move(flatland_handle_);
+  }
+
+ private:
+  async::TestLoop loop_;
+  std::unique_ptr<async::LoopInterface> session_subloop_;
+
+  FakeFlatland fake_flatland_;
+
+  fidl::InterfaceHandle<fuchsia::ui::composition::Flatland> flatland_handle_;
+};
+
+TEST_F(FlatlandConnectionTest, Initialization) {
+  // Create the FlatlandConnection but don't pump the loop.  No FIDL calls are
+  // completed yet.
+  const std::string debug_name = GetCurrentTestName();
+  flutter_runner::FlatlandConnection flatland_connection(
+      debug_name, TakeFlatlandHandle(), []() { FAIL(); },
+      [](auto...) { FAIL(); }, 1, fml::TimeDelta::Zero());
+  EXPECT_EQ(fake_flatland().debug_name(), "");
+
+  // Simulate an AwaitVsync that comes immediately.
+  bool await_vsync_fired = false;
+  AwaitVsyncChecked(flatland_connection, await_vsync_fired,
+                    kDefaultFlatlandPresentationInterval);
+  EXPECT_TRUE(await_vsync_fired);
+
+  // Ensure the debug name is set.
+  loop().RunUntilIdle();
+  EXPECT_EQ(fake_flatland().debug_name(), debug_name);
+}
+
+TEST_F(FlatlandConnectionTest, FlatlandDisconnect) {
+  // Set up a callback which allows sensing of the error state.
+  bool error_fired = false;
+  fml::closure on_session_error = [&error_fired]() { error_fired = true; };
+
+  // Create the FlatlandConnection but don't pump the loop.  No FIDL calls are
+  // completed yet.
+  flutter_runner::FlatlandConnection flatland_connection(
+      GetCurrentTestName(), TakeFlatlandHandle(), std::move(on_session_error),
+      [](auto...) { FAIL(); }, 1, fml::TimeDelta::Zero());
+  EXPECT_FALSE(error_fired);
+
+  // Simulate a flatland disconnection, then Pump the loop.  The error callback
+  // will fire.
+  fake_flatland().Disconnect(
+      fuchsia::ui::composition::FlatlandError::BAD_OPERATION);
+  loop().RunUntilIdle();
+  EXPECT_TRUE(error_fired);
+}
+
+TEST_F(FlatlandConnectionTest, BasicPresent) {
+  // Set up callbacks which allow sensing of how many presents were handled.
+  size_t presents_called = 0u;
+  zx_handle_t release_fence_handle;
+  fake_flatland().SetPresentHandler([&presents_called,
+                                     &release_fence_handle](auto present_args) {
+    presents_called++;
+    release_fence_handle = present_args.release_fences().empty()
+                               ? ZX_HANDLE_INVALID
+                               : present_args.release_fences().front().get();
+  });
+
+  // Set up a callback which allows sensing of how many vsync's
+  // (`OnFramePresented` events) were handled.
+  size_t vsyncs_handled = 0u;
+  on_frame_presented_event on_frame_presented = [&vsyncs_handled](auto...) {
+    vsyncs_handled++;
+  };
+
+  // Create the FlatlandConnection but don't pump the loop.  No FIDL calls are
+  // completed yet.
+  flutter_runner::FlatlandConnection flatland_connection(
+      GetCurrentTestName(), TakeFlatlandHandle(), []() { FAIL(); },
+      std::move(on_frame_presented), 1, fml::TimeDelta::Zero());
+  EXPECT_EQ(presents_called, 0u);
+  EXPECT_EQ(vsyncs_handled, 0u);
+
+  // Pump the loop. Nothing is called.
+  loop().RunUntilIdle();
+  EXPECT_EQ(presents_called, 0u);
+  EXPECT_EQ(vsyncs_handled, 0u);
+
+  // Simulate an AwaitVsync that comes after the first call.
+  bool await_vsync_fired = false;
+  AwaitVsyncChecked(flatland_connection, await_vsync_fired,
+                    kDefaultFlatlandPresentationInterval);
+  EXPECT_TRUE(await_vsync_fired);
+
+  // Call Present and Pump the loop; `Present` and its callback is called. No
+  // release fence should be queued.
+  await_vsync_fired = false;
+  zx::event first_release_fence;
+  zx::event::create(0, &first_release_fence);
+  const zx_handle_t first_release_fence_handle = first_release_fence.get();
+  flatland_connection.EnqueueReleaseFence(std::move(first_release_fence));
+  flatland_connection.Present();
+  loop().RunUntilIdle();
+  EXPECT_EQ(presents_called, 1u);
+  EXPECT_EQ(release_fence_handle, ZX_HANDLE_INVALID);
+  EXPECT_EQ(vsyncs_handled, 0u);
+  EXPECT_FALSE(await_vsync_fired);
+
+  // Fire the `OnNextFrameBegin` event. AwaitVsync should be fired.
+  AwaitVsyncChecked(flatland_connection, await_vsync_fired,
+                    kDefaultFlatlandPresentationInterval);
+  fuchsia::ui::composition::OnNextFrameBeginValues on_next_frame_begin_values;
+  on_next_frame_begin_values.set_additional_present_credits(3);
+  fake_flatland().FireOnNextFrameBeginEvent(
+      std::move(on_next_frame_begin_values));
+  loop().RunUntilIdle();
+  EXPECT_TRUE(await_vsync_fired);
+
+  // Fire the `OnFramePresented` event associated with the first `Present`,
+  fake_flatland().FireOnFramePresentedEvent(
+      fuchsia::scenic::scheduling::FramePresentedInfo());
+  loop().RunUntilIdle();
+  EXPECT_EQ(vsyncs_handled, 1u);
+
+  // Call Present for a second time and Pump the loop; `Present` and its
+  // callback is called. Release fences for the earlier present is used.
+  await_vsync_fired = false;
+  flatland_connection.Present();
+  loop().RunUntilIdle();
+  EXPECT_EQ(presents_called, 2u);
+  EXPECT_EQ(release_fence_handle, first_release_fence_handle);
+}
+
+}  // namespace flutter_runner::testing