From 542dd331d76bc535d8f7d0eaa14ad41aadf5ace5 Mon Sep 17 00:00:00 2001
From: Philip Rideout <philiprideout@gmail.com>
Date: Wed, 23 Feb 2022 13:19:10 -0800
Subject: [PATCH] Vulkan: Add asserts that check render target size.

---
 filament/backend/src/vulkan/VulkanContext.cpp |  9 +++-
 filament/backend/src/vulkan/VulkanContext.h   |  3 +-
 filament/backend/src/vulkan/VulkanDriver.cpp  | 42 +++++++++++++------
 3 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/filament/backend/src/vulkan/VulkanContext.cpp b/filament/backend/src/vulkan/VulkanContext.cpp
index c0fb00e7c4..c29af81435 100644
--- a/filament/backend/src/vulkan/VulkanContext.cpp
+++ b/filament/backend/src/vulkan/VulkanContext.cpp
@@ -20,6 +20,8 @@
 #include "VulkanTexture.h"
 #include "VulkanUtility.h"
 
+#include <algorithm> // for std::max
+
 #include <utils/Panic.h>
 #include <utils/FixedCapacityVector.h>
 
@@ -42,7 +44,12 @@ VkImageLayout VulkanAttachment::getLayout() const {
     return texture ? texture->getVkLayout(layer, level) : VK_IMAGE_LAYOUT_UNDEFINED;
 }
 
-VkImageView VulkanAttachment::getImageView(VkImageAspectFlags aspect) {
+VkExtent2D VulkanAttachment::getExtent2D() const {
+    assert_invariant(texture);
+    return { std::max(1u, texture->width >> level), std::max(1u, texture->height >> level) };
+}
+
+VkImageView VulkanAttachment::getImageView(VkImageAspectFlags aspect) const {
     assert_invariant(texture);
     return texture->getAttachmentView(level, layer, aspect);
 }
diff --git a/filament/backend/src/vulkan/VulkanContext.h b/filament/backend/src/vulkan/VulkanContext.h
index def3668a97..4c24d8cc93 100644
--- a/filament/backend/src/vulkan/VulkanContext.h
+++ b/filament/backend/src/vulkan/VulkanContext.h
@@ -46,7 +46,8 @@ struct VulkanAttachment {
     VkImage getImage() const;
     VkFormat getFormat() const;
     VkImageLayout getLayout() const;
-    VkImageView getImageView(VkImageAspectFlags aspect);
+    VkExtent2D getExtent2D() const;
+    VkImageView getImageView(VkImageAspectFlags aspect) const;
 };
 
 struct VulkanTimestamps {
diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp
index ca35173643..2e64f34224 100644
--- a/filament/backend/src/vulkan/VulkanDriver.cpp
+++ b/filament/backend/src/vulkan/VulkanDriver.cpp
@@ -536,22 +536,36 @@ void VulkanDriver::createRenderTargetR(Handle<HwRenderTarget> rth,
     VulkanAttachment colorTargets[MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT] = {};
     for (int i = 0; i < MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT; i++) {
         if (color[i].handle) {
-            colorTargets[i].texture = handle_cast<VulkanTexture*>(color[i].handle);
+            colorTargets[i] = {
+                .texture = handle_cast<VulkanTexture*>(color[i].handle),
+                .level = color[i].level,
+                .layer = color[i].layer,
+            };
+            UTILS_UNUSED_IN_RELEASE VkExtent2D extent = colorTargets[i].getExtent2D();
+            assert_invariant(extent.width >= width && extent.height >= height);
         }
-        colorTargets[i].level = color[i].level;
-        colorTargets[i].layer = color[i].layer;
     }
 
     VulkanAttachment depthStencil[2] = {};
-    TextureHandle handle = depth.handle;
-    depthStencil[0].texture = handle ? handle_cast<VulkanTexture*>(handle) : nullptr;
-    depthStencil[0].level = depth.level;
-    depthStencil[0].layer = depth.layer;
+    if (depth.handle) {
+        depthStencil[0] = {
+            .texture = handle_cast<VulkanTexture*>(depth.handle),
+            .level = depth.level,
+            .layer = depth.layer,
+        };
+        UTILS_UNUSED_IN_RELEASE VkExtent2D extent = depthStencil[0].getExtent2D();
+        assert_invariant(extent.width >= width && extent.height >= height);
+    }
 
-    handle = stencil.handle;
-    depthStencil[1].texture = handle ? handle_cast<VulkanTexture*>(handle) : nullptr;
-    depthStencil[1].level = stencil.level;
-    depthStencil[1].layer = stencil.layer;
+    if (stencil.handle) {
+        depthStencil[1] = {
+            .texture = handle_cast<VulkanTexture*>(stencil.handle),
+            .level = stencil.level,
+            .layer = stencil.layer,
+        };
+        UTILS_UNUSED_IN_RELEASE VkExtent2D extent = depthStencil[1].getExtent2D();
+        assert_invariant(extent.width >= width && extent.height >= height);
+    }
 
     auto renderTarget = construct<VulkanRenderTarget>(rth, mContext,
             width, height, samples, colorTargets, depthStencil, mStagePool);
@@ -1551,7 +1565,11 @@ void VulkanDriver::readPixels(Handle<HwRenderTarget> src, uint32_t x, uint32_t y
     // Perform the copy into the staging area. At this point we know that the src layout is
     // TRANSFER_SRC_OPTIMAL and the staging area is GENERAL.
 
-    vkCmdCopyImage(cmdbuffer, srcTarget->getColor(0).getImage(),
+    UTILS_UNUSED_IN_RELEASE VkExtent2D srcExtent = srcAttachment.getExtent2D();
+    assert_invariant(imageCopyRegion.srcOffset.x + imageCopyRegion.extent.width <= srcExtent.width);
+    assert_invariant(imageCopyRegion.srcOffset.y + imageCopyRegion.extent.height <= srcExtent.height);
+
+    vkCmdCopyImage(cmdbuffer, srcAttachment.getImage(),
             VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, stagingImage, VK_IMAGE_LAYOUT_GENERAL,
             1, &imageCopyRegion);