Skip to content

Commit

Permalink
GPU: Prevent temp FBOs from overwriting each other.
Browse files Browse the repository at this point in the history 
Sometimes we'd use two temp FBOs in the same draw (e.g. shader blending +
depal.)  This could cause the same temp FBO to get used for two purposes,
causing weird behavior.
  • Loading branch information
@unknownbrackets
unknownbrackets committed May 6, 2018
1 parent 8959a90 commit 8d07e6d
Show file tree
Hide file tree
Showing 12 changed files with 25 additions and 35 deletions.
8 changes: 4 additions & 4 deletions GPU/Common/FramebufferCommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1832,8 +1832,8 @@ void FramebufferManagerCommon::GetCardboardSettings(CardboardSettings *cardboard
cardboardSettings->screenHeight = cardboardScreenHeight;
}

Draw::Framebuffer *FramebufferManagerCommon::GetTempFBO(u16 w, u16 h, Draw::FBColorDepth depth) {
u64 key = ((u64)depth << 32) | ((u32)w << 16) | h;
Draw::Framebuffer *FramebufferManagerCommon::GetTempFBO(TempFBO reason, u16 w, u16 h, Draw::FBColorDepth depth) {
u64 key = ((u64)reason << 48) | ((u64)depth << 32) | ((u32)w << 16) | h;
auto it = tempFBOs_.find(key);
if (it != tempFBOs_.end()) {
it->second.last_frame_used = gpuStats.numFlips;
Expand All @@ -1845,7 +1845,7 @@ Draw::Framebuffer *FramebufferManagerCommon::GetTempFBO(u16 w, u16 h, Draw::FBCo
if (!fbo)
return fbo;

const TempFBO info = { fbo, gpuStats.numFlips };
const TempFBOInfo info = { fbo, gpuStats.numFlips };
tempFBOs_[key] = info;
return fbo;
}
Expand Down Expand Up @@ -1914,7 +1914,7 @@ bool FramebufferManagerCommon::GetFramebuffer(u32 fb_address, int fb_stride, GEB
w = vfb->width * maxRes;
h = vfb->height * maxRes;

Draw::Framebuffer *tempFBO = GetTempFBO(w, h);
Draw::Framebuffer *tempFBO = GetTempFBO(TempFBO::COPY, w, h);
VirtualFramebuffer tempVfb = *vfb;
tempVfb.fbo = tempFBO;
tempVfb.bufferWidth = vfb->width;
Expand Down
13 changes: 10 additions & 3 deletions GPU/Common/FramebufferCommon.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,13 @@ enum DrawTextureFlags {
DRAWTEX_KEEP_STENCIL_ALPHA = 4,
};

enum class TempFBO {
DEPAL,
BLIT,
// For copies of framebuffers (e.g. shader blending.)
COPY,
};

inline Draw::DataFormat GEFormatToThin3D(int geFormat) {
switch (geFormat) {
case GE_FORMAT_4444:
Expand Down Expand Up @@ -290,7 +297,7 @@ class FramebufferManagerCommon {

virtual void Resized();

Draw::Framebuffer *GetTempFBO(u16 w, u16 h, Draw::FBColorDepth colorDepth = Draw::FBO_8888);
Draw::Framebuffer *GetTempFBO(TempFBO reason, u16 w, u16 h, Draw::FBColorDepth colorDepth = Draw::FBO_8888);

// Debug features
virtual bool GetFramebuffer(u32 fb_address, int fb_stride, GEBufferFormat format, GPUDebugBuffer &buffer, int maxRes);
Expand Down Expand Up @@ -400,12 +407,12 @@ class FramebufferManagerCommon {

bool needGLESRebinds_ = false;

struct TempFBO {
struct TempFBOInfo {
Draw::Framebuffer *fbo;
int last_frame_used;
};

std::map<u64, TempFBO> tempFBOs_;
std::map<u64, TempFBOInfo> tempFBOs_;

std::vector<Draw::Framebuffer *> fbosToDelete_;

Expand Down
4 changes: 2 additions & 2 deletions GPU/D3D11/FramebufferManagerD3D11.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ void FramebufferManagerD3D11::BindFramebufferAsColorTexture(int stage, VirtualFr
// Currently rendering to this framebuffer. Need to make a copy.
if (!skipCopy && framebuffer == currentRenderVfb_) {
// TODO: Maybe merge with bvfbs_? Not sure if those could be packing, and they're created at a different size.
Draw::Framebuffer *renderCopy = GetTempFBO(framebuffer->renderWidth, framebuffer->renderHeight, (Draw::FBColorDepth)framebuffer->colorDepth);
Draw::Framebuffer *renderCopy = GetTempFBO(TempFBO::COPY, framebuffer->renderWidth, framebuffer->renderHeight, (Draw::FBColorDepth)framebuffer->colorDepth);
if (renderCopy) {
VirtualFramebuffer copyInfo = *framebuffer;
copyInfo.fbo = renderCopy;
Expand Down Expand Up @@ -671,7 +671,7 @@ void FramebufferManagerD3D11::BlitFramebuffer(VirtualFramebuffer *dst, int dstX,
// Direct3D doesn't support rect -> self.
Draw::Framebuffer *srcFBO = src->fbo;
if (src == dst) {
Draw::Framebuffer *tempFBO = GetTempFBO(src->renderWidth, src->renderHeight, (Draw::FBColorDepth)src->colorDepth);
Draw::Framebuffer *tempFBO = GetTempFBO(TempFBO::BLIT, src->renderWidth, src->renderHeight, (Draw::FBColorDepth)src->colorDepth);
SimpleBlit(tempFBO, dstX1, dstY1, dstX2, dstY2, src->fbo, srcX1, srcY1, srcX2, srcY2, false);
srcFBO = tempFBO;
}
Expand Down
2 changes: 1 addition & 1 deletion GPU/D3D11/TextureCacheD3D11.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ void TextureCacheD3D11::ApplyTextureFramebuffer(TexCacheEntry *entry, VirtualFra
const GEPaletteFormat clutFormat = gstate.getClutPaletteFormat();
ID3D11ShaderResourceView *clutTexture = depalShaderCache_->GetClutTexture(clutFormat, clutHash_, clutBuf_, expand32);

Draw::Framebuffer *depalFBO = framebufferManagerD3D11_->GetTempFBO(framebuffer->renderWidth, framebuffer->renderHeight, Draw::FBO_8888);
Draw::Framebuffer *depalFBO = framebufferManagerD3D11_->GetTempFBO(TempFBO::DEPAL, framebuffer->renderWidth, framebuffer->renderHeight, Draw::FBO_8888);
shaderManager_->DirtyLastShader();
draw_->BindPipeline(nullptr);

Expand Down
15 changes: 2 additions & 13 deletions GPU/Directx9/FramebufferDX9.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,6 @@ static const D3DVERTEXELEMENT9 g_FramebufferVertexElements[] = {
if (drawPixelsTex_) {
drawPixelsTex_->Release();
}
for (auto it = tempFBOs_.begin(), end = tempFBOs_.end(); it != end; ++it) {
it->second.fbo->Release();
}
for (auto it = offscreenSurfaces_.begin(), end = offscreenSurfaces_.end(); it != end; ++it) {
it->second.surface->Release();
}
Expand Down Expand Up @@ -448,7 +445,7 @@ static const D3DVERTEXELEMENT9 g_FramebufferVertexElements[] = {
}
if (!skipCopy && currentRenderVfb_ && framebuffer->fb_address == gstate.getFrameBufRawAddress()) {
// TODO: Maybe merge with bvfbs_? Not sure if those could be packing, and they're created at a different size.
Draw::Framebuffer *renderCopy = GetTempFBO(framebuffer->renderWidth, framebuffer->renderHeight, (Draw::FBColorDepth)framebuffer->colorDepth);
Draw::Framebuffer *renderCopy = GetTempFBO(TempFBO::COPY, framebuffer->renderWidth, framebuffer->renderHeight, (Draw::FBColorDepth)framebuffer->colorDepth);
if (renderCopy) {
VirtualFramebuffer copyInfo = *framebuffer;
copyInfo.fbo = renderCopy;
Expand Down Expand Up @@ -514,7 +511,7 @@ static const D3DVERTEXELEMENT9 g_FramebufferVertexElements[] = {
// Direct3D 9 doesn't support rect -> self.
Draw::Framebuffer *srcFBO = src->fbo;
if (src == dst) {
Draw::Framebuffer *tempFBO = GetTempFBO(src->renderWidth, src->renderHeight, (Draw::FBColorDepth)src->colorDepth);
Draw::Framebuffer *tempFBO = GetTempFBO(TempFBO::BLIT, src->renderWidth, src->renderHeight, (Draw::FBColorDepth)src->colorDepth);
bool result = draw_->BlitFramebuffer(
src->fbo, srcX1, srcY1, srcX2, srcY2,
tempFBO, dstX1, dstY1, dstX2, dstY2,
Expand Down Expand Up @@ -709,11 +706,6 @@ static const D3DVERTEXELEMENT9 g_FramebufferVertexElements[] = {
}
bvfbs_.clear();

for (auto it = tempFBOs_.begin(), end = tempFBOs_.end(); it != end; ++it) {
it->second.fbo->Release();
}
tempFBOs_.clear();

for (auto it = offscreenSurfaces_.begin(), end = offscreenSurfaces_.end(); it != end; ++it) {
it->second.surface->Release();
}
Expand Down Expand Up @@ -761,9 +753,6 @@ static const D3DVERTEXELEMENT9 g_FramebufferVertexElements[] = {
if (offscreen) {
success = GetRenderTargetFramebuffer(renderTarget, offscreen, w, h, buffer);
}
if (tempFBO) {
tempFBO->Release();
}
}

return success;
Expand Down
5 changes: 0 additions & 5 deletions GPU/Directx9/FramebufferDX9.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,11 @@ class FramebufferManagerDX9 : public FramebufferManagerCommon {
ShaderManagerDX9 *shaderManagerDX9_;
DrawEngineDX9 *drawEngineD3D9_;

struct TempFBO {
Draw::Framebuffer *fbo;
int last_frame_used;
};
struct OffscreenSurface {
LPDIRECT3DSURFACE9 surface;
int last_frame_used;
};

std::map<u64, TempFBO> tempFBOs_;
std::map<u64, OffscreenSurface> offscreenSurfaces_;

#if 0
Expand Down
2 changes: 1 addition & 1 deletion GPU/Directx9/TextureCacheDX9.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ void TextureCacheDX9::ApplyTextureFramebuffer(TexCacheEntry *entry, VirtualFrame
const GEPaletteFormat clutFormat = gstate.getClutPaletteFormat();
LPDIRECT3DTEXTURE9 clutTexture = depalShaderCache_->GetClutTexture(clutFormat, clutHash_, clutBuf_);

Draw::Framebuffer *depalFBO = framebufferManagerDX9_->GetTempFBO(framebuffer->renderWidth, framebuffer->renderHeight, Draw::FBO_8888);
Draw::Framebuffer *depalFBO = framebufferManagerDX9_->GetTempFBO(TempFBO::DEPAL, framebuffer->renderWidth, framebuffer->renderHeight, Draw::FBO_8888);
draw_->BindFramebufferAsRenderTarget(depalFBO, { Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE });
shaderManager_->DirtyLastShader();

Expand Down
2 changes: 1 addition & 1 deletion GPU/GLES/FramebufferManagerGLES.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ void FramebufferManagerGLES::BindFramebufferAsColorTexture(int stage, VirtualFra
}
if (!skipCopy && currentRenderVfb_ && framebuffer->fb_address == gstate.getFrameBufRawAddress()) {
// TODO: Maybe merge with bvfbs_? Not sure if those could be packing, and they're created at a different size.
Draw::Framebuffer *renderCopy = GetTempFBO(framebuffer->renderWidth, framebuffer->renderHeight, (Draw::FBColorDepth)framebuffer->colorDepth);
Draw::Framebuffer *renderCopy = GetTempFBO(TempFBO::COPY, framebuffer->renderWidth, framebuffer->renderHeight, (Draw::FBColorDepth)framebuffer->colorDepth);
if (renderCopy) {
VirtualFramebuffer copyInfo = *framebuffer;
copyInfo.fbo = renderCopy;
Expand Down
2 changes: 1 addition & 1 deletion GPU/GLES/StencilBufferGLES.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ bool FramebufferManagerGLES::NotifyStencilUpload(u32 addr, int size, bool skipZe

Draw::Framebuffer *blitFBO = nullptr;
if (useBlit) {
blitFBO = GetTempFBO(w, h, Draw::FBO_8888);
blitFBO = GetTempFBO(TempFBO::COPY, w, h, Draw::FBO_8888);
draw_->BindFramebufferAsRenderTarget(blitFBO, { Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE });
} else if (dstBuffer->fbo) {
draw_->BindFramebufferAsRenderTarget(dstBuffer->fbo, { Draw::RPAction::KEEP, Draw::RPAction::KEEP, Draw::RPAction::CLEAR });
Expand Down
2 changes: 1 addition & 1 deletion GPU/GLES/TextureCacheGLES.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ void TextureCacheGLES::ApplyTextureFramebuffer(TexCacheEntry *entry, VirtualFram
if (depal) {
const GEPaletteFormat clutFormat = gstate.getClutPaletteFormat();
GLRTexture *clutTexture = depalShaderCache_->GetClutTexture(clutFormat, clutHash_, clutBuf_);
Draw::Framebuffer *depalFBO = framebufferManagerGL_->GetTempFBO(framebuffer->renderWidth, framebuffer->renderHeight, Draw::FBO_8888);
Draw::Framebuffer *depalFBO = framebufferManagerGL_->GetTempFBO(TempFBO::DEPAL, framebuffer->renderWidth, framebuffer->renderHeight, Draw::FBO_8888);
draw_->BindFramebufferAsRenderTarget(depalFBO, { Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE });
shaderManager_->DirtyLastShader();

Expand Down
2 changes: 1 addition & 1 deletion GPU/Vulkan/FramebufferVulkan.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ VkImageView FramebufferManagerVulkan::BindFramebufferAsColorTexture(int stage, V
// Currently rendering to this framebuffer. Need to make a copy.
if (!skipCopy && framebuffer == currentRenderVfb_) {
// TODO: Maybe merge with bvfbs_? Not sure if those could be packing, and they're created at a different size.
Draw::Framebuffer *renderCopy = GetTempFBO(framebuffer->renderWidth, framebuffer->renderHeight, (Draw::FBColorDepth)framebuffer->colorDepth);
Draw::Framebuffer *renderCopy = GetTempFBO(TempFBO::COPY, framebuffer->renderWidth, framebuffer->renderHeight, (Draw::FBColorDepth)framebuffer->colorDepth);
if (renderCopy) {
VirtualFramebuffer copyInfo = *framebuffer;
copyInfo.fbo = renderCopy;
Expand Down
3 changes: 1 addition & 2 deletions GPU/Vulkan/TextureCacheVulkan.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,7 @@ void TextureCacheVulkan::ApplyTextureFramebuffer(TexCacheEntry *entry, VirtualFr
const GEPaletteFormat clutFormat = gstate.getClutPaletteFormat();
VulkanTexture *clutTexture = depalShaderCache_->GetClutTexture(clutFormat, clutHash_, clutBuf_);

Draw::Framebuffer *depalFBO = framebufferManager_->GetTempFBO(
framebuffer->renderWidth, framebuffer->renderHeight, Draw::FBO_8888);
Draw::Framebuffer *depalFBO = framebufferManager_->GetTempFBO(TempFBO::DEPAL, framebuffer->renderWidth, framebuffer->renderHeight, Draw::FBO_8888);
draw_->BindFramebufferAsRenderTarget(depalFBO, { Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE });

Vulkan2D::Vertex verts[4] = {
Expand Down

0 comments on commit 8d07e6d

Please sign in to comment.