Test the surface size wait and its 60-frame give-up

.github/workflows/android.yml

@@ -74,6 +74,14 @@ jobs:
             packages/splatkit-android/tests/rendering/vulkan/presentation/WorldFrameCompletionTest.cpp \
             -o build/native-contracts/world-frame-completion
           build/native-contracts/world-frame-completion
+      - name: Check surface size wait on host
+        run: |
+          mkdir -p build/native-contracts
+          c++ -std=c++17 -Wall -Wextra -Wpedantic -Werror \
+            -I packages/splatkit-android/src/main/cpp \
+            packages/splatkit-android/tests/rendering/vulkan/presentation/SurfaceSizeWaitTest.cpp \
+            -o build/native-contracts/surface-size-wait
+          build/native-contracts/surface-size-wait
       - name: Keep the AAR
         uses: actions/upload-artifact@v7
         with:

packages/splatkit-android/src/main/cpp/rendering/vulkan/presentation/SurfaceSizeWait.h

@@ -0,0 +1,56 @@
+#pragma once
+
+#include <cstdint>
+#include <optional>
+
+namespace splatkit {
+
+// Decides, frame by frame, when a swapchain may be rebuilt for a new window size.
+// No Vulkan or Android types, so a host build tests it.
+class SurfaceSizeWait {
+ public:
+  struct Size {
+    uint32_t width = 0;
+    uint32_t height = 0;
+    bool operator==(const Size& other) const {
+      return width == other.width && height == other.height;
+    }
+    bool operator!=(const Size& other) const { return !(*this == other); }
+  };
+
+  enum class Step { wait, rebuild, giveUp };
+
+  // Frames to wait for the surface to report the window's size. A surface that never does
+  // (a fixed-size buffer) is built at the size it reports instead of redrawing forever.
+  static constexpr uint32_t kMaxStaleChecks = 60;
+
+  // The window now has this size and the swapchain must reach it; starts a new wait.
+  void windowResized(Size window) {
+    window_ = window;
+    staleChecks_ = 0;
+  }
+  Size window() const { return window_; }
+  // True while the swapchain is not the window's size.
+  bool pending(Size swapchain) const { return swapchain != window_; }
+
+  // One frame's decision. `reported` is the size the surface reports, empty when the query
+  // failed, which counts as stale. `rebuild` and `giveUp` both restart the count; after a
+  // `giveUp` rebuild the caller passes the swapchain it got to `adopt`.
+  Step check(std::optional<Size> reported) {
+    if (reported && *reported == window_) {
+      staleChecks_ = 0;
+      return Step::rebuild;
+    }
+    if (++staleChecks_ <= kMaxStaleChecks) return Step::wait;
+    staleChecks_ = 0;
+    return Step::giveUp;
+  }
+  // The swapchain size the surface settled on, taken as the window's size from here on.
+  void adopt(Size swapchain) { window_ = swapchain; }
+
+ private:
+  Size window_;
+  uint32_t staleChecks_ = 0;
+};
+
+}  // namespace splatkit

packages/splatkit-android/src/main/cpp/rendering/vulkan/presentation/VulkanSurface.cpp

@@ -1,6 +1,7 @@
 #include "rendering/vulkan/presentation/VulkanSurface.h"
 
 #include <algorithm>
+#include <optional>
 #include <utility>
 
 #include <vulkan/vulkan_android.h>
