Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 0b9ebe5

Browse files
null77Commit Bot
authored andcommitted
Vulkan: Add "ImageViewHelper".
This allows views to track a different lifetime than vk::ImageHelper. This in turn will fix the race condition on ContextVk destruction when releasing ImageViews owned by TextureVk and RenderbufferVk. For now this is a refactoring change only. Bug: angleproject:2464 Change-Id: I9581975bd5d4913233bbed8439dd4a632cc78a2a Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1843231 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: Ian Elliott <ianelliott@google.com>
1 parent 81ab9c9 commit 0b9ebe5

File tree

11 files changed

+355
-254
lines changed

11 files changed

+355
-254
lines changed

src/libANGLE/renderer/vulkan/ProgramVk.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1472,8 +1472,8 @@ angle::Result ProgramVk::updateImagesDescriptorSet(ContextVk *contextVk,
14721472
vk::ImageHelper *image = &textureVk->getImage();
14731473
const vk::ImageView *imageView = nullptr;
14741474

1475-
ANGLE_TRY(textureVk->getLayerLevelStorageImageView(
1476-
contextVk, (binding.layered == GL_TRUE), binding.layer, binding.level, &imageView));
1475+
ANGLE_TRY(textureVk->getStorageImageView(contextVk, (binding.layered == GL_TRUE),
1476+
binding.level, binding.layer, &imageView));
14771477

14781478
// Note: binding.access is unused because it is implied by the shader.
14791479

src/libANGLE/renderer/vulkan/RenderTargetVk.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ class RenderTargetVk final : public FramebufferAttachmentRenderTarget
9191
uint32_t mLayerIndex;
9292
};
9393

94+
// A vector of rendertargets
95+
using RenderTargetVector = std::vector<RenderTargetVk>;
96+
9497
} // namespace rx
9598

9699
#endif // LIBANGLE_RENDERER_VULKAN_RENDERTARGETVK_H_

src/libANGLE/renderer/vulkan/RenderbufferVk.cpp

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -78,17 +78,13 @@ angle::Result RenderbufferVk::setStorageImpl(const gl::Context *context,
7878
VkMemoryPropertyFlags flags = VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
7979
ANGLE_TRY(mImage->initMemory(contextVk, renderer->getMemoryProperties(), flags));
8080

81-
VkImageAspectFlags aspect = vk::GetFormatAspectFlags(textureFormat);
82-
83-
// Note that LUMA textures are not color-renderable, so a read-view with swizzle is not
84-
// needed.
85-
ANGLE_TRY(mImage->initImageView(contextVk, gl::TextureType::_2D, aspect, gl::SwizzleState(),
86-
&mImageView, 0, 1));
81+
const vk::ImageView *imageView = nullptr;
82+
ANGLE_TRY(mImageViews.getLevelLayerDrawImageView(contextVk, *mImage, 0, 0, &imageView));
8783

8884
// Clear the renderbuffer if it has emulated channels.
8985
mImage->stageClearIfEmulatedFormat(gl::ImageIndex::Make2D(0), vkFormat);
9086

91-
mRenderTarget.init(mImage, &mImageView, 0, 0);
87+
mRenderTarget.init(mImage, imageView, 0, 0);
9288
}
9389

9490
return angle::Result::Continue;
@@ -176,11 +172,11 @@ angle::Result RenderbufferVk::setStorageEGLImageTarget(const gl::Context *contex
176172
imageVk->getImage()->getSamples());
177173
}
178174

179-
ANGLE_TRY(mImage->initLayerImageView(contextVk, viewType, aspect, gl::SwizzleState(),
180-
&mImageView, imageVk->getImageLevel(), 1,
181-
imageVk->getImageLayer(), 1));
175+
const vk::ImageView *imageView = nullptr;
176+
ANGLE_TRY(mImageViews.getLevelLayerDrawImageView(contextVk, *mImage, imageVk->getImageLevel(),
177+
imageVk->getImageLayer(), &imageView));
182178

183-
mRenderTarget.init(mImage, &mImageView, imageVk->getImageLevel(), imageVk->getImageLayer());
179+
mRenderTarget.init(mImage, imageView, imageVk->getImageLevel(), imageVk->getImageLayer());
184180

185181
return angle::Result::Continue;
186182
}
@@ -232,7 +228,7 @@ void RenderbufferVk::releaseImage(ContextVk *contextVk)
232228
mImage = nullptr;
233229
}
234230

235-
contextVk->addGarbage(&mImageView);
231+
mImageViews.release(contextVk);
236232
}
237233

238234
} // namespace rx

src/libANGLE/renderer/vulkan/RenderbufferVk.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class RenderbufferVk : public RenderbufferImpl
6060

