Replaced raw pointers to RefCountObject to BindingPointer.
This fixes the ANGLE crashes in Chrome when using canvas 2D.
The issue was this:
Renderbuffer *mColorbufferProxy = new RenderBuffer(...); // Reference count is zero.
BindingPointer<RenderBuffer> tempRef;
tempRef.set(mColorbufferProxy); // Reference count is one.
tempRef.set(NULL); // Reference count is zero and object is destroyed, leaving mColorbufferProxy dangling.
I also initially suspected the problem was that FBOs are not treated as shared and the implementation of shared FBOs is still in the patch. I believe GLES2 supports shared FBOs.
My reading of the GLES2 spec is that when a shared object is deleted, it loses its id but retains its state if left bound elsewhere. I added that to RefCountObject.
Review URL: http://codereview.appspot.com/2120045
git-svn-id: https://angleproject.googlecode.com/svn/trunk@417 736b8ea6-26fd-11df-bfd4-992fa37f6226
diff --git a/src/libGLESv2/Context.cpp b/src/libGLESv2/Context.cpp
index 7b692a2..b004a4a 100644
--- a/src/libGLESv2/Context.cpp
+++ b/src/libGLESv2/Context.cpp
@@ -122,11 +122,8 @@
// In order that access to these initial textures not be lost, they are treated as texture
// objects all of whose names are 0.
- mTexture2DZero = new Texture2D(0);
- mTextureCubeMapZero = new TextureCubeMap(0);
-
- mColorbufferZero = NULL;
- mDepthStencilbufferZero = NULL;
+ mTexture2DZero.set(new Texture2D(0));
+ mTextureCubeMapZero.set(new TextureCubeMap(0));
mState.activeSampler = 0;
bindArrayBuffer(0);
@@ -137,11 +134,6 @@
bindDrawFramebuffer(0);
bindRenderbuffer(0);
- for (int type = 0; type < SAMPLER_TYPE_COUNT; type++)
- {
- mIncompleteTextures[type] = NULL;
- }
-
mState.currentProgram = 0;
mState.packAlignment = 4;
@@ -179,11 +171,6 @@
mState.currentProgram = 0;
}
- while (!mFramebufferMap.empty())
- {
- deleteFramebuffer(mFramebufferMap.begin()->first);
- }
-
while (!mFenceMap.empty())
{
deleteFence(mFenceMap.begin()->first);
@@ -205,7 +192,7 @@
for (int type = 0; type < SAMPLER_TYPE_COUNT; type++)
{
- delete mIncompleteTextures[type];
+ mIncompleteTextures[type].set(NULL);
}
for (int i = 0; i < MAX_VERTEX_ATTRIBS; i++)
@@ -218,9 +205,12 @@
mState.texture2D.set(NULL);
mState.textureCubeMap.set(NULL);
mState.renderbuffer.set(NULL);
+ mState.readFramebuffer.set(NULL);
+ mState.drawFramebuffer.set(NULL);
- delete mTexture2DZero;
- delete mTextureCubeMapZero;
+ mTexture2DZero.set(NULL);
+ mTextureCubeMapZero.set(NULL);
+ mFramebufferZero.set(NULL);
delete mBufferBackEnd;
delete mVertexDataManager;
@@ -723,12 +713,12 @@
GLuint Context::getReadFramebufferHandle() const
{
- return mState.readFramebuffer;
+ return mState.readFramebuffer.id();
}
GLuint Context::getDrawFramebufferHandle() const
{
- return mState.drawFramebuffer;
+ return mState.drawFramebuffer.id();
}
GLuint Context::getRenderbufferHandle() const
@@ -818,19 +808,9 @@
return mResourceManager->createRenderbuffer();
}
-// Returns an unused framebuffer name
GLuint Context::createFramebuffer()
{
- unsigned int handle = 1;
-
- while (mFramebufferMap.find(handle) != mFramebufferMap.end())
- {
- handle++;
- }
-
- mFramebufferMap[handle] = NULL;
-
- return handle;
+ return mResourceManager->createFramebuffer();
}
GLuint Context::createFence()
@@ -889,15 +869,12 @@
void Context::deleteFramebuffer(GLuint framebuffer)
{
- FramebufferMap::iterator framebufferObject = mFramebufferMap.find(framebuffer);
-
- if (framebufferObject != mFramebufferMap.end())
+ if (mResourceManager->getFramebuffer(framebuffer))
{
detachFramebuffer(framebuffer);
-
- delete framebufferObject->second;
- mFramebufferMap.erase(framebufferObject);
}
+
+ mResourceManager->deleteFramebuffer(framebuffer);
}
void Context::deleteFence(GLuint fence)
@@ -936,14 +913,14 @@
return mResourceManager->getRenderbuffer(handle);
}
-Framebuffer *Context::getReadFramebuffer()
+Framebuffer *Context::getFramebuffer(GLuint handle)
{
- return getFramebuffer(mState.readFramebuffer);
-}
+ if (handle == 0)
+ {
+ return mFramebufferZero.get();
+ }
-Framebuffer *Context::getDrawFramebuffer()
-{
- return getFramebuffer(mState.drawFramebuffer);
+ return mResourceManager->getFramebuffer(handle);
}
void Context::bindArrayBuffer(unsigned int buffer)
@@ -980,22 +957,16 @@
void Context::bindReadFramebuffer(GLuint framebuffer)
{
- if (!getFramebuffer(framebuffer))
- {
- mFramebufferMap[framebuffer] = new Framebuffer();
- }
-
- mState.readFramebuffer = framebuffer;
+ mResourceManager->checkFramebufferAllocation(framebuffer);
+
+ mState.readFramebuffer.set(getFramebuffer(framebuffer));
}
void Context::bindDrawFramebuffer(GLuint framebuffer)
{
- if (!getFramebuffer(framebuffer))
- {
- mFramebufferMap[framebuffer] = new Framebuffer();
- }
-
- mState.drawFramebuffer = framebuffer;
+ mResourceManager->checkFramebufferAllocation(framebuffer);
+
+ mState.drawFramebuffer.set(getFramebuffer(framebuffer));
}
void Context::bindRenderbuffer(GLuint renderbuffer)
@@ -1029,8 +1000,17 @@
void Context::setFramebufferZero(Framebuffer *buffer)
{
- delete mFramebufferMap[0];
- mFramebufferMap[0] = buffer;
+ if (mState.readFramebuffer.get() == mFramebufferZero.get())
+ {
+ mState.readFramebuffer.set(buffer);
+ }
+
+ if (mState.drawFramebuffer.get() == mFramebufferZero.get())
+ {
+ mState.drawFramebuffer.set(buffer);
+ }
+
+ mFramebufferZero.set(buffer);
}
void Context::setRenderbufferStorage(RenderbufferStorage *renderbuffer)
@@ -1039,20 +1019,6 @@
renderbufferObject->setStorage(renderbuffer);
}
-Framebuffer *Context::getFramebuffer(unsigned int handle)
-{
- FramebufferMap::iterator framebuffer = mFramebufferMap.find(handle);
-
- if (framebuffer == mFramebufferMap.end())
- {
- return NULL;
- }
- else
- {
- return framebuffer->second;
- }
-}
-
Fence *Context::getFence(unsigned int handle)
{
FenceMap::iterator fence = mFenceMap.find(handle);
@@ -1086,7 +1052,7 @@
{
if (mState.texture2D.id() == 0) // Special case: 0 refers to different initial textures based on the target
{
- return mTexture2DZero;
+ return mTexture2DZero.get();
}
return static_cast<Texture2D*>(mState.texture2D.get());
@@ -1096,7 +1062,7 @@
{
if (mState.textureCubeMap.id() == 0) // Special case: 0 refers to different initial textures based on the target
{
- return mTextureCubeMapZero;
+ return mTextureCubeMapZero.get();
}
return static_cast<TextureCubeMap*>(mState.textureCubeMap.get());
@@ -1111,14 +1077,24 @@
switch (type)
{
default: UNREACHABLE();
- case SAMPLER_2D: return mTexture2DZero;
- case SAMPLER_CUBE: return mTextureCubeMapZero;
+ case SAMPLER_2D: return mTexture2DZero.get();
+ case SAMPLER_CUBE: return mTextureCubeMapZero.get();
}
}
return mState.samplerTexture[type][sampler].get();
}
+Framebuffer *Context::getReadFramebuffer()
+{
+ return mState.readFramebuffer.get();
+}
+
+Framebuffer *Context::getDrawFramebuffer()
+{
+ return mState.drawFramebuffer.get();
+}
+
bool Context::getBooleanv(GLenum pname, GLboolean *params)
{
switch (pname)
@@ -1214,8 +1190,8 @@
case GL_ARRAY_BUFFER_BINDING: *params = mState.arrayBuffer.id(); break;
case GL_ELEMENT_ARRAY_BUFFER_BINDING: *params = mState.elementArrayBuffer.id(); break;
//case GL_FRAMEBUFFER_BINDING: // now equivalent to GL_DRAW_FRAMEBUFFER_BINDING_ANGLE
- case GL_DRAW_FRAMEBUFFER_BINDING_ANGLE: *params = mState.drawFramebuffer; break;
- case GL_READ_FRAMEBUFFER_BINDING_ANGLE: *params = mState.readFramebuffer; break;
+ case GL_DRAW_FRAMEBUFFER_BINDING_ANGLE: *params = mState.drawFramebuffer.id(); break;
+ case GL_READ_FRAMEBUFFER_BINDING_ANGLE: *params = mState.readFramebuffer.id(); break;
case GL_RENDERBUFFER_BINDING: *params = mState.renderbuffer.id(); break;
case GL_CURRENT_PROGRAM: *params = mState.currentProgram; break;
case GL_PACK_ALIGNMENT: *params = mState.packAlignment; break;
@@ -2998,12 +2974,12 @@
// If a framebuffer that is currently bound to the target FRAMEBUFFER is deleted, it is as though
// BindFramebuffer had been executed with the target of FRAMEBUFFER and framebuffer of zero.
- if (mState.readFramebuffer == framebuffer)
+ if (mState.readFramebuffer.id() == framebuffer)
{
bindReadFramebuffer(0);
}
- if (mState.drawFramebuffer == framebuffer)
+ if (mState.drawFramebuffer.id() == framebuffer)
{
bindDrawFramebuffer(0);
}
@@ -3041,7 +3017,7 @@
Texture *Context::getIncompleteTexture(SamplerType type)
{
- Texture *t = mIncompleteTextures[type];
+ Texture *t = mIncompleteTextures[type].get();
if (t == NULL)
{
@@ -3077,7 +3053,7 @@
break;
}
- mIncompleteTextures[type] = t;
+ mIncompleteTextures[type].set(t);
}
return t;
diff --git a/src/libGLESv2/Context.h b/src/libGLESv2/Context.h
index 253fa6a..e1a0c63 100644
--- a/src/libGLESv2/Context.h
+++ b/src/libGLESv2/Context.h
@@ -184,8 +184,8 @@
BindingPointer<Buffer> elementArrayBuffer;
BindingPointer<Texture> texture2D;
BindingPointer<Texture> textureCubeMap;
- GLuint readFramebuffer;
- GLuint drawFramebuffer;
+ BindingPointer<Framebuffer> readFramebuffer;
+ BindingPointer<Framebuffer> drawFramebuffer;
BindingPointer<Renderbuffer> renderbuffer;
GLuint currentProgram;
@@ -422,14 +422,10 @@
State mState;
- Texture2D *mTexture2DZero;
- TextureCubeMap *mTextureCubeMapZero;
-
- Colorbuffer *mColorbufferZero;
- DepthStencilbuffer *mDepthStencilbufferZero;
-
- typedef std::map<GLuint, Framebuffer*> FramebufferMap;
- FramebufferMap mFramebufferMap;
+ BindingPointer<Texture2D> mTexture2DZero;
+ BindingPointer<TextureCubeMap> mTextureCubeMapZero;
+
+ BindingPointer<Framebuffer> mFramebufferZero;
typedef std::map<GLuint, Fence*> FenceMap;
FenceMap mFenceMap;
@@ -443,7 +439,7 @@
Blit *mBlit;
- Texture *mIncompleteTextures[SAMPLER_TYPE_COUNT];
+ BindingPointer<Texture> mIncompleteTextures[SAMPLER_TYPE_COUNT];
// Recorded errors
bool mInvalidEnum;
diff --git a/src/libGLESv2/Framebuffer.cpp b/src/libGLESv2/Framebuffer.cpp
index 6e45340..d00b4da 100644
--- a/src/libGLESv2/Framebuffer.cpp
+++ b/src/libGLESv2/Framebuffer.cpp
@@ -17,7 +17,7 @@
namespace gl
{
-Framebuffer::Framebuffer()
+Framebuffer::Framebuffer(GLuint id) : RefCountObject(id)
{
mColorbufferType = GL_NONE;
mDepthbufferType = GL_NONE;
@@ -429,6 +429,7 @@
}
DefaultFramebuffer::DefaultFramebuffer(Colorbuffer *color, DepthStencilbuffer *depthStencil)
+ : Framebuffer(0)
{
mColorbufferType = GL_RENDERBUFFER;
mDepthbufferType = GL_RENDERBUFFER;
diff --git a/src/libGLESv2/Framebuffer.h b/src/libGLESv2/Framebuffer.h
index 0995145..57437d2 100644
--- a/src/libGLESv2/Framebuffer.h
+++ b/src/libGLESv2/Framebuffer.h
@@ -25,10 +25,10 @@
class Stencilbuffer;
class DepthStencilbuffer;
-class Framebuffer
+class Framebuffer : public RefCountObject
{
public:
- Framebuffer();
+ Framebuffer(GLuint id);
virtual ~Framebuffer();
diff --git a/src/libGLESv2/RefCountObject.cpp b/src/libGLESv2/RefCountObject.cpp
index e3fd36e..26c164b 100644
--- a/src/libGLESv2/RefCountObject.cpp
+++ b/src/libGLESv2/RefCountObject.cpp
@@ -9,6 +9,8 @@
// that need to be reference counted for correct cross-context deletion.
// (Concretely, textures, buffers and renderbuffers.)
+#include "main.h"
+
#include "RefCountObject.h"
namespace gl
@@ -18,6 +20,7 @@
{
mId = id;
mRefCount = 0;
+ mIsDeleted = false;
}
RefCountObject::~RefCountObject()
@@ -39,6 +42,22 @@
}
}
+GLuint RefCountObject::id() const
+{
+ if (mIsDeleted)
+ {
+ return error(GL_INVALID_OPERATION, 0);
+ }
+
+ return mId;
+}
+
+void RefCountObject::markAsDeleted()
+{
+ mId = 0;
+ mIsDeleted = true;
+}
+
void RefCountObjectBindingPointer::set(RefCountObject *newObject)
{
// addRef first in case newObject == mObject and this is the last reference to it.
diff --git a/src/libGLESv2/RefCountObject.h b/src/libGLESv2/RefCountObject.h
index a149350..c678df4 100644
--- a/src/libGLESv2/RefCountObject.h
+++ b/src/libGLESv2/RefCountObject.h
@@ -31,10 +31,13 @@
virtual void addRef() const;
virtual void release() const;
- GLuint id() const { return mId; }
+ GLuint id() const;
+
+ void markAsDeleted();
private:
GLuint mId;
+ bool mIsDeleted;
mutable std::size_t mRefCount;
};
diff --git a/src/libGLESv2/ResourceManager.cpp b/src/libGLESv2/ResourceManager.cpp
index 12a86c1..5abd756 100644
--- a/src/libGLESv2/ResourceManager.cpp
+++ b/src/libGLESv2/ResourceManager.cpp
@@ -12,6 +12,7 @@
#include "libGLESv2/Buffer.h"
#include "libGLESv2/Program.h"
#include "libGLESv2/RenderBuffer.h"
+#include "libGLESv2/FrameBuffer.h"
#include "libGLESv2/Shader.h"
#include "libGLESv2/Texture.h"
@@ -146,13 +147,32 @@
return handle;
}
+// Returns an unused framebuffer name
+GLuint ResourceManager::createFramebuffer()
+{
+ unsigned int handle = 1;
+
+ while (mFramebufferMap.find(handle) != mFramebufferMap.end())
+ {
+ handle++;
+ }
+
+ mFramebufferMap[handle] = NULL;
+
+ return handle;
+}
+
void ResourceManager::deleteBuffer(GLuint buffer)
{
BufferMap::iterator bufferObject = mBufferMap.find(buffer);
if (bufferObject != mBufferMap.end())
{
- if (bufferObject->second) bufferObject->second->release();
+ if (bufferObject->second)
+ {
+ bufferObject->second->markAsDeleted();
+ bufferObject->second->release();
+ }
mBufferMap.erase(bufferObject);
}
}
@@ -199,7 +219,11 @@
if (textureObject != mTextureMap.end())
{
- if (textureObject->second) textureObject->second->release();
+ if (textureObject->second)
+ {
+ textureObject->second->markAsDeleted();
+ textureObject->second->release();
+ }
mTextureMap.erase(textureObject);
}
}
@@ -210,11 +234,30 @@
if (renderbufferObject != mRenderbufferMap.end())
{
- if (renderbufferObject->second) renderbufferObject->second->release();
+ if (renderbufferObject->second)
+ {
+ renderbufferObject->second->markAsDeleted();
+ renderbufferObject->second->release();
+ }
mRenderbufferMap.erase(renderbufferObject);
}
}
+void ResourceManager::deleteFramebuffer(GLuint framebuffer)
+{
+ FramebufferMap::iterator framebufferObject = mFramebufferMap.find(framebuffer);
+
+ if (framebufferObject != mFramebufferMap.end())
+ {
+ if (framebufferObject->second)
+ {
+ framebufferObject->second->markAsDeleted();
+ framebufferObject->second->release();
+ }
+ mFramebufferMap.erase(framebufferObject);
+ }
+}
+
Buffer *ResourceManager::getBuffer(unsigned int handle)
{
BufferMap::iterator buffer = mBufferMap.find(handle);
@@ -287,6 +330,20 @@
}
}
+Framebuffer *ResourceManager::getFramebuffer(unsigned int handle)
+{
+ FramebufferMap::iterator framebuffer = mFramebufferMap.find(handle);
+
+ if (framebuffer == mFramebufferMap.end())
+ {
+ return NULL;
+ }
+ else
+ {
+ return framebuffer->second;
+ }
+}
+
void ResourceManager::setRenderbuffer(GLuint handle, Renderbuffer *buffer)
{
mRenderbufferMap[handle] = buffer;
@@ -337,4 +394,14 @@
}
}
+void ResourceManager::checkFramebufferAllocation(GLuint framebuffer)
+{
+ if (framebuffer != 0 && !getFramebuffer(framebuffer))
+ {
+ Framebuffer *framebufferObject = new Framebuffer(framebuffer);
+ mFramebufferMap[framebuffer] = framebufferObject;
+ framebufferObject->addRef();
+ }
+}
+
}
diff --git a/src/libGLESv2/ResourceManager.h b/src/libGLESv2/ResourceManager.h
index 346e51f..97f82b7 100644
--- a/src/libGLESv2/ResourceManager.h
+++ b/src/libGLESv2/ResourceManager.h
@@ -24,6 +24,7 @@
class Program;
class Texture;
class Renderbuffer;
+class Framebuffer;
enum SamplerType
{
@@ -47,25 +48,29 @@
GLuint createProgram();
GLuint createTexture();
GLuint createRenderbuffer();
+ GLuint createFramebuffer();
void deleteBuffer(GLuint buffer);
void deleteShader(GLuint shader);
void deleteProgram(GLuint program);
void deleteTexture(GLuint texture);
void deleteRenderbuffer(GLuint renderbuffer);
+ void deleteFramebuffer(GLuint framebuffer);
Buffer *getBuffer(GLuint handle);
Shader *getShader(GLuint handle);
Program *getProgram(GLuint handle);
Texture *getTexture(GLuint handle);
Renderbuffer *getRenderbuffer(GLuint handle);
+ Framebuffer *getFramebuffer(GLuint handle);
void setRenderbuffer(GLuint handle, Renderbuffer *renderbuffer);
void checkBufferAllocation(unsigned int buffer);
void checkTextureAllocation(GLuint texture, SamplerType type);
void checkRenderbufferAllocation(GLuint renderbuffer);
-
+ void checkFramebufferAllocation(GLuint framebuffer);
+
private:
DISALLOW_COPY_AND_ASSIGN(ResourceManager);
@@ -85,6 +90,9 @@
typedef std::map<GLuint, Renderbuffer*> RenderbufferMap;
RenderbufferMap mRenderbufferMap;
+
+ typedef std::map<GLuint, Framebuffer*> FramebufferMap;
+ FramebufferMap mFramebufferMap;
};
}
diff --git a/src/libGLESv2/Texture.h b/src/libGLESv2/Texture.h
index 9a6bf84..4653b36 100644
--- a/src/libGLESv2/Texture.h
+++ b/src/libGLESv2/Texture.h
@@ -18,6 +18,7 @@
#include <d3d9.h>
#include "libGLESv2/Renderbuffer.h"
+#include "libGLESv2/RefCountObject.h"
#include "libGLESv2/utilities.h"
#include "common/debug.h"
@@ -239,7 +240,7 @@
IDirect3DTexture9 *mTexture;
- Renderbuffer *mColorbufferProxy;
+ BindingPointer<Renderbuffer> mColorbufferProxy;
};
class TextureCubeMap : public Texture
@@ -299,7 +300,7 @@
IDirect3DCubeTexture9 *mTexture;
- Renderbuffer *mFaceProxies[6];
+ BindingPointer<Renderbuffer> mFaceProxies[6];
};
}