Skip to content
Permalink
Browse files
Merge pull request #8827 from stenzek/adreno-more-like-brokenreno
FramebufferManager: Fix invalid readback of EFB D24S8 depth
  • Loading branch information
stenzek committed May 28, 2020
2 parents 53aff81 + ad37395 commit b4e0633
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 38 deletions.
@@ -190,11 +190,18 @@ bool FramebufferManager::CreateEFBFramebuffer()
m_efb_resolve_color_texture = g_renderer->CreateTexture(
TextureConfig(efb_color_texture_config.width, efb_color_texture_config.height, 1,
efb_color_texture_config.layers, 1, efb_color_texture_config.format, 0));
if (!m_efb_resolve_color_texture)
return false;
}

// We also need one to convert the D24S8 to R32F if that is being used (Adreno).
if (g_ActiveConfig.MultisamplingEnabled() || GetEFBDepthFormat() != AbstractTextureFormat::R32F)
{
m_efb_depth_resolve_texture = g_renderer->CreateTexture(
TextureConfig(efb_depth_texture_config.width, efb_depth_texture_config.height, 1,
efb_depth_texture_config.layers, 1, GetEFBDepthCopyFormat(),
AbstractTextureFlag_RenderTarget));
if (!m_efb_resolve_color_texture || !m_efb_depth_resolve_texture)
if (!m_efb_depth_resolve_texture)
return false;

m_efb_depth_resolve_framebuffer =
@@ -248,10 +255,14 @@ AbstractTexture* FramebufferManager::ResolveEFBColorTexture(const MathUtil::Rect
return m_efb_resolve_color_texture.get();
}

AbstractTexture* FramebufferManager::ResolveEFBDepthTexture(const MathUtil::Rectangle<int>& region)
AbstractTexture* FramebufferManager::ResolveEFBDepthTexture(const MathUtil::Rectangle<int>& region,
bool force_r32f)
{
if (!IsEFBMultisampled())
if (!IsEFBMultisampled() || !force_r32f ||

This comment has been minimized.

Copy link
@Miksel12

Miksel12 Sep 20, 2020

Contributor

This PR introduced some issues(https://bugs.dolphin-emu.org/issues/12265), removing !force_r32f fixes it but I guess that breaks readback of EFB D24S8. So I think the logic behind this is incorrect.

m_efb_depth_texture->GetFormat() == AbstractTextureFormat::R32F)
{
return m_efb_depth_texture.get();
}

// It's not valid to resolve an out-of-range rectangle.
MathUtil::Rectangle<int> clamped_region = region;
@@ -900,47 +911,18 @@ void FramebufferManager::DoSaveState(PointerWrap& p)
// This won't be bit-exact when loading, which could cause interesting rendering side-effects for
// a frame. But whatever, MSAA doesn't exactly behave that well anyway.
AbstractTexture* color_texture = ResolveEFBColorTexture(m_efb_color_texture->GetRect());
AbstractTexture* depth_texture = ResolveEFBDepthTexture(m_efb_depth_texture->GetRect());
AbstractTexture* depth_texture = ResolveEFBDepthTexture(m_efb_depth_texture->GetRect(), true);

// We don't want to save these as rendertarget textures, just the data itself when deserializing.
const TextureConfig color_texture_config(color_texture->GetWidth(), color_texture->GetHeight(),
color_texture->GetLevels(), color_texture->GetLayers(),
1, GetEFBColorFormat(), 0);
g_texture_cache->SerializeTexture(color_texture, color_texture_config, p);

if (AbstractTexture::IsCompatibleDepthAndColorFormats(m_efb_depth_texture->GetFormat(),
GetEFBDepthCopyFormat()))
{
const TextureConfig depth_texture_config(depth_texture->GetWidth(), depth_texture->GetHeight(),
depth_texture->GetLevels(), depth_texture->GetLayers(),
1, GetEFBDepthCopyFormat(), 0);
g_texture_cache->SerializeTexture(depth_texture, depth_texture_config, p);
}
else
{
// If the EFB is backed by a D24S8 texture, we first have to convert it to R32F.
const TextureConfig temp_texture_config(
depth_texture->GetWidth(), depth_texture->GetHeight(), depth_texture->GetLevels(),
depth_texture->GetLayers(), 1, GetEFBDepthCopyFormat(), AbstractTextureFlag_RenderTarget);
std::unique_ptr<AbstractTexture> temp_texture = g_renderer->CreateTexture(temp_texture_config);
std::unique_ptr<AbstractFramebuffer> temp_fb =
g_renderer->CreateFramebuffer(temp_texture.get(), nullptr);
if (temp_texture && temp_fb)
{
g_renderer->ScaleTexture(temp_fb.get(), temp_texture->GetRect(), depth_texture,
depth_texture->GetRect());

const TextureConfig depth_texture_config(
depth_texture->GetWidth(), depth_texture->GetHeight(), depth_texture->GetLevels(),
depth_texture->GetLayers(), 1, temp_texture->GetFormat(), 0);
g_texture_cache->SerializeTexture(depth_texture, depth_texture_config, p);
}
else
{
PanicAlert("Failed to create temp texture for depth saving");
g_texture_cache->SerializeTexture(color_texture, color_texture_config, p);
}
}
const TextureConfig depth_texture_config(depth_texture->GetWidth(), depth_texture->GetHeight(),
depth_texture->GetLevels(), depth_texture->GetLayers(),
1, GetEFBDepthCopyFormat(), 0);
g_texture_cache->SerializeTexture(depth_texture, depth_texture_config, p);
}

void FramebufferManager::DoLoadState(PointerWrap& p)
@@ -74,7 +74,8 @@ class FramebufferManager final

// Resolve color/depth textures to a non-msaa texture, and return it.
AbstractTexture* ResolveEFBColorTexture(const MathUtil::Rectangle<int>& region);
AbstractTexture* ResolveEFBDepthTexture(const MathUtil::Rectangle<int>& region);
AbstractTexture* ResolveEFBDepthTexture(const MathUtil::Rectangle<int>& region,
bool force_r32f = false);

// Reinterpret pixel format of EFB color texture.
// Assumes no render pass is currently in progress.

0 comments on commit b4e0633

Please sign in to comment.