@@ -31,9 +32,8 @@ bool VulkanSurface::attach(ANativeWindow* window) {
 
   ANativeWindow_acquire(window);
   window_ = window;
-  windowExtent_ = {static_cast<uint32_t>(std::max(0, ANativeWindow_getWidth(window))),
-                   static_cast<uint32_t>(std::max(0, ANativeWindow_getHeight(window)))};
-  staleChecks_ = 0;
+  sizeWait_.windowResized({static_cast<uint32_t>(std::max(0, ANativeWindow_getWidth(window))),
+                           static_cast<uint32_t>(std::max(0, ANativeWindow_getHeight(window)))});
   if (!createSurface() || !recreateSwapchain()) {
     LOGE("could not attach to the surface");
     teardown();
@@ -44,10 +44,9 @@ bool VulkanSurface::attach(ANativeWindow* window) {
 
 bool VulkanSurface::resize(uint32_t width, uint32_t height) {
   if (!swapchain_) return true;
-  windowExtent_ = {width, height};
-  staleChecks_ = 0;
+  sizeWait_.windowResized({width, height});
   const VkExtent2D current = swapchain_->extent();
-  if (current.width == width && current.height == height) return true;
+  if (!sizeWait_.pending({current.width, current.height})) return true;
   LOGI("surface resized to %ux%u, swapchain was %ux%u", width, height, current.width,
        current.height);
   return rebuildWhenSurfaceReportsWindowSize();
@@ -84,7 +83,7 @@ bool VulkanSurface::rebuildIfChanged(VulkanFrameLoop::Status status) {
 bool VulkanSurface::rebuildPending() const {
   if (!swapchain_) return false;
   const VkExtent2D current = swapchain_->extent();
-  return current.width != windowExtent_.width || current.height != windowExtent_.height;
+  return sizeWait_.pending({current.width, current.height});
 }
 
 // A rebuild that fails leaves nothing to draw into, so the surface is dropped and stays
@@ -177,26 +176,30 @@ bool VulkanSurface::rebuildOrDrop() {
 // Until a frame is presented, the surface keeps reporting its old size after the window
 // changed, and a swapchain built from that report is the old size again. So the rebuild
 // waits for the report to match the window. A report that never matches ends the wait
-// after `kMaxStaleChecks` frames and the swapchain takes the size the surface gives.
+// after `SurfaceSizeWait::kMaxStaleChecks` frames and the swapchain takes the size the
+// surface gives.
 bool VulkanSurface::rebuildWhenSurfaceReportsWindowSize() {
   VkSurfaceCapabilitiesKHR caps{};
+  std::optional<SurfaceSizeWait::Size> reported;
   if (vkGetPhysicalDeviceSurfaceCapabilitiesKHR(ctx_.physicalDevice(), surface_, &caps) ==
       VK_SUCCESS) {
-    const bool reported = caps.currentExtent.width == windowExtent_.width &&
-                          caps.currentExtent.height == windowExtent_.height;
-    if (reported) {
+    reported = SurfaceSizeWait::Size{caps.currentExtent.width, caps.currentExtent.height};
+  }
+  switch (sizeWait_.check(reported)) {
+    case SurfaceSizeWait::Step::wait:
+      return true;
+    case SurfaceSizeWait::Step::rebuild:
       LOGI("surface now reports %ux%u", caps.currentExtent.width, caps.currentExtent.height);
-      staleChecks_ = 0;
       return rebuildOrDrop();
-    }
+    case SurfaceSizeWait::Step::giveUp:
+      break;
   }
-  if (++staleChecks_ <= kMaxStaleChecks) return true;
+  const SurfaceSizeWait::Size window = sizeWait_.window();
   LOGW("surface still reports %ux%u, window is %ux%u; using the surface's size",
-       caps.currentExtent.width, caps.currentExtent.height, windowExtent_.width,
-       windowExtent_.height);
-  staleChecks_ = 0;
+       caps.currentExtent.width, caps.currentExtent.height, window.width, window.height);
   if (!rebuildOrDrop()) return false;
-  windowExtent_ = swapchain_->extent();
+  const VkExtent2D settled = swapchain_->extent();
+  sizeWait_.adopt({settled.width, settled.height});
   return true;
 }
 

packages/splatkit-android/src/main/cpp/rendering/vulkan/presentation/VulkanSurface.h

@@ -8,6 +8,7 @@
 #include <vulkan/vulkan.h>
 
 #include "rendering/vulkan/device/VulkanContext.h"
+#include "rendering/vulkan/presentation/SurfaceSizeWait.h"
 #include "rendering/vulkan/presentation/VulkanFrameLoop.h"
 #include "rendering/vulkan/presentation/VulkanRenderTarget.h"
 #include "rendering/vulkan/presentation/VulkanSwapchain.h"
@@ -23,9 +24,6 @@ class VulkanSurface {
  public:
   static constexpr float kMinRenderScale = 0.1f;
   static constexpr float kMaxRenderScale = 2.0f;
-  // Frames a rebuild waits for the surface to report the window's size. A surface that never
-  // does (a fixed-size buffer) is built at the size it reports instead of redrawing forever.
-  static constexpr uint32_t kMaxStaleChecks = 60;
 
   // `beforeTeardown` runs while the device is idle, before the target, swapchain and
   // surface are destroyed, so the owner can release what draws into them.
@@ -87,8 +85,7 @@ class VulkanSurface {
   VulkanFrameLoop& frameLoop_;
   std::function<void()> beforeTeardown_;
   ANativeWindow* window_ = nullptr;
-  VkExtent2D windowExtent_{};  // the size the swapchain must reach
-  uint32_t staleChecks_ = 0;
+  SurfaceSizeWait sizeWait_;  // the size the swapchain must reach, and how long to wait for it
   VkSurfaceKHR surface_ = VK_NULL_HANDLE;
   std::unique_ptr<VulkanSwapchain> swapchain_;
   std::unique_ptr<VulkanRenderTarget> target_;  // only when renderScale_ != 1

packages/splatkit-android/tests/rendering/vulkan/presentation/SurfaceSizeWaitTest.cpp

@@ -0,0 +1,111 @@
+// Host-only test, built with an include path for splatkit-android/src/main/cpp.
+// Run without NDEBUG.
+#include <cassert>
+
+#include "rendering/vulkan/presentation/SurfaceSizeWait.h"
+
+using splatkit::SurfaceSizeWait;
+using Size = SurfaceSizeWait::Size;
+using Step = SurfaceSizeWait::Step;
+
+namespace {
+
+constexpr Size kPortrait{1080, 2340};
+constexpr Size kLandscape{2340, 1080};
+
+void convergesAtOnceWhenTheSurfaceAlreadyReportsTheWindow() {
+  SurfaceSizeWait wait;
+  wait.windowResized(kLandscape);
+  assert(wait.pending(kPortrait));
+  assert(!wait.pending(kLandscape));
+  assert(wait.check(kLandscape) == Step::rebuild);
+}
+
+void waitsWhileStaleAndRebuildsOnTheFirstMatchingFrame() {
+  SurfaceSizeWait wait;
+  wait.windowResized(kLandscape);
+  for (int frame = 0; frame < 10; ++frame) assert(wait.check(kPortrait) == Step::wait);
+  assert(wait.check(kLandscape) == Step::rebuild);
+}
+
+void aFailedQueryCountsAsStale() {
+  SurfaceSizeWait wait;
+  wait.windowResized(kLandscape);
+  assert(wait.check(std::nullopt) == Step::wait);
+  assert(wait.check(kLandscape) == Step::rebuild);
+}
+
+void givesUpAfterExactlyTheMaximumStaleFrames() {
+  SurfaceSizeWait wait;
+  wait.windowResized(kLandscape);
+  for (uint32_t frame = 0; frame < SurfaceSizeWait::kMaxStaleChecks; ++frame) {
+    assert(wait.check(kPortrait) == Step::wait);
+  }
+  assert(wait.check(kPortrait) == Step::giveUp);
+  wait.adopt(kPortrait);
+  assert(wait.window() == kPortrait);
+  assert(!wait.pending(kPortrait));
+}
+
+void aMatchOnTheLastWaitingFrameStillRebuilds() {
+  SurfaceSizeWait wait;
+  wait.windowResized(kLandscape);
+  for (uint32_t frame = 0; frame + 1 < SurfaceSizeWait::kMaxStaleChecks; ++frame) {
+    assert(wait.check(kPortrait) == Step::wait);
+  }
+  assert(wait.check(kLandscape) == Step::rebuild);
+}
+
+void theCountRestartsAfterAGiveUp() {
+  SurfaceSizeWait wait;
+  wait.windowResized(kLandscape);
+  for (uint32_t frame = 0; frame <= SurfaceSizeWait::kMaxStaleChecks; ++frame) {
+    wait.check(kPortrait);
+  }
+  wait.adopt(kPortrait);
+  wait.windowResized(kLandscape);
+  for (uint32_t frame = 0; frame < SurfaceSizeWait::kMaxStaleChecks; ++frame) {
+    assert(wait.check(kPortrait) == Step::wait);
+  }
+  assert(wait.check(kPortrait) == Step::giveUp);
+}
+
+void aMatchResetsTheCount() {
+  SurfaceSizeWait wait;
+  wait.windowResized(kLandscape);
+  for (uint32_t frame = 0; frame < SurfaceSizeWait::kMaxStaleChecks - 1; ++frame) {
+    wait.check(kPortrait);
+  }
+  assert(wait.check(kLandscape) == Step::rebuild);
+  for (uint32_t frame = 0; frame < SurfaceSizeWait::kMaxStaleChecks; ++frame) {
+    assert(wait.check(kPortrait) == Step::wait);
+  }
+  assert(wait.check(kPortrait) == Step::giveUp);
+}
+
+void aNewResizeResetsTheCount() {
+  SurfaceSizeWait wait;
+  wait.windowResized(kLandscape);
+  for (uint32_t frame = 0; frame < SurfaceSizeWait::kMaxStaleChecks - 1; ++frame) {
+    assert(wait.check(kPortrait) == Step::wait);
+  }
+  wait.windowResized(kPortrait);
+  assert(wait.window() == kPortrait);
+  for (uint32_t frame = 0; frame < SurfaceSizeWait::kMaxStaleChecks; ++frame) {
+    assert(wait.check(kLandscape) == Step::wait);
+  }
+  assert(wait.check(kLandscape) == Step::giveUp);
+}
+
+}  // namespace
+
+int main() {
+  convergesAtOnceWhenTheSurfaceAlreadyReportsTheWindow();
+  waitsWhileStaleAndRebuildsOnTheFirstMatchingFrame();
+  aFailedQueryCountsAsStale();
+  givesUpAfterExactlyTheMaximumStaleFrames();
+  aMatchOnTheLastWaitingFrameStillRebuilds();
+  theCountRestartsAfterAGiveUp();
+  aMatchResetsTheCount();
+  aNewResizeResetsTheCount();
+}