Commit 6b120b9f by Jamie Madill

Add checks for FBO attachment layer.

We would allow the app to attach layers that were out-of-bounds. Fix this by checking against the underlying resource dimensions. Also rework the code a bit to clean up the texture size query, which is available from the ImageDesc. BUG=angleproject:869 Change-Id: I984f1db16daea6ca650d795884d8ec2cb8f05ebb Reviewed-on: https://chromium-review.googlesource.com/313991 Tryjob-Request: Jamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org> Tested-by: 's avatarJamie Madill <jmadill@chromium.org>
parent ee322723
...@@ -360,7 +360,8 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const ...@@ -360,7 +360,8 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
{ {
if (colorAttachment.isAttached()) if (colorAttachment.isAttached())
{ {
if (colorAttachment.getWidth() == 0 || colorAttachment.getHeight() == 0) const Extents &size = colorAttachment.getSize();
if (size.width == 0 || size.height == 0)
{ {
return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT; return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
} }
...@@ -379,6 +380,11 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const ...@@ -379,6 +380,11 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
{ {
return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT; return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
} }
if (colorAttachment.layer() >= size.depth)
{
return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
}
} }
else if (colorAttachment.type() == GL_RENDERBUFFER) else if (colorAttachment.type() == GL_RENDERBUFFER)
{ {
...@@ -419,7 +425,8 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const ...@@ -419,7 +425,8 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
const FramebufferAttachment &depthAttachment = mData.mDepthAttachment; const FramebufferAttachment &depthAttachment = mData.mDepthAttachment;
if (depthAttachment.isAttached()) if (depthAttachment.isAttached())
{ {
if (depthAttachment.getWidth() == 0 || depthAttachment.getHeight() == 0) const Extents &size = depthAttachment.getSize();
if (size.width == 0 || size.height == 0)
{ {
return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT; return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
} }
...@@ -467,7 +474,8 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const ...@@ -467,7 +474,8 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
const FramebufferAttachment &stencilAttachment = mData.mStencilAttachment; const FramebufferAttachment &stencilAttachment = mData.mStencilAttachment;
if (stencilAttachment.isAttached()) if (stencilAttachment.isAttached())
{ {
if (stencilAttachment.getWidth() == 0 || stencilAttachment.getHeight() == 0) const Extents &size = stencilAttachment.getSize();
if (size.width == 0 || size.height == 0)
{ {
return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT; return GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
} }
......
...@@ -112,9 +112,10 @@ class FramebufferAttachment final ...@@ -112,9 +112,10 @@ class FramebufferAttachment final
GLint mipLevel() const; GLint mipLevel() const;
GLint layer() const; GLint layer() const;
GLsizei getWidth() const; // The size of the underlying resource the attachment points to. The 'depth' value will
GLsizei getHeight() const; // correspond to a 3D texture depth or the layer count of a 2D array texture. For Surfaces and
Extents getSize() const { return Extents(getWidth(), getHeight(), 1); } // Renderbuffers, it will always be 1.
Extents getSize() const;
GLenum getInternalFormat() const; GLenum getInternalFormat() const;
GLsizei getSamples() const; GLsizei getSamples() const;
GLenum type() const { return mType; } GLenum type() const { return mType; }
...@@ -150,8 +151,7 @@ class FramebufferAttachmentObject ...@@ -150,8 +151,7 @@ class FramebufferAttachmentObject
FramebufferAttachmentObject() {} FramebufferAttachmentObject() {}
virtual ~FramebufferAttachmentObject() {} virtual ~FramebufferAttachmentObject() {}
virtual GLsizei getAttachmentWidth(const FramebufferAttachment::Target &target) const = 0; virtual Extents getAttachmentSize(const FramebufferAttachment::Target &target) const = 0;
virtual GLsizei getAttachmentHeight(const FramebufferAttachment::Target &target) const = 0;
virtual GLenum getAttachmentInternalFormat(const FramebufferAttachment::Target &target) const = 0; virtual GLenum getAttachmentInternalFormat(const FramebufferAttachment::Target &target) const = 0;
virtual GLsizei getAttachmentSamples(const FramebufferAttachment::Target &target) const = 0; virtual GLsizei getAttachmentSamples(const FramebufferAttachment::Target &target) const = 0;
...@@ -166,14 +166,9 @@ class FramebufferAttachmentObject ...@@ -166,14 +166,9 @@ class FramebufferAttachmentObject
virtual rx::FramebufferAttachmentObjectImpl *getAttachmentImpl() const = 0; virtual rx::FramebufferAttachmentObjectImpl *getAttachmentImpl() const = 0;
}; };
inline GLsizei FramebufferAttachment::getWidth() const inline Extents FramebufferAttachment::getSize() const
{ {
return mResource->getAttachmentWidth(mTarget); return mResource->getAttachmentSize(mTarget);
}
inline GLsizei FramebufferAttachment::getHeight() const
{
return mResource->getAttachmentHeight(mTarget);
} }
inline GLenum FramebufferAttachment::getInternalFormat() const inline GLenum FramebufferAttachment::getInternalFormat() const
......
...@@ -165,4 +165,9 @@ GLuint Renderbuffer::getId() const ...@@ -165,4 +165,9 @@ GLuint Renderbuffer::getId() const
{ {
return id(); return id();
} }
Extents Renderbuffer::getAttachmentSize(const FramebufferAttachment::Target & /*target*/) const
{
return Extents(mWidth, mHeight, 1);
}
} }
...@@ -50,8 +50,7 @@ class Renderbuffer : public egl::ImageSibling, public gl::FramebufferAttachmentO ...@@ -50,8 +50,7 @@ class Renderbuffer : public egl::ImageSibling, public gl::FramebufferAttachmentO
GLuint getStencilSize() const; GLuint getStencilSize() const;
// FramebufferAttachmentObject Impl // FramebufferAttachmentObject Impl
GLsizei getAttachmentWidth(const FramebufferAttachment::Target &/*target*/) const override { return getWidth(); } Extents getAttachmentSize(const FramebufferAttachment::Target &target) const override;
GLsizei getAttachmentHeight(const FramebufferAttachment::Target &/*target*/) const override { return getHeight(); }
GLenum getAttachmentInternalFormat(const FramebufferAttachment::Target &/*target*/) const override { return getInternalFormat(); } GLenum getAttachmentInternalFormat(const FramebufferAttachment::Target &/*target*/) const override { return getInternalFormat(); }
GLsizei getAttachmentSamples(const FramebufferAttachment::Target &/*target*/) const override { return getSamples(); } GLsizei getAttachmentSamples(const FramebufferAttachment::Target &/*target*/) const override { return getSamples(); }
......
...@@ -203,6 +203,11 @@ void Surface::releaseTexImageFromTexture() ...@@ -203,6 +203,11 @@ void Surface::releaseTexImageFromTexture()
mTexture.set(nullptr); mTexture.set(nullptr);
} }
gl::Extents Surface::getAttachmentSize(const gl::FramebufferAttachment::Target & /*target*/) const
{
return gl::Extents(getWidth(), getHeight(), 1);
}
GLenum Surface::getAttachmentInternalFormat(const gl::FramebufferAttachment::Target &target) const GLenum Surface::getAttachmentInternalFormat(const gl::FramebufferAttachment::Target &target) const
{ {
const egl::Config *config = getConfig(); const egl::Config *config = getConfig();
......
...@@ -70,8 +70,7 @@ class Surface final : public gl::FramebufferAttachmentObject ...@@ -70,8 +70,7 @@ class Surface final : public gl::FramebufferAttachmentObject
EGLint isFixedSize() const; EGLint isFixedSize() const;
// FramebufferAttachmentObject implementation // FramebufferAttachmentObject implementation
GLsizei getAttachmentWidth(const gl::FramebufferAttachment::Target &/*target*/) const override { return getWidth(); } gl::Extents getAttachmentSize(const gl::FramebufferAttachment::Target &target) const override;
GLsizei getAttachmentHeight(const gl::FramebufferAttachment::Target &/*target*/) const override { return getHeight(); }
GLenum getAttachmentInternalFormat(const gl::FramebufferAttachment::Target &target) const override; GLenum getAttachmentInternalFormat(const gl::FramebufferAttachment::Target &target) const override;
GLsizei getAttachmentSamples(const gl::FramebufferAttachment::Target &target) const override; GLsizei getAttachmentSamples(const gl::FramebufferAttachment::Target &target) const override;
......
...@@ -839,16 +839,9 @@ Texture::SamplerCompletenessCache::SamplerCompletenessCache() ...@@ -839,16 +839,9 @@ Texture::SamplerCompletenessCache::SamplerCompletenessCache()
{ {
} }
GLsizei Texture::getAttachmentWidth(const gl::FramebufferAttachment::Target &target) const Extents Texture::getAttachmentSize(const gl::FramebufferAttachment::Target &target) const
{ {
return static_cast<GLsizei>( return getImageDesc(target.textureIndex().type, target.textureIndex().mipIndex).size;
getWidth(target.textureIndex().type, target.textureIndex().mipIndex));
}
GLsizei Texture::getAttachmentHeight(const gl::FramebufferAttachment::Target &target) const
{
return static_cast<GLsizei>(
getHeight(target.textureIndex().type, target.textureIndex().mipIndex));
} }
GLenum Texture::getAttachmentInternalFormat(const gl::FramebufferAttachment::Target &target) const GLenum Texture::getAttachmentInternalFormat(const gl::FramebufferAttachment::Target &target) const
......
...@@ -166,8 +166,7 @@ class Texture final : public egl::ImageSibling, public gl::FramebufferAttachment ...@@ -166,8 +166,7 @@ class Texture final : public egl::ImageSibling, public gl::FramebufferAttachment
const rx::TextureImpl *getImplementation() const { return mTexture; } const rx::TextureImpl *getImplementation() const { return mTexture; }
// FramebufferAttachmentObject implementation // FramebufferAttachmentObject implementation
GLsizei getAttachmentWidth(const FramebufferAttachment::Target &target) const override; Extents getAttachmentSize(const FramebufferAttachment::Target &target) const override;
GLsizei getAttachmentHeight(const FramebufferAttachment::Target &target) const override;
GLenum getAttachmentInternalFormat(const FramebufferAttachment::Target &target) const override; GLenum getAttachmentInternalFormat(const FramebufferAttachment::Target &target) const override;
GLsizei getAttachmentSamples(const FramebufferAttachment::Target &target) const override; GLsizei getAttachmentSamples(const FramebufferAttachment::Target &target) const override;
......
...@@ -229,21 +229,15 @@ gl::Error Clear11::clearFramebuffer(const ClearParameters &clearParams, const gl ...@@ -229,21 +229,15 @@ gl::Error Clear11::clearFramebuffer(const ClearParameters &clearParams, const gl
const gl::FramebufferAttachment *colorAttachment = fboData.getFirstColorAttachment(); const gl::FramebufferAttachment *colorAttachment = fboData.getFirstColorAttachment();
if (colorAttachment != nullptr) if (colorAttachment != nullptr)
{ {
framebufferSize.width = colorAttachment->getWidth(); framebufferSize = colorAttachment->getSize();
framebufferSize.height = colorAttachment->getHeight();
framebufferSize.depth = 1;
} }
else if (depthAttachment != nullptr) else if (depthAttachment != nullptr)
{ {
framebufferSize.width = depthAttachment->getWidth(); framebufferSize = depthAttachment->getSize();
framebufferSize.height = depthAttachment->getHeight();
framebufferSize.depth = 1;
} }
else if (stencilAttachment != nullptr) else if (stencilAttachment != nullptr)
{ {
framebufferSize.width = stencilAttachment->getWidth(); framebufferSize = stencilAttachment->getSize();
framebufferSize.height = stencilAttachment->getHeight();
framebufferSize.depth = 1;
} }
else else
{ {
......
...@@ -1635,7 +1635,8 @@ gl::Error Renderer11::applyRenderTarget(const gl::Framebuffer *framebuffer) ...@@ -1635,7 +1635,8 @@ gl::Error Renderer11::applyRenderTarget(const gl::Framebuffer *framebuffer)
// check for zero-sized default framebuffer, which is a special case. // check for zero-sized default framebuffer, which is a special case.
// in this case we do not wish to modify any state and just silently return false. // in this case we do not wish to modify any state and just silently return false.
// this will not report any gl error but will cause the calling method to return. // this will not report any gl error but will cause the calling method to return.
if (colorbuffer->getWidth() == 0 || colorbuffer->getHeight() == 0) const gl::Extents &size = colorbuffer->getSize();
if (size.width == 0 || size.height == 0)
{ {
return gl::Error(GL_NO_ERROR); return gl::Error(GL_NO_ERROR);
} }
......
...@@ -1277,15 +1277,14 @@ gl::Error Renderer9::getNullColorbuffer(const gl::FramebufferAttachment *depthbu ...@@ -1277,15 +1277,14 @@ gl::Error Renderer9::getNullColorbuffer(const gl::FramebufferAttachment *depthbu
{ {
ASSERT(depthbuffer); ASSERT(depthbuffer);
GLsizei width = depthbuffer->getWidth(); const gl::Extents &size = depthbuffer->getSize();
GLsizei height = depthbuffer->getHeight();
// search cached nullcolorbuffers // search cached nullcolorbuffers
for (int i = 0; i < NUM_NULL_COLORBUFFER_CACHE_ENTRIES; i++) for (int i = 0; i < NUM_NULL_COLORBUFFER_CACHE_ENTRIES; i++)
{ {
if (mNullColorbufferCache[i].buffer != NULL && if (mNullColorbufferCache[i].buffer != NULL &&
mNullColorbufferCache[i].width == width && mNullColorbufferCache[i].width == size.width &&
mNullColorbufferCache[i].height == height) mNullColorbufferCache[i].height == size.height)
{ {
mNullColorbufferCache[i].lruCount = ++mMaxNullColorbufferLRU; mNullColorbufferCache[i].lruCount = ++mMaxNullColorbufferLRU;
*outColorBuffer = mNullColorbufferCache[i].buffer; *outColorBuffer = mNullColorbufferCache[i].buffer;
...@@ -1294,7 +1293,7 @@ gl::Error Renderer9::getNullColorbuffer(const gl::FramebufferAttachment *depthbu ...@@ -1294,7 +1293,7 @@ gl::Error Renderer9::getNullColorbuffer(const gl::FramebufferAttachment *depthbu
} }
gl::Renderbuffer *nullRenderbuffer = new gl::Renderbuffer(createRenderbuffer(), 0); gl::Renderbuffer *nullRenderbuffer = new gl::Renderbuffer(createRenderbuffer(), 0);
gl::Error error = nullRenderbuffer->setStorage(GL_NONE, width, height); gl::Error error = nullRenderbuffer->setStorage(GL_NONE, size.width, size.height);
if (error.isError()) if (error.isError())
{ {
SafeDelete(nullRenderbuffer); SafeDelete(nullRenderbuffer);
...@@ -1316,8 +1315,8 @@ gl::Error Renderer9::getNullColorbuffer(const gl::FramebufferAttachment *depthbu ...@@ -1316,8 +1315,8 @@ gl::Error Renderer9::getNullColorbuffer(const gl::FramebufferAttachment *depthbu
delete oldest->buffer; delete oldest->buffer;
oldest->buffer = nullbuffer; oldest->buffer = nullbuffer;
oldest->lruCount = ++mMaxNullColorbufferLRU; oldest->lruCount = ++mMaxNullColorbufferLRU;
oldest->width = width; oldest->width = size.width;
oldest->height = height; oldest->height = size.height;
*outColorBuffer = nullbuffer; *outColorBuffer = nullbuffer;
return gl::Error(GL_NO_ERROR); return gl::Error(GL_NO_ERROR);
......
...@@ -543,9 +543,11 @@ static bool IsPartialBlit(gl::Context *context, const gl::FramebufferAttachment ...@@ -543,9 +543,11 @@ static bool IsPartialBlit(gl::Context *context, const gl::FramebufferAttachment
GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1,
GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1) GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1)
{ {
if (srcX0 != 0 || srcY0 != 0 || dstX0 != 0 || dstY0 != 0 || const Extents &writeSize = writeBuffer->getSize();
dstX1 != writeBuffer->getWidth() || dstY1 != writeBuffer->getHeight() || const Extents &readSize = readBuffer->getSize();
srcX1 != readBuffer->getWidth() || srcY1 != readBuffer->getHeight())
if (srcX0 != 0 || srcY0 != 0 || dstX0 != 0 || dstY0 != 0 || dstX1 != writeSize.width ||
dstY1 != writeSize.height || srcX1 != readSize.width || srcY1 != readSize.height)
{ {
return true; return true;
} }
...@@ -553,9 +555,8 @@ static bool IsPartialBlit(gl::Context *context, const gl::FramebufferAttachment ...@@ -553,9 +555,8 @@ static bool IsPartialBlit(gl::Context *context, const gl::FramebufferAttachment
{ {
const Rectangle &scissor = context->getState().getScissor(); const Rectangle &scissor = context->getState().getScissor();
return scissor.x > 0 || scissor.y > 0 || return scissor.x > 0 || scissor.y > 0 || scissor.width < writeSize.width ||
scissor.width < writeBuffer->getWidth() || scissor.height < writeSize.height;
scissor.height < writeBuffer->getHeight();
} }
else else
{ {
......
...@@ -538,10 +538,6 @@ ...@@ -538,10 +538,6 @@
1097 WIN : dEQP-GLES3.functional.fbo.completeness.renderable.texture.depth.rg8_snorm = FAIL 1097 WIN : dEQP-GLES3.functional.fbo.completeness.renderable.texture.depth.rg8_snorm = FAIL
1097 WIN : dEQP-GLES3.functional.fbo.completeness.renderable.texture.depth.rgb8_snorm = FAIL 1097 WIN : dEQP-GLES3.functional.fbo.completeness.renderable.texture.depth.rgb8_snorm = FAIL
1097 WIN : dEQP-GLES3.functional.fbo.completeness.renderable.texture.depth.rgba8_snorm = FAIL 1097 WIN : dEQP-GLES3.functional.fbo.completeness.renderable.texture.depth.rgba8_snorm = FAIL
1097 WIN : dEQP-GLES3.functional.fbo.completeness.layer.2darr_1_3 = FAIL
1097 WIN : dEQP-GLES3.functional.fbo.completeness.layer.2darr_4_15 = FAIL
1097 WIN : dEQP-GLES3.functional.fbo.completeness.layer.3d_1_15 = FAIL
1097 WIN : dEQP-GLES3.functional.fbo.completeness.layer.3d_4_15 = FAIL
1097 WIN : dEQP-GLES3.functional.fbo.render.resize.tex2d_rgba16f_stencil_rbo_stencil_index8 = FAIL 1097 WIN : dEQP-GLES3.functional.fbo.render.resize.tex2d_rgba16f_stencil_rbo_stencil_index8 = FAIL
1097 WIN : dEQP-GLES3.functional.fbo.render.resize.tex2d_rgba8_stencil_rbo_stencil_index8 = FAIL 1097 WIN : dEQP-GLES3.functional.fbo.render.resize.tex2d_rgba8_stencil_rbo_stencil_index8 = FAIL
1097 WIN : dEQP-GLES3.functional.fbo.render.recreate_depth_stencil.tex2d_rgba8_stencil_rbo_stencil_index8 = FAIL 1097 WIN : dEQP-GLES3.functional.fbo.render.recreate_depth_stencil.tex2d_rgba8_stencil_rbo_stencil_index8 = FAIL
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment