Skip to content
Permalink
Browse files

OpenGL: Fix bug where we could end up calling glUniformMatrix without…

… a bound program. Found by GL debug callback on NV.

This adds a bit of extra checking that's only enabled in _DEBUG builds.
  • Loading branch information...
hrydgard committed Jul 28, 2018
1 parent e51ea85 commit 42f4d7b40f0c86f996c764be0386d179dbb2aca3
@@ -582,6 +582,7 @@ void FramebufferManagerCommon::NotifyRenderFramebufferSwitched(VirtualFramebuffe
DownloadFramebufferOnSwitch(prevVfb);
}
textureCache_->ForgetLastTexture();
shaderManager_->DirtyLastShader();

// Copy depth pixel value from the read framebuffer to the draw framebuffer
if (prevVfb && !g_Config.bDisableSlowFramebufEffects) {
@@ -600,6 +601,7 @@ void FramebufferManagerCommon::NotifyRenderFramebufferSwitched(VirtualFramebuffe

if (useBufferedRendering_) {
if (vfb->fbo) {
shaderManager_->DirtyLastShader();
if (gl_extensions.IsGLES) {
// Some tiled mobile GPUs benefit IMMENSELY from clearing an FBO before rendering
// to it. This broke stuff before, so now it only clears on the first use of an
@@ -839,13 +841,15 @@ void FramebufferManagerCommon::SetViewport2D(int x, int y, int w, int h) {

void FramebufferManagerCommon::CopyDisplayToOutput() {
DownloadFramebufferOnSwitch(currentRenderVfb_);
shaderManager_->DirtyLastShader();

currentRenderVfb_ = 0;

if (displayFramebufPtr_ == 0) {
DEBUG_LOG(FRAMEBUF, "Display disabled, displaying only black");
// No framebuffer to display! Clear to black.
if (useBufferedRendering_) {
shaderManager_->DirtyLastShader();
draw_->BindFramebufferAsRenderTarget(nullptr, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR });
}
gstate_c.Dirty(DIRTY_BLEND_STATE);
@@ -910,6 +914,7 @@ void FramebufferManagerCommon::CopyDisplayToOutput() {
}

if (!vfb) {
shaderManager_->DirtyLastShader();
if (useBufferedRendering_) {
// Bind and clear the backbuffer. This should be the first time during the frame that it's bound.
draw_->BindFramebufferAsRenderTarget(nullptr, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR });
@@ -925,6 +930,7 @@ void FramebufferManagerCommon::CopyDisplayToOutput() {
DEBUG_LOG(FRAMEBUF, "Found no FBO to display! displayFBPtr = %08x", displayFramebufPtr_);
// No framebuffer to display! Clear to black.
if (useBufferedRendering_) {
shaderManager_->DirtyLastShader();
// Bind and clear the backbuffer. This should be the first time during the frame that it's bound.
draw_->BindFramebufferAsRenderTarget(nullptr, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR });
}
@@ -963,6 +969,7 @@ void FramebufferManagerCommon::CopyDisplayToOutput() {
float v1 = (272.0f + offsetY) / (float)vfb->bufferHeight;

if (!usePostShader_) {
shaderManager_->DirtyLastShader();
draw_->BindFramebufferAsRenderTarget(nullptr, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR });
draw_->BindFramebufferAsTexture(vfb->fbo, 0, Draw::FB_COLOR_BIT, 0);
draw_->SetScissorRect(0, 0, pixelWidth_, pixelHeight_);
@@ -987,13 +994,13 @@ void FramebufferManagerCommon::CopyDisplayToOutput() {
}
} else if (usePostShader_ && extraFBOs_.size() == 1 && !postShaderAtOutputResolution_) {
// An additional pass, post-processing shader to the extra FBO.
shaderManager_->DirtyLastShader(); // dirty lastShader_
draw_->BindFramebufferAsRenderTarget(extraFBOs_[0], { Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE });
draw_->BindFramebufferAsTexture(vfb->fbo, 0, Draw::FB_COLOR_BIT, 0);
int fbo_w, fbo_h;
draw_->GetFramebufferDimensions(extraFBOs_[0], &fbo_w, &fbo_h);
SetViewport2D(0, 0, fbo_w, fbo_h);
draw_->SetScissorRect(0, 0, fbo_w, fbo_h);
shaderManager_->DirtyLastShader(); // dirty lastShader_
PostShaderUniforms uniforms{};
CalculatePostShaderUniforms(vfb->bufferWidth, vfb->bufferHeight, renderWidth_, renderHeight_, &uniforms);
BindPostShader(uniforms);
@@ -1031,6 +1038,7 @@ void FramebufferManagerCommon::CopyDisplayToOutput() {
DrawActiveTexture(x, y, w, h, (float)pixelWidth_, (float)pixelHeight_, u0, v0, u1, v1, uvRotation, flags);
}
} else {
shaderManager_->DirtyLastShader(); // dirty lastShader_ BEFORE drawing
draw_->BindFramebufferAsRenderTarget(nullptr, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR });
draw_->BindFramebufferAsTexture(vfb->fbo, 0, Draw::FB_COLOR_BIT, 0);
draw_->SetScissorRect(0, 0, pixelWidth_, pixelHeight_);
@@ -1040,7 +1048,6 @@ void FramebufferManagerCommon::CopyDisplayToOutput() {
std::swap(v0, v1);
DrawTextureFlags flags = (!postShaderIsUpscalingFilter_ && g_Config.iBufFilter == SCALE_LINEAR) ? DRAWTEX_LINEAR : DRAWTEX_NEAREST;

shaderManager_->DirtyLastShader(); // dirty lastShader_ BEFORE drawing
PostShaderUniforms uniforms{};
CalculatePostShaderUniforms(vfb->bufferWidth, vfb->bufferHeight, vfb->renderWidth, vfb->renderHeight, &uniforms);
BindPostShader(uniforms);
@@ -1181,6 +1188,7 @@ void FramebufferManagerCommon::ResizeFramebufFBO(VirtualFramebuffer *vfb, int w,
return;
}

shaderManager_->DirtyLastShader();
vfb->fbo = draw_->CreateFramebuffer({ vfb->renderWidth, vfb->renderHeight, 1, 1, true, (Draw::FBColorDepth)vfb->colorDepth });
if (old.fbo) {
INFO_LOG(FRAMEBUF, "Resizing FBO for %08x : %d x %d x %d", vfb->fb_address, w, h, vfb->format);
@@ -2128,6 +2136,7 @@ void FramebufferManagerCommon::DownloadFramebufferForClut(u32 fb_address, u32 lo
}

void FramebufferManagerCommon::RebindFramebuffer() {
shaderManager_->DirtyLastShader();
if (currentRenderVfb_ && currentRenderVfb_->fbo) {
draw_->BindFramebufferAsRenderTarget(currentRenderVfb_->fbo, { Draw::RPAction::KEEP, Draw::RPAction::KEEP, Draw::RPAction::KEEP });
} else {
@@ -625,7 +625,7 @@ void ShaderManagerGLES::DirtyShader() {
shaderSwitchDirtyUniforms_ = 0;
}

void ShaderManagerGLES::DirtyLastShader() { // disables vertex arrays
void ShaderManagerGLES::DirtyLastShader() {
lastShader_ = nullptr;
lastVShaderSame_ = false;
}
@@ -166,7 +166,7 @@ class ShaderManagerGLES : public ShaderManagerCommon {
void DeviceRestore(Draw::DrawContext *draw);

void DirtyShader();
void DirtyLastShader() override; // disables vertex arrays
void DirtyLastShader() override;

int GetNumVertexShaders() const { return (int)vsCache_.size(); }
int GetNumFragmentShaders() const { return (int)fsCache_.size(); }
@@ -110,6 +110,7 @@ bool FramebufferManagerGLES::NotifyStencilUpload(u32 addr, int size, bool skipZe
// Common when creating buffers, it's already 0. We're done.
return false;
}
shaderManagerGL_->DirtyLastShader();

// Let's not bother with the shader if it's just zero.
if (dstBuffer->fbo) {
@@ -473,11 +473,12 @@ void TextureCacheGLES::ApplyTextureFramebuffer(TexCacheEntry *entry, VirtualFram
depal = depalShaderCache_->GetDepalettizeShader(clutMode, framebuffer->drawnFormat);
}
if (depal) {
shaderManager_->DirtyLastShader();

const GEPaletteFormat clutFormat = gstate.getClutPaletteFormat();
GLRTexture *clutTexture = depalShaderCache_->GetClutTexture(clutFormat, clutHash_, clutBuf_);
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();

render_->SetScissor(GLRect2D{ 0, 0, (int)framebuffer->renderWidth, (int)framebuffer->renderHeight });
render_->SetViewport(GLRViewport{ 0.0f, 0.0f, (float)framebuffer->renderWidth, (float)framebuffer->renderHeight, 0.0f, 1.0f });
@@ -749,6 +749,7 @@ void GLQueueRunner::PerformRenderPass(const GLRStep &step) {
}
case GLRRenderCommand::UNIFORM4I:
{
assert(curProgram);
int loc = c.uniform4.loc ? *c.uniform4.loc : -1;
if (c.uniform4.name) {
loc = curProgram->GetUniformLoc(c.uniform4.name);
@@ -774,6 +775,7 @@ void GLQueueRunner::PerformRenderPass(const GLRStep &step) {
}
case GLRRenderCommand::UNIFORMMATRIX:
{
assert(curProgram);
int loc = c.uniform4.loc ? *c.uniform4.loc : -1;
if (c.uniform4.name) {
loc = curProgram->GetUniformLoc(c.uniform4.name);
@@ -228,6 +228,9 @@ void GLRenderManager::StopThread() {

void GLRenderManager::BindFramebufferAsRenderTarget(GLRFramebuffer *fb, GLRRenderPassAction color, GLRRenderPassAction depth, GLRRenderPassAction stencil, uint32_t clearColor, float clearDepth, uint8_t clearStencil) {
assert(insideFrame_);
#ifdef _DEBUG
curProgram_ = nullptr;
#endif
// Eliminate dupes.
if (steps_.size() && steps_.back()->render.framebuffer == fb && steps_.back()->stepType == GLRStepType::RENDER) {
if (color != GLRRenderPassAction::CLEAR && depth != GLRRenderPassAction::CLEAR && stencil != GLRRenderPassAction::CLEAR) {
@@ -360,6 +363,10 @@ void GLRenderManager::CopyImageToMemorySync(GLRTexture *texture, int mipLevel, i
void GLRenderManager::BeginFrame() {
VLOG("BeginFrame");

#ifdef _DEBUG
curProgram_ = nullptr;
#endif

int curFrame = GetCurFrame();
FrameData &frameData = frameData_[curFrame];

@@ -551,6 +551,9 @@ class GLRenderManager {
_dbg_assert_(G3D, program != nullptr);
data.program.program = program;
curRenderStep_->commands.push_back(data);
#ifdef _DEBUG
curProgram_ = program;
#endif
}

void BindPixelPackBuffer(GLRBuffer *buffer) { // Want to support an offset but can't in ES 2.0. We supply an offset when binding the buffers instead.
@@ -604,6 +607,9 @@ class GLRenderManager {

void SetUniformI(GLint *loc, int count, const int *udata) {
_dbg_assert_(G3D, curRenderStep_ && curRenderStep_->stepType == GLRStepType::RENDER);
#ifdef _DEBUG
assert(curProgram_);
#endif
GLRRenderData data{ GLRRenderCommand::UNIFORM4I };
data.uniform4.loc = loc;
data.uniform4.count = count;
@@ -613,6 +619,9 @@ class GLRenderManager {

void SetUniformI1(GLint *loc, int udata) {
_dbg_assert_(G3D, curRenderStep_ && curRenderStep_->stepType == GLRStepType::RENDER);
#ifdef _DEBUG
assert(curProgram_);
#endif
GLRRenderData data{ GLRRenderCommand::UNIFORM4I };
data.uniform4.loc = loc;
data.uniform4.count = 1;
@@ -622,6 +631,9 @@ class GLRenderManager {

void SetUniformF(GLint *loc, int count, const float *udata) {
_dbg_assert_(G3D, curRenderStep_ && curRenderStep_->stepType == GLRStepType::RENDER);
#ifdef _DEBUG
assert(curProgram_);
#endif
GLRRenderData data{ GLRRenderCommand::UNIFORM4F };
data.uniform4.loc = loc;
data.uniform4.count = count;
@@ -631,6 +643,9 @@ class GLRenderManager {

void SetUniformF1(GLint *loc, const float udata) {
_dbg_assert_(G3D, curRenderStep_ && curRenderStep_->stepType == GLRStepType::RENDER);
#ifdef _DEBUG
assert(curProgram_);
#endif
GLRRenderData data{ GLRRenderCommand::UNIFORM4F };
data.uniform4.loc = loc;
data.uniform4.count = 1;
@@ -640,6 +655,9 @@ class GLRenderManager {

void SetUniformF(const char *name, int count, const float *udata) {
_dbg_assert_(G3D, curRenderStep_ && curRenderStep_->stepType == GLRStepType::RENDER);
#ifdef _DEBUG
assert(curProgram_);
#endif
GLRRenderData data{ GLRRenderCommand::UNIFORM4F };
data.uniform4.name = name;
data.uniform4.count = count;
@@ -649,6 +667,9 @@ class GLRenderManager {

void SetUniformM4x4(GLint *loc, const float *udata) {
_dbg_assert_(G3D, curRenderStep_ && curRenderStep_->stepType == GLRStepType::RENDER);
#ifdef _DEBUG
assert(curProgram_);
#endif
GLRRenderData data{ GLRRenderCommand::UNIFORMMATRIX };
data.uniformMatrix4.loc = loc;
memcpy(data.uniformMatrix4.m, udata, sizeof(float) * 16);
@@ -657,6 +678,9 @@ class GLRenderManager {

void SetUniformM4x4(const char *name, const float *udata) {
_dbg_assert_(G3D, curRenderStep_ && curRenderStep_->stepType == GLRStepType::RENDER);
#ifdef _DEBUG
assert(curProgram_);
#endif
GLRRenderData data{ GLRRenderCommand::UNIFORMMATRIX };
data.uniformMatrix4.name = name;
memcpy(data.uniformMatrix4.m, udata, sizeof(float) * 16);
@@ -937,4 +961,8 @@ class GLRenderManager {

int targetWidth_ = 0;
int targetHeight_ = 0;

#ifdef _DEBUG
GLRProgram *curProgram_ = nullptr;
#endif
};

0 comments on commit 42f4d7b

Please sign in to comment.
You can’t perform that action at this time.