Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vulkan image barrier fix, image leak fix from #17534 #17536

Merged
merged 3 commits into from
May 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Common/GPU/D3D11/thin3d_d3d11.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,16 @@ class D3D11DrawContext : public DrawContext {
stencilDirty_ = true;
}

void EndFrame() override;

void Draw(int vertexCount, int offset) override;
void DrawIndexed(int vertexCount, int offset) override;
void DrawUP(const void *vdata, int vertexCount) override;
void Clear(int mask, uint32_t colorval, float depthVal, int stencilVal) override;

void BeginFrame() override;
void EndFrame() override;

int GetFrameCount() override { return frameCount_; }

std::string GetInfoString(InfoField info) const override {
switch (info) {
Expand Down Expand Up @@ -221,6 +223,7 @@ class D3D11DrawContext : public DrawContext {
int nextIndexBufferOffset_ = 0;

InvalidationCallback invalidationCallback_;
int frameCount_ = 0;

// Dynamic state
float blendFactor_[4]{};
Expand Down Expand Up @@ -423,6 +426,7 @@ void D3D11DrawContext::HandleEvent(Event ev, int width, int height, void *param1

void D3D11DrawContext::EndFrame() {
curPipeline_ = nullptr;
frameCount_++;
}

void D3D11DrawContext::SetViewport(const Viewport &viewport) {
Expand Down
3 changes: 3 additions & 0 deletions Common/GPU/D3D9/thin3d_d3d9.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,7 @@ class D3D9Context : public DrawContext {
}

void EndFrame() override;
int GetFrameCount() override { return frameCount_; }

void UpdateDynamicUniformBuffer(const void *ub, size_t size) override;

Expand Down Expand Up @@ -640,6 +641,7 @@ class D3D9Context : public DrawContext {
D3DCAPS9 d3dCaps_;
char shadeLangVersion_[64]{};
DeviceCaps caps_{};
int frameCount_ = 0;

// Bound state
AutoRef<D3D9Pipeline> curPipeline_;
Expand Down Expand Up @@ -964,6 +966,7 @@ void D3D9Context::BindNativeTexture(int index, void *nativeTexture) {

void D3D9Context::EndFrame() {
curPipeline_ = nullptr;
frameCount_++;
}

static void SemanticToD3D9UsageAndIndex(int semantic, BYTE *usage, BYTE *index) {
Expand Down
6 changes: 6 additions & 0 deletions Common/GPU/OpenGL/thin3d_gl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,10 @@ class OpenGLContext : public DrawContext {
void BeginFrame() override;
void EndFrame() override;

int GetFrameCount() override {
return frameCount_;
}

void UpdateBuffer(Buffer *buffer, const uint8_t *data, size_t offset, size_t size, UpdateBufferFlags flags) override;
void UpdateTextureLevels(Texture *texture, const uint8_t **data, TextureCallback initDataCallback, int numLevels) override;

Expand Down Expand Up @@ -492,6 +496,7 @@ class OpenGLContext : public DrawContext {
void ApplySamplers();

GLRenderManager renderManager_;
int frameCount_ = 0;

DeviceCaps caps_{};

Expand Down Expand Up @@ -795,6 +800,7 @@ void OpenGLContext::EndFrame() {
renderManager_.Finish();

Invalidate(InvalidationFlags::CACHED_RENDER_STATE);
frameCount_++;
}

void OpenGLContext::Invalidate(InvalidationFlags flags) {
Expand Down
10 changes: 8 additions & 2 deletions Common/GPU/Vulkan/VulkanDebug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,14 @@ VKAPI_ATTR VkBool32 VKAPI_CALL VulkanDebugUtilsCallback(
return false;
case 1303270965:
// Benign perf warning, image blit using GENERAL layout.
// UNASSIGNED
return false;
// TODO: Oops, turns out we filtered out a bit too much here!
// We really need that performance flag check to sort out the stuff that matters.
// Will enable it soon, but it'll take some fixing.
//
if (messageType & VK_DEBUG_UTILS_MESSAGE_TYPE_PERFORMANCE_BIT_EXT)
return false;
break;

case 606910136:
case -392708513:
case -384083808:
Expand Down
16 changes: 16 additions & 0 deletions Common/GPU/Vulkan/VulkanImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,22 @@ void VulkanTexture::EndCreate(VkCommandBuffer cmd, bool vertexTexture, VkPipelin
prevStage == VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT ? VK_ACCESS_SHADER_WRITE_BIT : VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_SHADER_READ_BIT);
}

void VulkanTexture::PrepareForTransferDst(VkCommandBuffer cmd, int levels) {
TransitionImageLayout2(cmd, image_, 0, levels, 1,
VK_IMAGE_ASPECT_COLOR_BIT,
VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT,
VK_ACCESS_SHADER_READ_BIT, VK_ACCESS_TRANSFER_WRITE_BIT);
}

void VulkanTexture::RestoreAfterTransferDst(VkCommandBuffer cmd, int levels) {
TransitionImageLayout2(cmd, image_, 0, levels, 1,
VK_IMAGE_ASPECT_COLOR_BIT,
VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL,
VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT,
VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_SHADER_READ_BIT);
}

VkImageView VulkanTexture::CreateViewForMip(int mip) {
VkImageViewCreateInfo view_info = { VK_STRUCTURE_TYPE_IMAGE_VIEW_CREATE_INFO };
view_info.image = image_;
Expand Down
4 changes: 4 additions & 0 deletions Common/GPU/Vulkan/VulkanImage.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ class VulkanTexture {
void GenerateMips(VkCommandBuffer cmd, int firstMipToGenerate, bool fromCompute);
void EndCreate(VkCommandBuffer cmd, bool vertexTexture, VkPipelineStageFlags prevStage, VkImageLayout layout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL);

// For updating levels after creation. Careful with the timelines!
void PrepareForTransferDst(VkCommandBuffer cmd, int levels);
void RestoreAfterTransferDst(VkCommandBuffer cmd, int levels);

// When loading mips from compute shaders, you need to pass VK_IMAGE_LAYOUT_GENERAL to the above function.
// In addition, ignore UploadMip and GenerateMip, and instead use GetViewForMip. Make sure to delete the returned views when used.
VkImageView CreateViewForMip(int mip);
Expand Down
10 changes: 9 additions & 1 deletion Common/GPU/Vulkan/thin3d_vulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,10 @@ class VKContext : public DrawContext {
void EndFrame() override;
void WipeQueue() override;

int GetFrameCount() override {
return frameCount_;
}

void FlushState() override {}

void ResetStats() override {
Expand Down Expand Up @@ -528,6 +532,7 @@ class VKContext : public DrawContext {
VulkanTexture *GetNullTexture();
VulkanContext *vulkan_ = nullptr;

int frameCount_ = 0;
VulkanRenderManager renderManager_;

VulkanTexture *nullTexture_ = nullptr;
Expand Down Expand Up @@ -799,8 +804,9 @@ bool VKTexture::Create(VkCommandBuffer cmd, VulkanPushPool *pushBuffer, const Te
void VKTexture::Update(VkCommandBuffer cmd, VulkanPushPool *pushBuffer, const uint8_t * const *data, TextureCallback initDataCallback, int numLevels) {
// Before we can use UpdateInternal, we need to transition the image to the same state as after CreateDirect,
// making it ready for writing.

vkTex_->PrepareForTransferDst(cmd, numLevels);
UpdateInternal(cmd, pushBuffer, data, initDataCallback, numLevels);
vkTex_->RestoreAfterTransferDst(cmd, numLevels);
}

void VKTexture::UpdateInternal(VkCommandBuffer cmd, VulkanPushPool *pushBuffer, const uint8_t * const *data, TextureCallback initDataCallback, int numLevels) {
Expand Down Expand Up @@ -1104,6 +1110,8 @@ void VKContext::EndFrame() {

// Unbind stuff, to avoid accidentally relying on it across frames (and provide some protection against forgotten unbinds of deleted things).
Invalidate(InvalidationFlags::CACHED_RENDER_STATE);

frameCount_++;
}

void VKContext::Invalidate(InvalidationFlags flags) {
Expand Down
4 changes: 4 additions & 0 deletions Common/GPU/thin3d.h
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,10 @@ class DrawContext {
// Not very elegant, but more elegant than the old passId hack.
virtual void SetInvalidationCallback(InvalidationCallback callback) = 0;

// Total amount of frames rendered. Unaffected by game pause, so more robust than gpuStats.numFlips
virtual int GetFrameCount() = 0;
virtual int GetFramesInFlight() { return 3; }

protected:
ShaderModule *vsPresets_[VS_MAX_PRESET];
ShaderModule *fsPresets_[FS_MAX_PRESET];
Expand Down
15 changes: 9 additions & 6 deletions GPU/Common/FramebufferManagerCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1402,16 +1402,19 @@ Draw::Texture *FramebufferManagerCommon::MakePixelTexture(const u8 *srcPixels, G

Draw::DataFormat texFormat = srcPixelFormat == GE_FORMAT_DEPTH16 ? depthFormat : preferredPixelsFormat_;

int frameNumber = draw_->GetFrameCount();

// Look for a matching texture we can re-use.
for (auto &iter : drawPixelsCache_) {
if (iter.frameNumber > gpuStats.numFlips - 3 || iter.tex->Width() != width || iter.tex->Height() != height || iter.tex->Format() != texFormat) {
if (iter.frameNumber >= frameNumber - 3 || iter.tex->Width() != width || iter.tex->Height() != height || iter.tex->Format() != texFormat) {
continue;
}

// OK, current one seems good, let's use it (and mark it used).
gpuStats.numDrawPixels++;
draw_->UpdateTextureLevels(iter.tex, &srcPixels, generateTexture, 1);
iter.frameNumber = gpuStats.numFlips;
// NOTE: numFlips is no good - this is called every frame when paused sometimes!
iter.frameNumber = frameNumber;
return iter.tex;
}

Expand Down Expand Up @@ -1440,9 +1443,9 @@ Draw::Texture *FramebufferManagerCommon::MakePixelTexture(const u8 *srcPixels, G
gpuStats.numDrawPixels++;
gpuStats.numTexturesDecoded++; // Separate stat for this later?

INFO_LOG(G3D, "Creating drawPixelsCache texture: %dx%d", tex->Width(), tex->Height());
// INFO_LOG(G3D, "Creating drawPixelsCache texture: %dx%d", tex->Width(), tex->Height());

DrawPixelsEntry entry{ tex, gpuStats.numFlips };
DrawPixelsEntry entry{ tex, frameNumber };
drawPixelsCache_.push_back(entry);
return tex;
}
Expand Down Expand Up @@ -1695,9 +1698,9 @@ void FramebufferManagerCommon::DecimateFBOs() {
// And DrawPixels cached textures.

for (auto it = drawPixelsCache_.begin(); it != drawPixelsCache_.end(); ) {
int age = gpuStats.numFlips - it->frameNumber;
int age = draw_->GetFrameCount() - it->frameNumber;
if (age > 10) {
INFO_LOG(G3D, "Releasing drawPixelsCache texture: %dx%d", it->tex->Width(), it->tex->Height());
// INFO_LOG(G3D, "Releasing drawPixelsCache texture: %dx%d", it->tex->Width(), it->tex->Height());
it->tex->Release();
it->tex = nullptr;
it = drawPixelsCache_.erase(it);
Expand Down