Fix corner case for render buffer reference count
The spec leaves a lot of corner cases when deleting renderbuffer. It is
automatically detached from the current framebuffer. This does not
happen to framebuffers that are not current.
We add more granular ref counts to handle the situation when it is bound
to a framebuffer that has been detached.
https://www.khronos.org/registry/OpenGL-Refpages/es3.0/html/glDeleteRenderbuffers.xhtml
Bug: 228188569
Test: run cts -m CtsSkQPTestCases -t org.skia.skqp.SkQPRunner#UnitTest_ES2BlendWithNoTexture
Change-Id: I2b63419717cc3a3e211d9e555bae3f3932f010f5
diff --git a/shared/OpenglCodecCommon/GLClientState.cpp b/shared/OpenglCodecCommon/GLClientState.cpp
index 17c789e..91f67d5 100644
--- a/shared/OpenglCodecCommon/GLClientState.cpp
+++ b/shared/OpenglCodecCommon/GLClientState.cpp
@@ -2127,10 +2127,11 @@
void GLClientState::bindRenderbuffer(GLenum target, GLuint name) {
(void)target; // Must be GL_RENDERBUFFER
- RenderbufferInfo::ScopedView view(mRboState.rboData);
- if (name != mRboState.boundRenderbuffer) {
- view.unref(mRboState.boundRenderbuffer);
+ if (name == mRboState.boundRenderbuffer) {
+ return;
}
+ RenderbufferInfo::ScopedView view(mRboState.rboData);
+ view.unref(mRboState.boundRenderbuffer);
mRboState.boundRenderbuffer = name;
@@ -2496,6 +2497,7 @@
}
void GLClientState::removeFramebuffers(GLsizei n, const GLuint* framebuffers) {
+ RenderbufferInfo::ScopedView view(mRboState.rboData);
for (size_t i = 0; i < n; i++) {
if (framebuffers[i] != 0) { // Never remove the zero fb.
if (framebuffers[i] == mFboState.boundDrawFramebuffer) {
@@ -2504,6 +2506,22 @@
if (framebuffers[i] == mFboState.boundReadFramebuffer) {
bindFramebuffer(GL_READ_FRAMEBUFFER, 0);
}
+ // Remove references to all attachments
+ auto fboProps = mFboState.fboData.find(framebuffers[i]);
+ if (fboProps != mFboState.fboData.end()) {
+ for (size_t j = 0; j < fboProps->second.colorAttachmenti_hasRbo.size(); j++) {
+ view.unref(fboProps->second.colorAttachmenti_rbos[j]);
+ }
+ if (fboProps->second.depthAttachment_hasRbo && fboProps->second.depthAttachment_rbo) {
+ view.unref(fboProps->second.depthAttachment_rbo);
+ }
+ if (fboProps->second.stencilAttachment_hasRbo && fboProps->second.stencilAttachment_rbo) {
+ view.unref(fboProps->second.stencilAttachment_rbo);
+ }
+ if (fboProps->second.depthstencilAttachment_hasRbo && fboProps->second.depthstencilAttachment_rbo) {
+ view.unref(fboProps->second.depthstencilAttachment_rbo);
+ }
+ }
mFboState.fboData.erase(framebuffers[i]);
}
}
@@ -2696,11 +2714,13 @@
boundFboProps(target).completenessDirty = true;
+ RenderbufferInfo::ScopedView view(mRboState.rboData);
if (colorAttachmentIndex != -1) {
if (boundFboProps(target).colorAttachmenti_hasRbo[colorAttachmentIndex] &&
boundFboProps(target).colorAttachmenti_rbos[colorAttachmentIndex] == renderbuffer) {
boundFboProps(target).colorAttachmenti_rbos[colorAttachmentIndex] = 0;
boundFboProps(target).colorAttachmenti_hasRbo[colorAttachmentIndex] = false;
+ view.unref(renderbuffer);
}
}
@@ -2710,6 +2730,7 @@
boundFboProps(target).depthAttachment_hasRbo) {
boundFboProps(target).depthAttachment_rbo = 0;
boundFboProps(target).depthAttachment_hasRbo = false;
+ view.unref(renderbuffer);
}
break;
case GL_STENCIL_ATTACHMENT:
@@ -2717,6 +2738,7 @@
boundFboProps(target).stencilAttachment_hasRbo) {
boundFboProps(target).stencilAttachment_rbo = 0;
boundFboProps(target).stencilAttachment_hasRbo = false;
+ view.unref(renderbuffer);
}
break;
case GL_DEPTH_STENCIL_ATTACHMENT:
@@ -2724,16 +2746,19 @@
boundFboProps(target).depthAttachment_hasRbo) {
boundFboProps(target).depthAttachment_rbo = 0;
boundFboProps(target).depthAttachment_hasRbo = false;
+ view.unref(renderbuffer);
}
if (boundFboProps(target).stencilAttachment_rbo == renderbuffer &&
boundFboProps(target).stencilAttachment_hasRbo) {
boundFboProps(target).stencilAttachment_rbo = 0;
boundFboProps(target).stencilAttachment_hasRbo = false;
+ view.unref(renderbuffer);
}
if (boundFboProps(target).depthstencilAttachment_rbo == renderbuffer &&
boundFboProps(target).depthstencilAttachment_hasRbo) {
boundFboProps(target).depthstencilAttachment_rbo = 0;
boundFboProps(target).depthstencilAttachment_hasRbo = false;
+ view.unref(renderbuffer);
}
break;
}
@@ -2748,21 +2773,34 @@
boundFboProps(target).completenessDirty = true;
+ RenderbufferInfo::ScopedView view(mRboState.rboData);
if (colorAttachmentIndex != -1) {
+ view.unref(boundFboProps(target).colorAttachmenti_rbos[colorAttachmentIndex]);
+ view.ref(renderbuffer);
boundFboProps(target).colorAttachmenti_rbos[colorAttachmentIndex] = renderbuffer;
boundFboProps(target).colorAttachmenti_hasRbo[colorAttachmentIndex] = attach;
}
switch (attachment) {
case GL_DEPTH_ATTACHMENT:
+ view.unref(boundFboProps(target).depthAttachment_rbo);
+ view.ref(renderbuffer);
boundFboProps(target).depthAttachment_rbo = renderbuffer;
boundFboProps(target).depthAttachment_hasRbo = attach;
break;
case GL_STENCIL_ATTACHMENT:
+ view.unref(boundFboProps(target).stencilAttachment_rbo);
+ view.ref(renderbuffer);
boundFboProps(target).stencilAttachment_rbo = renderbuffer;
boundFboProps(target).stencilAttachment_hasRbo = attach;
break;
case GL_DEPTH_STENCIL_ATTACHMENT:
+ view.unref(boundFboProps(target).depthAttachment_rbo);
+ view.ref(renderbuffer);
+ view.unref(boundFboProps(target).stencilAttachment_rbo);
+ view.ref(renderbuffer);
+ view.unref(boundFboProps(target).depthstencilAttachment_rbo);
+ view.ref(renderbuffer);
boundFboProps(target).depthAttachment_rbo = renderbuffer;
boundFboProps(target).depthAttachment_hasRbo = attach;
boundFboProps(target).stencilAttachment_rbo = renderbuffer;
diff --git a/shared/OpenglCodecCommon/GLClientState.h b/shared/OpenglCodecCommon/GLClientState.h
index bc65aa7..a9da8d0 100644
--- a/shared/OpenglCodecCommon/GLClientState.h
+++ b/shared/OpenglCodecCommon/GLClientState.h
@@ -100,14 +100,14 @@
bool depthstencilAttachment_hasTexObj;
std::vector<GLuint> colorAttachmenti_rbos;
- GLuint depthAttachment_rbo;
- GLuint stencilAttachment_rbo;
- GLuint depthstencilAttachment_rbo;
+ GLuint depthAttachment_rbo = 0;
+ GLuint stencilAttachment_rbo = 0;
+ GLuint depthstencilAttachment_rbo = 0;
std::vector<bool> colorAttachmenti_hasRbo;
- bool depthAttachment_hasRbo;
- bool stencilAttachment_hasRbo;
- bool depthstencilAttachment_hasRbo;
+ bool depthAttachment_hasRbo = false;
+ bool stencilAttachment_hasRbo = false;
+ bool depthstencilAttachment_hasRbo = false;
GLuint defaultWidth;
GLuint defaultHeight;
diff --git a/shared/OpenglCodecCommon/StateTrackingSupport.h b/shared/OpenglCodecCommon/StateTrackingSupport.h
index b742c13..7a009d8 100644
--- a/shared/OpenglCodecCommon/StateTrackingSupport.h
+++ b/shared/OpenglCodecCommon/StateTrackingSupport.h
@@ -169,11 +169,18 @@
return res;
}
void ref(GLuint id) {
+ if (id == 0) {
+ return;
+ }
+ if (!hasRbo(id)) addFresh(id);
RboProps* props = get(id);
if (!props) return;
++props->refcount;
}
bool unref(GLuint id) {
+ if (id == 0) {
+ return false;
+ }
RboProps* props = get(id);
if (!props) return false;
if (!props->refcount) return false;