6161
bool mOwnsImage;
6262
vk::ImageHelper *mImage;
63-
vk::ImageView mImageView;
63+
vk::ImageViewHelper mImageViews;
6464
RenderTargetVk mRenderTarget;
6565
};
6666

src/libANGLE/renderer/vulkan/SurfaceVk.cpp

Lines changed: 37 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,7 @@ angle::Result OffscreenSurfaceVk::AttachmentImage::initialize(DisplayVk *display
148148

149149
VkMemoryPropertyFlags flags = VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
150150
ANGLE_TRY(image.initMemory(displayVk, renderer->getMemoryProperties(), flags));
151-
152-
VkImageAspectFlags aspect = vk::GetFormatAspectFlags(textureFormat);
153-
154-
ANGLE_TRY(image.initImageView(displayVk, gl::TextureType::_2D, aspect, gl::SwizzleState(),
155-
&imageView, 0, 1));
151+
ANGLE_TRY(imageViews.getLevelLayerDrawImageView(displayVk, image, 0, 0, &drawView));
156152

157153
// Clear the image if it has emulated channels.
158154
image.stageClearIfEmulatedFormat(gl::ImageIndex::Make2D(0), vkFormat);
@@ -169,18 +165,14 @@ void OffscreenSurfaceVk::AttachmentImage::destroy(const egl::Display *display)
169165
// destruction. If this assumption is incorrect, we could use the last submit serial
170166
// to determine when to destroy the surface.
171167
image.destroy(device);
172-
imageView.destroy(device);
168+
imageViews.destroy(device);
173169
}
174170

175171
OffscreenSurfaceVk::OffscreenSurfaceVk(const egl::SurfaceState &surfaceState,
176172
EGLint width,
177173
EGLint height)
178174
: SurfaceVk(surfaceState), mWidth(width), mHeight(height)
179-
{
180-
mColorRenderTarget.init(&mColorAttachment.image, &mColorAttachment.imageView, 0, 0);
181-
mDepthStencilRenderTarget.init(&mDepthStencilAttachment.image,
182-
&mDepthStencilAttachment.imageView, 0, 0);
183-
}
175+
{}
184176

185177
OffscreenSurfaceVk::~OffscreenSurfaceVk() {}
186178

@@ -203,12 +195,15 @@ angle::Result OffscreenSurfaceVk::initializeImpl(DisplayVk *displayVk)
203195
{
204196
ANGLE_TRY(mColorAttachment.initialize(
205197
displayVk, mWidth, mHeight, renderer->getFormat(config->renderTargetFormat), samples));
198+
mColorRenderTarget.init(&mColorAttachment.image, mColorAttachment.drawView, 0, 0);
206199
}
207200

208201
if (config->depthStencilFormat != GL_NONE)
209202
{
210203
ANGLE_TRY(mDepthStencilAttachment.initialize(
211204
displayVk, mWidth, mHeight, renderer->getFormat(config->depthStencilFormat), samples));
205+
mDepthStencilRenderTarget.init(&mDepthStencilAttachment.image,
206+
mDepthStencilAttachment.drawView, 0, 0);
212207
}
213208

214209
return angle::Result::Continue;
@@ -363,7 +358,7 @@ SwapchainImage::~SwapchainImage() = default;
363358

