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

Commit 86ca5d2

Browse files
cclaoCommit Bot
authored andcommitted
Vulkan: Add plumbing to render pass when ImageHelper gets deleted
ImageHelper object is not refcounted and garbage collected and endRenderPass call is deferred until next render pass starts. This caused a situation that an ImageHelper object gets deleted while still referenced in the open render pass. This CL make sure that we call into all shared context's open renderpass when an image goes away so that they can take appropriate action for this. Bug: b/169618408 Change-Id: I5075e805980084db82ca3e699462272eee5d2d59 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2443571 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com> Commit-Queue: Charlie Lao <cclao@google.com>
1 parent da61c40 commit 86ca5d2

File tree

11 files changed

+62
-9
lines changed

11 files changed

+62
-9
lines changed

src/libANGLE/renderer/vulkan/ContextVk.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -773,6 +773,9 @@ ContextVk::~ContextVk() = default;
773773

774774
void ContextVk::onDestroy(const gl::Context *context)
775775
{
776+
// Remove context from the share group
777+
mShareGroupVk->getShareContextSet()->erase(this);
778+
776779
// This will not destroy any resources. It will release them to be collected after finish.
777780
mIncompleteTextures.onDestroy(context);
778781

@@ -974,6 +977,9 @@ angle::Result ContextVk::initialize()
974977
mStagingBuffer.init(mRenderer, kStagingBufferUsageFlags, stagingBufferAlignment,
975978
kStagingBufferSize, true);
976979

980+
// Add context into the share group
981+
mShareGroupVk->getShareContextSet()->insert(this);
982+
977983
return angle::Result::Continue;
978984
}
979985

src/libANGLE/renderer/vulkan/ContextVk.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,14 @@ class ContextVk : public ContextImpl, public vk::Context
553553
vk::AliasingMode::Allowed, image);
554554
}
555555

