From bd3d97b17b641bcb174d188049fc0ce581a0ff16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 20 Aug 2022 07:59:11 +0200 Subject: [PATCH 1/3] Fix bug in framebuffer matching where the wrong format was checked for depth matching. Fixes #15867 --- GPU/Common/TextureCacheCommon.cpp | 25 +++++++++++++------------ GPU/GeConstants.cpp | 1 + 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/GPU/Common/TextureCacheCommon.cpp b/GPU/Common/TextureCacheCommon.cpp index 00f3822fbc..774aa8116d 100644 --- a/GPU/Common/TextureCacheCommon.cpp +++ b/GPU/Common/TextureCacheCommon.cpp @@ -868,6 +868,7 @@ bool TextureCacheCommon::MatchFramebuffer( uint32_t fb_address = channel == RASTER_DEPTH ? framebuffer->z_address : framebuffer->fb_address; uint32_t fb_stride = channel == RASTER_DEPTH ? framebuffer->z_stride : framebuffer->fb_stride; + GEBufferFormat fb_format = channel == RASTER_DEPTH ? GE_FORMAT_DEPTH16 : framebuffer->format; if (channel == RASTER_DEPTH && framebuffer->z_address == framebuffer->fb_address) { // Try to avoid silly matches to somewhat malformed buffers. @@ -913,14 +914,14 @@ bool TextureCacheCommon::MatchFramebuffer( } // NOTE: This check is okay because the first texture formats are the same as the buffer formats. if (IsTextureFormatBufferCompatible(entry.format)) { - if (TextureFormatMatchesBufferFormat(entry.format, framebuffer->format) || (framebuffer->usageFlags & FB_USAGE_BLUE_TO_ALPHA)) { + if (TextureFormatMatchesBufferFormat(entry.format, fb_format) || (framebuffer->usageFlags & FB_USAGE_BLUE_TO_ALPHA)) { return true; - } else if (IsTextureFormat16Bit(entry.format) && IsBufferFormat16Bit(framebuffer->format)) { - WARN_LOG_ONCE(diffFormat1, G3D, "Texturing from framebuffer with reinterpretable format: %s != %s", GeTextureFormatToString(entry.format), GeBufferFormatToString(framebuffer->format)); + } else if (IsTextureFormat16Bit(entry.format) && IsBufferFormat16Bit(fb_format) && channel == RASTER_COLOR) { + WARN_LOG_ONCE(diffFormat1, G3D, "Texturing from framebuffer with reinterpretable format: %s != %s", GeTextureFormatToString(entry.format), GeBufferFormatToString(fb_format)); *matchInfo = FramebufferMatchInfo{ 0, 0, true, TextureFormatToBufferFormat(entry.format) }; return true; } else { - WARN_LOG_ONCE(diffFormat2, G3D, "Texturing from framebuffer with incompatible formats %s != %s", GeTextureFormatToString(entry.format), GeBufferFormatToString(framebuffer->format)); + WARN_LOG_ONCE(diffFormat2, G3D, "Not texturing from framebuffer with incompatible formats %s != %s", GeTextureFormatToString(entry.format), GeBufferFormatToString(fb_format)); return false; } } else { @@ -935,9 +936,9 @@ bool TextureCacheCommon::MatchFramebuffer( // Check works for D16 too (???) const bool matchingClutFormat = - (channel != RASTER_COLOR && entry.format == GE_TFMT_CLUT16) || - (channel == RASTER_COLOR && framebuffer->format == GE_FORMAT_8888 && entry.format == GE_TFMT_CLUT32) || - (channel == RASTER_COLOR && framebuffer->format != GE_FORMAT_8888 && entry.format == GE_TFMT_CLUT16); + (fb_format == GE_FORMAT_DEPTH16 && entry.format == GE_TFMT_CLUT16) || + (fb_format == GE_FORMAT_8888 && entry.format == GE_TFMT_CLUT32) || + (fb_format != GE_FORMAT_8888 && entry.format == GE_TFMT_CLUT16); const u32 bitOffset = (texaddr - addr) * 8; if (bitOffset != 0) { @@ -984,15 +985,15 @@ bool TextureCacheCommon::MatchFramebuffer( } return true; } else if (IsClutFormat((GETextureFormat)(entry.format)) || IsDXTFormat((GETextureFormat)(entry.format))) { - WARN_LOG_ONCE(fourEightBit, G3D, "%s format not supported when texturing from framebuffer of format %s", GeTextureFormatToString(entry.format), GeBufferFormatToString(framebuffer->format)); + WARN_LOG_ONCE(fourEightBit, G3D, "%s format not supported when texturing from framebuffer of format %s", GeTextureFormatToString(entry.format), GeBufferFormatToString(fb_format)); return false; } // This is either normal or we failed to generate a shader to depalettize - if ((int)framebuffer->format == (int)entry.format || matchingClutFormat) { - if ((int)framebuffer->format != (int)entry.format) { + if ((int)fb_format == (int)entry.format || matchingClutFormat) { + if ((int)fb_format != (int)entry.format) { WARN_LOG_ONCE(diffFormat2, G3D, "Texturing from framebuffer with different formats %s != %s at %08x", - GeTextureFormatToString(entry.format), GeBufferFormatToString(framebuffer->format), fb_address); + GeTextureFormatToString(entry.format), GeBufferFormatToString(fb_format), fb_address); return true; } else { WARN_LOG_ONCE(subarea, G3D, "Texturing from framebuffer at %08x +%dx%d", fb_address, matchInfo->xOffset, matchInfo->yOffset); @@ -1000,7 +1001,7 @@ bool TextureCacheCommon::MatchFramebuffer( } } else { WARN_LOG_ONCE(diffFormat2, G3D, "Texturing from framebuffer with incompatible format %s != %s at %08x", - GeTextureFormatToString(entry.format), GeBufferFormatToString(framebuffer->format), fb_address); + GeTextureFormatToString(entry.format), GeBufferFormatToString(fb_format), fb_address); return false; } } diff --git a/GPU/GeConstants.cpp b/GPU/GeConstants.cpp index ab627ee473..b093d96d56 100644 --- a/GPU/GeConstants.cpp +++ b/GPU/GeConstants.cpp @@ -6,6 +6,7 @@ const char *GeBufferFormatToString(GEBufferFormat fmt) { case GE_FORMAT_5551: return "5551"; case GE_FORMAT_565: return "565"; case GE_FORMAT_8888: return "8888"; + case GE_FORMAT_DEPTH16: return "DEPTH16"; default: return "N/A"; } } From 295f5f391e10a30b5884c563de6c36cf221ba94f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 18 Aug 2022 09:17:36 +0200 Subject: [PATCH 2/3] More consistent handling of lack of depth texture support. Small log improvement. --- GPU/Common/DepalettizeShaderCommon.cpp | 6 +++--- GPU/Common/TextureCacheCommon.cpp | 28 +++++++++++++++++--------- GPU/Common/TextureCacheCommon.h | 1 + GPU/D3D11/TextureCacheD3D11.cpp | 5 +++++ GPU/Directx9/TextureCacheDX9.cpp | 6 +++++- GPU/Vulkan/TextureCacheVulkan.cpp | 11 +++++++--- 6 files changed, 41 insertions(+), 16 deletions(-) diff --git a/GPU/Common/DepalettizeShaderCommon.cpp b/GPU/Common/DepalettizeShaderCommon.cpp index 0ec0b6c66c..9874e91d6e 100644 --- a/GPU/Common/DepalettizeShaderCommon.cpp +++ b/GPU/Common/DepalettizeShaderCommon.cpp @@ -44,7 +44,7 @@ static const VaryingDef varyings[1] = { { "vec2", "v_texcoord", Draw::SEM_TEXCOORD0, 0, "highp" }, }; -// Uses integer instructions available since OpenGL 3.0. Suitable for ES 3.0 as well. +// Uses integer instructions available since OpenGL 3.0, ES 3.0 (and 2.0 with extensions), and of course Vulkan and D3D11. void GenerateDepalShader300(ShaderWriter &writer, const DepalConfig &config, const ShaderLanguageDesc &lang) { const int shift = config.shift; const int mask = config.mask; @@ -128,7 +128,7 @@ void GenerateDepalShader300(ShaderWriter &writer, const DepalConfig &config, con writer.C(" vec4 outColor = ").SampleTexture2D("pal", "uv").C(";\n"); } -// FP only, to suit GL(ES) 2.0 +// FP only, to suit GL(ES) 2.0 and DX9 void GenerateDepalShaderFloat(ShaderWriter &writer, const DepalConfig &config, const ShaderLanguageDesc &lang) { char lookupMethod[128] = "index.r"; @@ -249,7 +249,7 @@ void GenerateDepalShaderFloat(ShaderWriter &writer, const DepalConfig &config, c // index_multiplier -= 0.01f / texturePixels; if (!formatOK) { - ERROR_LOG_REPORT_ONCE(depal, G3D, "%i depal unsupported: shift=%i mask=%02x offset=%d", config.pixelFormat, shift, mask, config.startPos); + ERROR_LOG_REPORT_ONCE(depal, G3D, "%s depal unsupported: shift=%d mask=%02x offset=%d", GeBufferFormatToString(config.pixelFormat), shift, mask, config.startPos); } // Offset by half a texel (plus clutBase) to turn NEAREST filtering into FLOOR. diff --git a/GPU/Common/TextureCacheCommon.cpp b/GPU/Common/TextureCacheCommon.cpp index 774aa8116d..d5de94ea28 100644 --- a/GPU/Common/TextureCacheCommon.cpp +++ b/GPU/Common/TextureCacheCommon.cpp @@ -515,6 +515,7 @@ TexCacheEntry *TextureCacheCommon::SetTexture() { nextNeedsChange_ = false; // Might need a rebuild if the hash fails, but that will be set later. nextNeedsRebuild_ = false; + failedTexture_ = false; VERBOSE_LOG(G3D, "Texture at %08x found in cache, applying", texaddr); return entry; //Done! } else { @@ -610,10 +611,9 @@ TexCacheEntry *TextureCacheCommon::SetTexture() { gstate_c.curTextureHeight = h; gstate_c.SetTextureIs3D((entry->status & TexCacheEntry::STATUS_3D) != 0); + failedTexture_ = false; nextTexture_ = entry; - if (nextFramebufferTexture_) { - nextFramebufferTexture_ = nullptr; // in case it was accidentally set somehow? - } + nextFramebufferTexture_ = nullptr; nextNeedsRehash_ = true; // We still need to rebuild, to allocate a texture. But we'll bail early. nextNeedsRebuild_ = true; @@ -633,7 +633,7 @@ std::vector TextureCacheCommon::GetFramebufferCandidates(const candidates.push_back(AttachCandidate{ match, entry, framebuffer, RASTER_COLOR, framebuffer->colorBindSeq }); } match = {}; - if (gstate_c.Supports(GPU_SUPPORTS_DEPTH_TEXTURE) && MatchFramebuffer(entry, framebuffer, texAddrOffset, RASTER_DEPTH, &match)) { + if (MatchFramebuffer(entry, framebuffer, texAddrOffset, RASTER_DEPTH, &match)) { candidates.push_back(AttachCandidate{ match, entry, framebuffer, RASTER_DEPTH, framebuffer->depthBindSeq }); } } @@ -1020,10 +1020,10 @@ void TextureCacheCommon::SetTextureFramebuffer(const AttachCandidate &candidate) _dbg_assert_msg_(framebuffer != nullptr, "Framebuffer must not be null."); framebuffer->usageFlags |= FB_USAGE_TEXTURE; - if (framebufferManager_->UseBufferedRendering()) { - // Keep the framebuffer alive. - framebuffer->last_frame_used = gpuStats.numFlips; + // Keep the framebuffer alive. + framebuffer->last_frame_used = gpuStats.numFlips; + if (framebufferManager_->UseBufferedRendering()) { // We need to force it, since we may have set it on a texture before attaching. gstate_c.curTextureWidth = framebuffer->bufferWidth; gstate_c.curTextureHeight = framebuffer->bufferHeight; @@ -1042,7 +1042,14 @@ void TextureCacheCommon::SetTextureFramebuffer(const AttachCandidate &candidate) gstate_c.SetNeedShaderTexclamp(true); } - nextFramebufferTexture_ = framebuffer; + if (candidate.channel == RASTER_DEPTH && !gstate_c.Supports(GPU_SUPPORTS_DEPTH_TEXTURE)) { + // Flag to bind a null texture if we can't support depth textures. + // Should only happen on old OpenGL. + nextFramebufferTexture_ = nullptr; + failedTexture_ = true; + } else { + nextFramebufferTexture_ = framebuffer; + } nextTexture_ = nullptr; } else { if (framebuffer->fbo) { @@ -1753,7 +1760,10 @@ void TextureCacheCommon::ApplyTexture() { if (!entry) { // Maybe we bound a framebuffer? InvalidateLastTexture(); - if (nextFramebufferTexture_) { + if (failedTexture_) { + // Backends should handle this by binding a black texture with 0 alpha. + BindTexture(nullptr); + } else if (nextFramebufferTexture_) { bool depth = Memory::IsDepthTexVRAMAddress(gstate.getTextureAddress(0)); // ApplyTextureFrameBuffer is responsible for setting SetTextureFullAlpha. ApplyTextureFramebuffer(nextFramebufferTexture_, gstate.getTextureFormat(), depth ? RASTER_DEPTH : RASTER_COLOR); diff --git a/GPU/Common/TextureCacheCommon.h b/GPU/Common/TextureCacheCommon.h index 5c95eb3b87..dbfbb316e4 100644 --- a/GPU/Common/TextureCacheCommon.h +++ b/GPU/Common/TextureCacheCommon.h @@ -433,6 +433,7 @@ protected: SimpleBuf tmpTexBufRearrange_; TexCacheEntry *nextTexture_ = nullptr; + bool failedTexture_ = false; VirtualFramebuffer *nextFramebufferTexture_ = nullptr; u32 clutHash_ = 0; diff --git a/GPU/D3D11/TextureCacheD3D11.cpp b/GPU/D3D11/TextureCacheD3D11.cpp index 8479a3dcb1..55917a7c24 100644 --- a/GPU/D3D11/TextureCacheD3D11.cpp +++ b/GPU/D3D11/TextureCacheD3D11.cpp @@ -236,6 +236,11 @@ void TextureCacheD3D11::UpdateCurrentClut(GEPaletteFormat clutFormat, u32 clutBa } void TextureCacheD3D11::BindTexture(TexCacheEntry *entry) { + if (!entry) { + ID3D11ShaderResourceView *textureView = nullptr; + context_->PSSetShaderResources(0, 1, &textureView); + return; + } ID3D11ShaderResourceView *textureView = DxView(entry); if (textureView != lastBoundTexture) { context_->PSSetShaderResources(0, 1, &textureView); diff --git a/GPU/Directx9/TextureCacheDX9.cpp b/GPU/Directx9/TextureCacheDX9.cpp index 1394183f1b..ea6b47e2e9 100644 --- a/GPU/Directx9/TextureCacheDX9.cpp +++ b/GPU/Directx9/TextureCacheDX9.cpp @@ -204,6 +204,10 @@ void TextureCacheDX9::UpdateCurrentClut(GEPaletteFormat clutFormat, u32 clutBase } void TextureCacheDX9::BindTexture(TexCacheEntry *entry) { + if (!entry) { + device_->SetTexture(0, nullptr); + return; + } LPDIRECT3DBASETEXTURE9 texture = DxTex(entry); if (texture != lastBoundTexture) { device_->SetTexture(0, texture); @@ -215,7 +219,7 @@ void TextureCacheDX9::BindTexture(TexCacheEntry *entry) { } void TextureCacheDX9::Unbind() { - device_->SetTexture(0, NULL); + device_->SetTexture(0, nullptr); InvalidateLastTexture(); } diff --git a/GPU/Vulkan/TextureCacheVulkan.cpp b/GPU/Vulkan/TextureCacheVulkan.cpp index eb6ab9507a..51ca567183 100644 --- a/GPU/Vulkan/TextureCacheVulkan.cpp +++ b/GPU/Vulkan/TextureCacheVulkan.cpp @@ -388,14 +388,19 @@ void TextureCacheVulkan::UpdateCurrentClut(GEPaletteFormat clutFormat, u32 clutB } void TextureCacheVulkan::BindTexture(TexCacheEntry *entry) { - _assert_(entry); - _assert_(entry->vkTex); + if (!entry) { + imageView_ = VK_NULL_HANDLE; + curSampler_ = VK_NULL_HANDLE; + return; + } + _dbg_assert_(entry->vkTex); entry->vkTex->Touch(); - imageView_ = entry->vkTex->GetImageView(); + int maxLevel = (entry->status & TexCacheEntry::STATUS_NO_MIPS) ? 0 : entry->maxLevel; SamplerCacheKey samplerKey = GetSamplingParams(maxLevel, entry); curSampler_ = samplerCache_.GetOrCreateSampler(samplerKey); + imageView_ = entry->vkTex->GetImageView(); drawEngine_->SetDepalTexture(VK_NULL_HANDLE); gstate_c.SetUseShaderDepal(false); } From 68480c548d33677ee31f188f3f0d52b41e1d0405 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Fri, 19 Aug 2022 00:02:22 +0200 Subject: [PATCH 3/3] Texture bind channel fix --- GPU/Common/TextureCacheCommon.cpp | 6 ++++-- GPU/Common/TextureCacheCommon.h | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/GPU/Common/TextureCacheCommon.cpp b/GPU/Common/TextureCacheCommon.cpp index d5de94ea28..7f1c86feb3 100644 --- a/GPU/Common/TextureCacheCommon.cpp +++ b/GPU/Common/TextureCacheCommon.cpp @@ -1023,6 +1023,8 @@ void TextureCacheCommon::SetTextureFramebuffer(const AttachCandidate &candidate) // Keep the framebuffer alive. framebuffer->last_frame_used = gpuStats.numFlips; + nextFramebufferTextureChannel_ = RASTER_COLOR; + if (framebufferManager_->UseBufferedRendering()) { // We need to force it, since we may have set it on a texture before attaching. gstate_c.curTextureWidth = framebuffer->bufferWidth; @@ -1049,6 +1051,7 @@ void TextureCacheCommon::SetTextureFramebuffer(const AttachCandidate &candidate) failedTexture_ = true; } else { nextFramebufferTexture_ = framebuffer; + nextFramebufferTextureChannel_ = candidate.channel; } nextTexture_ = nullptr; } else { @@ -1764,9 +1767,8 @@ void TextureCacheCommon::ApplyTexture() { // Backends should handle this by binding a black texture with 0 alpha. BindTexture(nullptr); } else if (nextFramebufferTexture_) { - bool depth = Memory::IsDepthTexVRAMAddress(gstate.getTextureAddress(0)); // ApplyTextureFrameBuffer is responsible for setting SetTextureFullAlpha. - ApplyTextureFramebuffer(nextFramebufferTexture_, gstate.getTextureFormat(), depth ? RASTER_DEPTH : RASTER_COLOR); + ApplyTextureFramebuffer(nextFramebufferTexture_, gstate.getTextureFormat(), nextFramebufferTextureChannel_); nextFramebufferTexture_ = nullptr; } diff --git a/GPU/Common/TextureCacheCommon.h b/GPU/Common/TextureCacheCommon.h index dbfbb316e4..a8b48878e2 100644 --- a/GPU/Common/TextureCacheCommon.h +++ b/GPU/Common/TextureCacheCommon.h @@ -435,6 +435,7 @@ protected: TexCacheEntry *nextTexture_ = nullptr; bool failedTexture_ = false; VirtualFramebuffer *nextFramebufferTexture_ = nullptr; + RasterChannel nextFramebufferTextureChannel_ = RASTER_COLOR; u32 clutHash_ = 0;