364359
SwapchainImage::SwapchainImage(SwapchainImage &&other)
365360
: image(std::move(other.image)),
366-
imageView(std::move(other.imageView)),
361+
imageViews(std::move(other.imageViews)),
367362
framebuffer(std::move(other.framebuffer)),
368363
presentHistory(std::move(other.presentHistory)),
369364
currentPresentHistoryIndex(other.currentPresentHistoryIndex)
@@ -405,12 +400,7 @@ WindowSurfaceVk::WindowSurfaceVk(const egl::SurfaceState &surfaceState,
405400
mCompositeAlpha(VK_COMPOSITE_ALPHA_OPAQUE_BIT_KHR),
406401
mCurrentSwapHistoryIndex(0),
407402
mCurrentSwapchainImageIndex(0)
408-
{
409-
// Initialize the color render target with the multisampled targets. If not multisampled, the
410-
// render target will be updated to refer to a swapchain image on every acquire.
411-
mColorRenderTarget.init(&mColorImageMS, &mColorImageViewMS, 0, 0);
412-
mDepthStencilRenderTarget.init(&mDepthStencilImage, &mDepthStencilImageView, 0, 0);
413-
}
403+
{}
414404

415405
WindowSurfaceVk::~WindowSurfaceVk()
416406
{
@@ -776,9 +766,13 @@ angle::Result WindowSurfaceVk::createSwapChain(vk::Context *context,
776766
ANGLE_TRY(mColorImageMS.initMemory(context, renderer->getMemoryProperties(),
777767
VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT));
778768

779-
ANGLE_TRY(mColorImageMS.initImageView(context, gl::TextureType::_2D,
780-
VK_IMAGE_ASPECT_COLOR_BIT, gl::SwizzleState(),
781-
&mColorImageViewMS, 0, 1));
769+
const vk::ImageView *msDrawView = nullptr;
770+
ANGLE_TRY(mColorImageMSViews.getLevelLayerDrawImageView(context, mColorImageMS, 0, 0,
771+
&msDrawView));
772+
773+
// Initialize the color render target with the multisampled targets. If not multisampled,
774+
// the render target will be updated to refer to a swapchain image on every acquire.
775+
mColorRenderTarget.init(&mColorImageMS, msDrawView, 0, 0);
782776

783777
// Clear the image if it has emulated channels.
784778
mColorImageMS.stageClearIfEmulatedFormat(gl::ImageIndex::Make2D(0), format);
@@ -796,9 +790,8 @@ angle::Result WindowSurfaceVk::createSwapChain(vk::Context *context,
796790
// If the multisampled image is used, we don't need a view on the swapchain image, as
797791
// it's only used as a resolve destination. This has the added benefit that we can't
798792
// accidentally use this image.
799-
ANGLE_TRY(member.image.initImageView(context, gl::TextureType::_2D,
800-
VK_IMAGE_ASPECT_COLOR_BIT, gl::SwizzleState(),
801-
&member.imageView, 0, 1));
793+
ANGLE_TRY(member.imageViews.getLevelLayerDrawImageView(context, member.image, 0, 0,
794+
&member.drawView));
802795

803796
// Clear the image if it has emulated channels. If a multisampled image exists, this
804797
// image will be unused until a pre-present resolve, at which point it will be fully
@@ -819,11 +812,11 @@ angle::Result WindowSurfaceVk::createSwapChain(vk::Context *context,
819812
ANGLE_TRY(mDepthStencilImage.initMemory(context, renderer->getMemoryProperties(),
820813
VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT));
821814

822-
const VkImageAspectFlags aspect = vk::GetDepthStencilAspectFlags(dsFormat.imageFormat());
815+
const vk::ImageView *dsDrawView = nullptr;
816+
ANGLE_TRY(mDepthStencilImageViews.getLevelLayerDrawImageView(context, mDepthStencilImage, 0,
817+
0, &dsDrawView));
823818

824-
ANGLE_TRY(mDepthStencilImage.initImageView(context, gl::TextureType::_2D, aspect,
825-
gl::SwizzleState(), &mDepthStencilImageView, 0,
826-
1));
819+
mDepthStencilRenderTarget.init(&mDepthStencilImage, dsDrawView, 0, 0);
827820

828821
// We will need to pass depth/stencil image views to the RenderTargetVk in the future.
829822

@@ -893,19 +886,14 @@ void WindowSurfaceVk::releaseSwapchainImages(ContextVk *contextVk)
893886
{
894887
mDepthStencilImage.releaseImage(renderer);
895888
mDepthStencilImage.releaseStagingBuffer(renderer);
896-
897-
if (mDepthStencilImageView.valid())
898-
{
899-
contextVk->addGarbage(&mDepthStencilImageView);
900-
}
889+
mDepthStencilImageViews.release(contextVk);
901890
}
902891

903892
if (mColorImageMS.valid())
904893
{
905894
mColorImageMS.releaseImage(renderer);
906895
mColorImageMS.releaseStagingBuffer(renderer);
907-
908-
contextVk->addGarbage(&mColorImageViewMS);
896+
mColorImageMSViews.release(contextVk);
909897
contextVk->addGarbage(&mFramebufferMS);
910898
}
911899

@@ -915,7 +903,7 @@ void WindowSurfaceVk::releaseSwapchainImages(ContextVk *contextVk)
915903
swapchainImage.image.resetImageWeakReference();
916904
swapchainImage.image.destroy(contextVk->getDevice());
917905

918-
contextVk->addGarbage(&swapchainImage.imageView);
906+
swapchainImage.imageViews.release(contextVk);
919907
contextVk->addGarbage(&swapchainImage.framebuffer);
920908

921909
// present history must have already been taken care of.
@@ -934,17 +922,17 @@ void WindowSurfaceVk::destroySwapChainImages(DisplayVk *displayVk)
934922
VkDevice device = displayVk->getDevice();
935923

936924
mDepthStencilImage.destroy(device);
937-
mDepthStencilImageView.destroy(device);
925+
mDepthStencilImageViews.destroy(device);
938926
mColorImageMS.destroy(device);
939-
mColorImageViewMS.destroy(device);
927+
mColorImageMSViews.destroy(device);
940928
mFramebufferMS.destroy(device);
941929

942930
for (SwapchainImage &swapchainImage : mSwapchainImages)
943931
{
944932
// We don't own the swapchain image handles, so we just remove our reference to it.
945933
swapchainImage.image.resetImageWeakReference();
946934
swapchainImage.image.destroy(device);
947-
swapchainImage.imageView.destroy(device);
935+
swapchainImage.imageViews.destroy(device);
948936
swapchainImage.framebuffer.destroy(device);
949937

950938
for (ImagePresentHistory &presentHistory : swapchainImage.presentHistory)
@@ -1198,7 +1186,7 @@ VkResult WindowSurfaceVk::nextSwapchainImage(vk::Context *context)
11981186
// multisampling, as the swapchain image is essentially unused until then.
11991187
if (!mColorImageMS.valid())
12001188
{
1201-
mColorRenderTarget.updateSwapchainImage(&image.image, &image.imageView);
1189+
mColorRenderTarget.updateSwapchainImage(&image.image, image.drawView);
12021190
}
12031191

12041192
return VK_SUCCESS;
@@ -1313,7 +1301,12 @@ angle::Result WindowSurfaceVk::getCurrentFramebuffer(vk::Context *context,
13131301
VkFramebufferCreateInfo framebufferInfo = {};
13141302

13151303
const gl::Extents extents = mColorRenderTarget.getExtents();
1316-
std::array<VkImageView, 2> imageViews = {{VK_NULL_HANDLE, mDepthStencilImageView.getHandle()}};
1304+
std::array<VkImageView, 2> imageViews = {};
1305+
1306+
if (mDepthStencilImage.valid())
1307+
{
1308+
imageViews[1] = mDepthStencilRenderTarget.getDrawImageView()->getHandle();
1309+
}
13171310

13181311
framebufferInfo.sType = VK_STRUCTURE_TYPE_FRAMEBUFFER_CREATE_INFO;
13191312
framebufferInfo.flags = 0;
@@ -1327,14 +1320,14 @@ angle::Result WindowSurfaceVk::getCurrentFramebuffer(vk::Context *context,
13271320
if (isMultiSampled())
13281321
{
13291322
// If multisampled, there is only a single color image and framebuffer.
1330-
imageViews[0] = mColorImageViewMS.getHandle();
1323+
imageViews[0] = mColorRenderTarget.getDrawImageView()->getHandle();
13311324
ANGLE_VK_TRY(context, mFramebufferMS.init(context->getDevice(), framebufferInfo));
13321325
}
13331326
else
13341327
{
13351328
for (SwapchainImage &swapchainImage : mSwapchainImages)
13361329
{
1337-
imageViews[0] = swapchainImage.imageView.getHandle();
1330+
imageViews[0] = swapchainImage.drawView->getHandle();
13381331
ANGLE_VK_TRY(context,
13391332
swapchainImage.framebuffer.init(context->getDevice(), framebufferInfo));
13401333
}
@@ -1399,7 +1392,7 @@ angle::Result WindowSurfaceVk::updateAndDrawOverlay(ContextVk *contextVk,
13991392
}
14001393

14011394
// Draw overlay
1402-
ANGLE_TRY(overlayVk->onPresent(contextVk, &image->image, &image->imageView));
1395+
ANGLE_TRY(overlayVk->onPresent(contextVk, &image->image, image->drawView));
14031396

14041397
overlay->getRunningGraphWidget(gl::WidgetId::VulkanCommandGraphSize)->next();
14051398

src/libANGLE/renderer/vulkan/SurfaceVk.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ class OffscreenSurfaceVk : public SurfaceVk
8888
void destroy(const egl::Display *display);
8989

9090
vk::ImageHelper image;
91-
vk::ImageView imageView;
91+
vk::ImageViewHelper imageViews;
92+
const vk::ImageView *drawView = nullptr;
9293
};
9394

9495
angle::Result initializeImpl(DisplayVk *displayVk);
@@ -162,7 +163,8 @@ struct SwapchainImage : angle::NonCopyable
162163
~SwapchainImage();
163164

164165
vk::ImageHelper image;
165-
vk::ImageView imageView;
166+
vk::ImageViewHelper imageViews;
167+
const vk::ImageView *drawView = nullptr;
166168
vk::Framebuffer framebuffer;
167169

168170
// A circular array of semaphores used for presenting this image.
@@ -286,11 +288,11 @@ class WindowSurfaceVk : public SurfaceVk
286288

287289
// Depth/stencil image. Possibly multisampled.
288290
vk::ImageHelper mDepthStencilImage;
289-
vk::ImageView mDepthStencilImageView;
291+
vk::ImageViewHelper mDepthStencilImageViews;
290292

291293
// Multisample color image, view and framebuffer, if multisampling enabled.
292294
vk::ImageHelper mColorImageMS;
293-
vk::ImageView mColorImageViewMS;
295+
vk::ImageViewHelper mColorImageMSViews;
294296
vk::Framebuffer mFramebufferMS;
295297
};
296298

0 commit comments

Comments
 (0)