556+
void onImageHelperRelease(const vk::ImageHelper *image)
557+
{
558+
if (mRenderPassCommands->started())
559+
{
560+
mRenderPassCommands->onImageHelperRelease(image);
561+
}
562+
}
563+
556564
vk::CommandBuffer &getOutsideRenderPassCommandBuffer()
557565
{
558566
return mOutsideRenderPassCommands->getCommandBuffer();

src/libANGLE/renderer/vulkan/DisplayVk.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ namespace rx
1919
{
2020
class RendererVk;
2121

22+
using ShareContextSet = std::set<ContextVk *>;
23+
2224
class ShareGroupVk : public ShareGroupImpl
2325
{
2426
public:
@@ -30,13 +32,17 @@ class ShareGroupVk : public ShareGroupImpl
3032
// synchronous update to the caches.
3133
PipelineLayoutCache &getPipelineLayoutCache() { return mPipelineLayoutCache; }
3234
DescriptorSetLayoutCache &getDescriptorSetLayoutCache() { return mDescriptorSetLayoutCache; }
35+
ShareContextSet *getShareContextSet() { return &mShareContextSet; }
3336

3437
private:
3538
// ANGLE uses a PipelineLayout cache to store compatible pipeline layouts.
3639
PipelineLayoutCache mPipelineLayoutCache;
3740

3841
// DescriptorSetLayouts are also managed in a cache.
3942
DescriptorSetLayoutCache mDescriptorSetLayoutCache;
43+
44+
// The list of contexts within the share group
45+
ShareContextSet mShareContextSet;
4046
};
4147

4248
class DisplayVk : public DisplayImpl, public vk::Context

src/libANGLE/renderer/vulkan/ImageVk.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ void ImageVk::onDestroy(const egl::Display *display)
3434

3535
if (mImage != nullptr && mOwnsImage)
3636
{
37+
// TODO: We need to handle the case that EGLImage used in two context that aren't shared.
38+
// https://issuetracker.google.com/169868803
3739
mImage->releaseImage(renderer);
3840
mImage->releaseStagingBuffer(renderer);
3941
SafeDelete(mImage);

src/libANGLE/renderer/vulkan/OverlayVk.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ angle::Result OverlayVk::cullWidgets(ContextVk *contextVk)
144144
RendererVk *renderer = contextVk->getRenderer();
145145

146146
// Release old culledWidgets image
147-
mCulledWidgets.releaseImage(renderer);
147+
mCulledWidgets.releaseImageFromShareContexts(renderer, contextVk);
148148
contextVk->addGarbage(&mCulledWidgetsView);
149149

150150
// Create a buffer to contain coordinates of enabled text and graph widgets. This buffer will

src/libANGLE/renderer/vulkan/RenderbufferVk.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ void RenderbufferVk::releaseImage(ContextVk *contextVk)
227227

228228
if (mImage && mOwnsImage)
229229
{
230-
mImage->releaseImage(renderer);
230+
mImage->releaseImageFromShareContexts(renderer, contextVk);
231231
mImage->releaseStagingBuffer(renderer);
232232
}
233233
else
@@ -240,7 +240,7 @@ void RenderbufferVk::releaseImage(ContextVk *contextVk)
240240

241241
if (mMultisampledImage.valid())
242242
{
243-
mMultisampledImage.releaseImage(renderer);
243+
mMultisampledImage.releaseImageFromShareContexts(renderer, contextVk);
244244
}
245245
mMultisampledImageViews.release(renderer);
246246
}

src/libANGLE/renderer/vulkan/SurfaceVk.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ void OffscreenSurfaceVk::AttachmentImage::destroy(const egl::Display *display)
227227
{
228228
DisplayVk *displayVk = vk::GetImpl(display);
229229
RendererVk *renderer = displayVk->getRenderer();
230+
// Front end must ensure all usage has been submitted.
230231
image.releaseImage(renderer);
231232
image.releaseStagingBuffer(renderer);
232233
imageViews.release(renderer);
@@ -1065,14 +1066,14 @@ void WindowSurfaceVk::releaseSwapchainImages(ContextVk *contextVk)
10651066

10661067
if (mDepthStencilImage.valid())
10671068
{
1068-
mDepthStencilImage.releaseImage(renderer);
1069+
mDepthStencilImage.releaseImageFromShareContexts(renderer, contextVk);
10691070
mDepthStencilImage.releaseStagingBuffer(renderer);
10701071
mDepthStencilImageViews.release(renderer);
10711072
}
10721073

10731074
if (mColorImageMS.valid())
10741075
{
1075-
mColorImageMS.releaseImage(renderer);
1076+
mColorImageMS.releaseImageFromShareContexts(renderer, contextVk);
10761077
mColorImageMS.releaseStagingBuffer(renderer);
10771078
mColorImageMSViews.release(renderer);
10781079
contextVk->addGarbage(&mFramebufferMS);

src/libANGLE/renderer/vulkan/TextureVk.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2382,7 +2382,7 @@ void TextureVk::releaseImage(ContextVk *contextVk)
23822382
{
23832383
if (mOwnsImage)
23842384
{
2385-
mImage->releaseImage(renderer);
2385+
mImage->releaseImageFromShareContexts(renderer, contextVk);
23862386
}
23872387
else
23882388
{
@@ -2395,7 +2395,7 @@ void TextureVk::releaseImage(ContextVk *contextVk)
23952395
{
23962396
if (image.valid())
23972397
{
2398-
image.releaseImage(renderer);
2398+
image.releaseImageFromShareContexts(renderer, contextVk);
23992399
}
24002400
}
24012401

src/libANGLE/renderer/vulkan/android/HardwareBufferImageSiblingVkAndroid.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,8 @@ void HardwareBufferImageSiblingVkAndroid::release(RendererVk *renderer)
313313
{
314314
if (mImage != nullptr)
315315
{
316+
// TODO: We need to handle the case that EGLImage used in two context that aren't shared.
317+
// https://issuetracker.google.com/169868803
316318
mImage->releaseImage(renderer);
317319
mImage->releaseStagingBuffer(renderer);
318320
SafeDelete(mImage);

src/libANGLE/renderer/vulkan/vk_helpers.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -872,6 +872,13 @@ void CommandBufferHelper::executeBarriers(ContextVk *contextVk, PrimaryCommandBu
872872
mPipelineBarrierMask.reset();
873873
}
874874

875+
void CommandBufferHelper::onImageHelperRelease(const vk::ImageHelper *image)
876+
{
877+
ASSERT(mIsRenderPassCommandBuffer);
878+
// TODO: https://issuetracker.google.com/168953278 We will use this to finalize image layout
879+
// transition
880+
}
881+
875882
void CommandBufferHelper::beginRenderPass(const Framebuffer &framebuffer,
876883
const gl::Rectangle &renderArea,
877884
const RenderPassDesc &renderPassDesc,
@@ -3044,6 +3051,21 @@ void ImageHelper::releaseImage(RendererVk *renderer)
30443051
mImageSerial = kInvalidImageSerial;
30453052
}
30463053

3054+
void ImageHelper::releaseImageFromShareContexts(RendererVk *renderer, ContextVk *contextVk)
3055+
{
3056+
if (contextVk && mImageSerial.valid())
3057+
{
3058+
ShareContextSet &shareContextSet = *contextVk->getShareGroupVk()->getShareContextSet();
3059+
for (ContextVk *ctx : shareContextSet)
3060+
{
3061+
ctx->onImageHelperRelease(this);
3062+
}
3063+
}
3064+
3065+
renderer->collectGarbageAndReinit(&mUse, &mImage, &mDeviceMemory);
3066+
mImageSerial = kInvalidImageSerial;
3067+
}
3068+
30473069
void ImageHelper::releaseStagingBuffer(RendererVk *renderer)
30483070
{
30493071
// Remove updates that never made it to the texture.
@@ -5311,6 +5333,7 @@ void ImageHelper::SubresourceUpdate::release(RendererVk *renderer)
53115333
{
53125334
if (updateSource == UpdateSource::Image)
53135335
{
5336+
// Staging images won't be used in render pass attachments.
53145337
image.image->releaseImage(renderer);
53155338
image.image->releaseStagingBuffer(renderer);
53165339
SafeDelete(image.image);

0 commit comments

Comments
 (0)