New approach to fixing Vulkan MSAA.
MakeFromBackendTextureAsRenderTarget is planned to be deprecated, so we
should use MakeFromBackendTexture with a sampleCount parameter instead.
On Vulkan, this ran into issues because we assumed an allocation for the
VkImage and the swapchain doesn't provide us with one. Fixed so we don't
need an allocation for Borrowed textures.
Bug: skia:
Change-Id: Ib26888020e093f4a734a4159eae898539c2273b7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/226839
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index 276e02a..89248fc 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -666,6 +666,9 @@
&layout));
const GrVkAlloc& alloc = tex->alloc();
+ if (VK_NULL_HANDLE == alloc.fMemory) {
+ return false;
+ }
VkDeviceSize offset = top * layout.rowPitch + left * bpp;
VkDeviceSize size = height*layout.rowPitch;
SkASSERT(size + offset <= alloc.fSize);
@@ -1104,12 +1107,12 @@
static bool check_image_info(const GrVkCaps& caps,
const GrVkImageInfo& info,
GrPixelConfig config,
- bool isWrappedRT) {
+ bool needsAllocation) {
if (VK_NULL_HANDLE == info.fImage) {
return false;
}
- if (VK_NULL_HANDLE == info.fAlloc.fMemory && !isWrappedRT) {
+ if (VK_NULL_HANDLE == info.fAlloc.fMemory && needsAllocation) {
return false;
}
@@ -1156,7 +1159,8 @@
return nullptr;
}
- if (!check_image_info(this->vkCaps(), imageInfo, backendTex.config(), false)) {
+ if (!check_image_info(this->vkCaps(), imageInfo, backendTex.config(),
+ kAdopt_GrWrapOwnership == ownership)) {
return nullptr;
}
if (!check_tex_image_info(this->vkCaps(), imageInfo)) {
@@ -1190,7 +1194,8 @@
return nullptr;
}
- if (!check_image_info(this->vkCaps(), imageInfo, backendTex.config(), false)) {
+ if (!check_image_info(this->vkCaps(), imageInfo, backendTex.config(),
+ kAdopt_GrWrapOwnership == ownership)) {
return nullptr;
}
if (!check_tex_image_info(this->vkCaps(), imageInfo)) {
@@ -1233,7 +1238,7 @@
return nullptr;
}
- if (!check_image_info(this->vkCaps(), info, backendRT.config(), true)) {
+ if (!check_image_info(this->vkCaps(), info, backendRT.config(), false)) {
return nullptr;
}
if (!check_rt_image_info(this->vkCaps(), info)) {
@@ -1273,10 +1278,7 @@
if (!tex.getVkImageInfo(&imageInfo)) {
return nullptr;
}
- // In some cases, we use this method to create an MSAA RT with the given texture
- // as the resolve RT. Because of this, the texture may have no backing memory,
- // so we treat the texture as an RT.
- if (!check_image_info(this->vkCaps(), imageInfo, tex.config(), true)) {
+ if (!check_image_info(this->vkCaps(), imageInfo, tex.config(), false)) {
return nullptr;
}
if (!check_rt_image_info(this->vkCaps(), imageInfo)) {
diff --git a/src/gpu/vk/GrVkImage.h b/src/gpu/vk/GrVkImage.h
index 10083a8..222fdd5 100644
--- a/src/gpu/vk/GrVkImage.h
+++ b/src/gpu/vk/GrVkImage.h
@@ -36,6 +36,7 @@
} else if (fIsBorrowed) {
fResource = new BorrowedResource(info.fImage, info.fAlloc, info.fImageTiling);
} else {
+ SkASSERT(VK_NULL_HANDLE != info.fAlloc.fMemory);
fResource = new Resource(info.fImage, info.fAlloc, info.fImageTiling);
}
}
diff --git a/src/gpu/vk/GrVkTexture.cpp b/src/gpu/vk/GrVkTexture.cpp
index eb0637f..4fba188 100644
--- a/src/gpu/vk/GrVkTexture.cpp
+++ b/src/gpu/vk/GrVkTexture.cpp
@@ -95,8 +95,9 @@
GrWrapCacheable cacheable, GrIOType ioType,
const GrVkImageInfo& info,
sk_sp<GrVkImageLayout> layout) {
- // Wrapped textures require both image and allocation (because they can be mapped)
- SkASSERT(VK_NULL_HANDLE != info.fImage && VK_NULL_HANDLE != info.fAlloc.fMemory);
+ // Adopted textures require both image and allocation because we're responsible for freeing
+ SkASSERT(VK_NULL_HANDLE != info.fImage &&
+ (kBorrow_GrWrapOwnership == wrapOwnership || VK_NULL_HANDLE != info.fAlloc.fMemory));
const GrVkImageView* imageView = GrVkImageView::Create(
gpu, info.fImage, info.fFormat, GrVkImageView::kColor_Type, info.fLevelCount,
diff --git a/src/gpu/vk/GrVkTextureRenderTarget.cpp b/src/gpu/vk/GrVkTextureRenderTarget.cpp
index 4b47481..bba4103 100644
--- a/src/gpu/vk/GrVkTextureRenderTarget.cpp
+++ b/src/gpu/vk/GrVkTextureRenderTarget.cpp
@@ -207,8 +207,9 @@
GrWrapCacheable cacheable,
const GrVkImageInfo& info,
sk_sp<GrVkImageLayout> layout) {
- // Wrapped textures require both image and allocation (because they can be mapped)
- SkASSERT(VK_NULL_HANDLE != info.fImage && VK_NULL_HANDLE != info.fAlloc.fMemory);
+ // Adopted textures require both image and allocation because we're responsible for freeing
+ SkASSERT(VK_NULL_HANDLE != info.fImage &&
+ (kBorrow_GrWrapOwnership == wrapOwnership || VK_NULL_HANDLE != info.fAlloc.fMemory));
GrMipMapsStatus mipMapsStatus = info.fLevelCount > 1 ? GrMipMapsStatus::kDirty
: GrMipMapsStatus::kNotAllocated;
diff --git a/tests/VkWrapTests.cpp b/tests/VkWrapTests.cpp
index 75d5e0c..a6fb439 100644
--- a/tests/VkWrapTests.cpp
+++ b/tests/VkWrapTests.cpp
@@ -71,7 +71,7 @@
backendTex.setPixelConfig(kPixelConfig);
tex = gpu->wrapBackendTexture(backendTex, kBorrow_GrWrapOwnership, GrWrapCacheable::kNo,
kRead_GrIOType);
- REPORTER_ASSERT(reporter, !tex);
+ REPORTER_ASSERT(reporter, tex);
tex = gpu->wrapBackendTexture(backendTex, kAdopt_GrWrapOwnership, GrWrapCacheable::kNo,
kRead_GrIOType);
REPORTER_ASSERT(reporter, !tex);
@@ -172,7 +172,7 @@
backendTex.setPixelConfig(kPixelConfig);
tex = gpu->wrapRenderableBackendTexture(backendTex, 1, kBorrow_GrWrapOwnership,
GrWrapCacheable::kNo);
- REPORTER_ASSERT(reporter, !tex);
+ REPORTER_ASSERT(reporter, tex);
tex = gpu->wrapRenderableBackendTexture(backendTex, 1, kAdopt_GrWrapOwnership,
GrWrapCacheable::kNo);
REPORTER_ASSERT(reporter, !tex);
diff --git a/tools/sk_app/VulkanWindowContext.cpp b/tools/sk_app/VulkanWindowContext.cpp
index d26c19e..f04649b 100644
--- a/tools/sk_app/VulkanWindowContext.cpp
+++ b/tools/sk_app/VulkanWindowContext.cpp
@@ -353,7 +353,7 @@
} else {
GrBackendTexture backendTexture(fWidth, fHeight, info);
- fSurfaces[i] = SkSurface::MakeFromBackendTextureAsRenderTarget(
+ fSurfaces[i] = SkSurface::MakeFromBackendTexture(
fContext.get(), backendTexture, kTopLeft_GrSurfaceOrigin, fSampleCount,
colorType, fDisplayParams.fColorSpace, &fDisplayParams.fSurfaceProps);
