Commit cc86d64e by Jamie Madill

Make Framebuffer size check ES2-only.

This is an ES2-only incompleteness check. We also need to require matching dimensions in D3D11, but make this an implementation specific check. Also make all implementation specific errors 'UNSUPPORTED' since that catches all "non-ES" framebuffer restrictions. Note that we can't be conformant here in D3D11 currently, since the spec only makes an exception for mismatching formats for UNSUPPORTED, not for size checks. However, we don't have an easy solution. BUG=angleproject:1225 Change-Id: Ic80a04bce397fc12643b010c874f432033babc5d Reviewed-on: https://chromium-review.googlesource.com/313990Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Tryjob-Request: Jamie Madill <jmadill@chromium.org> Tested-by: 's avatarJamie Madill <jmadill@chromium.org>
parent 5778557f
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "libANGLE/Framebuffer.h" #include "libANGLE/Framebuffer.h"
#include "common/Optional.h"
#include "common/utilities.h" #include "common/utilities.h"
#include "libANGLE/Config.h" #include "libANGLE/Config.h"
#include "libANGLE/Context.h" #include "libANGLE/Context.h"
...@@ -124,6 +125,42 @@ const FramebufferAttachment *Framebuffer::Data::getDepthStencilAttachment() cons ...@@ -124,6 +125,42 @@ const FramebufferAttachment *Framebuffer::Data::getDepthStencilAttachment() cons
return nullptr; return nullptr;
} }
bool Framebuffer::Data::attachmentsHaveSameDimensions() const
{
Optional<Extents> attachmentSize;
auto hasMismatchedSize = [&attachmentSize](const FramebufferAttachment &attachment)
{
if (!attachment.isAttached())
{
return false;
}
if (!attachmentSize.valid())
{
attachmentSize = attachment.getSize();
return false;
}
return (attachment.getSize() != attachmentSize.value());
};
for (const auto &attachment : mColorAttachments)
{
if (hasMismatchedSize(attachment))
{
return false;
}
}
if (hasMismatchedSize(mDepthAttachment))
{
return false;
}
return !hasMismatchedSize(mStencilAttachment);
}
Framebuffer::Framebuffer(const Caps &caps, rx::ImplFactory *factory, GLuint id) Framebuffer::Framebuffer(const Caps &caps, rx::ImplFactory *factory, GLuint id)
: mData(caps), mImpl(factory->createFramebuffer(mData)), mId(id) : mData(caps), mImpl(factory->createFramebuffer(mData)), mId(id)
{ {
...@@ -315,8 +352,6 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const ...@@ -315,8 +352,6 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
return GL_FRAMEBUFFER_COMPLETE; return GL_FRAMEBUFFER_COMPLETE;
} }
int width = 0;
int height = 0;
unsigned int colorbufferSize = 0; unsigned int colorbufferSize = 0;
int samples = -1; int samples = -1;
bool missingAttachment = true; bool missingAttachment = true;
...@@ -355,12 +390,6 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const ...@@ -355,12 +390,6 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
if (!missingAttachment) if (!missingAttachment)
{ {
// all color attachments must have the same width and height
if (colorAttachment.getWidth() != width || colorAttachment.getHeight() != height)
{
return GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS;
}
// APPLE_framebuffer_multisample, which EXT_draw_buffers refers to, requires that // APPLE_framebuffer_multisample, which EXT_draw_buffers refers to, requires that
// all color attachments have the same number of samples for the FBO to be complete. // all color attachments have the same number of samples for the FBO to be complete.
if (colorAttachment.getSamples() != samples) if (colorAttachment.getSamples() != samples)
...@@ -380,8 +409,6 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const ...@@ -380,8 +409,6 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
} }
else else
{ {
width = colorAttachment.getWidth();
height = colorAttachment.getHeight();
samples = colorAttachment.getSamples(); samples = colorAttachment.getSamples();
colorbufferSize = formatInfo.pixelBytes; colorbufferSize = formatInfo.pixelBytes;
missingAttachment = false; missingAttachment = false;
...@@ -428,15 +455,9 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const ...@@ -428,15 +455,9 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
if (missingAttachment) if (missingAttachment)
{ {
width = depthAttachment.getWidth();
height = depthAttachment.getHeight();
samples = depthAttachment.getSamples(); samples = depthAttachment.getSamples();
missingAttachment = false; missingAttachment = false;
} }
else if (width != depthAttachment.getWidth() || height != depthAttachment.getHeight())
{
return GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS;
}
else if (samples != depthAttachment.getSamples()) else if (samples != depthAttachment.getSamples())
{ {
return GL_FRAMEBUFFER_INCOMPLETE_MULTISAMPLE_ANGLE; return GL_FRAMEBUFFER_INCOMPLETE_MULTISAMPLE_ANGLE;
...@@ -483,15 +504,9 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const ...@@ -483,15 +504,9 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
if (missingAttachment) if (missingAttachment)
{ {
width = stencilAttachment.getWidth();
height = stencilAttachment.getHeight();
samples = stencilAttachment.getSamples(); samples = stencilAttachment.getSamples();
missingAttachment = false; missingAttachment = false;
} }
else if (width != stencilAttachment.getWidth() || height != stencilAttachment.getHeight())
{
return GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS;
}
else if (samples != stencilAttachment.getSamples()) else if (samples != stencilAttachment.getSamples())
{ {
return GL_FRAMEBUFFER_INCOMPLETE_MULTISAMPLE_ANGLE; return GL_FRAMEBUFFER_INCOMPLETE_MULTISAMPLE_ANGLE;
...@@ -504,7 +519,19 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const ...@@ -504,7 +519,19 @@ GLenum Framebuffer::checkStatus(const gl::Data &data) const
return GL_FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT; return GL_FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT;
} }
return mImpl->checkStatus(); // In ES 2.0, all color attachments must have the same width and height.
// In ES 3.0, there is no such restriction.
if (data.clientVersion < 3 && !mData.attachmentsHaveSameDimensions())
{
return GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS;
}
if (!mImpl->checkStatus())
{
return GL_FRAMEBUFFER_UNSUPPORTED;
}
return GL_FRAMEBUFFER_COMPLETE;
} }
Error Framebuffer::discard(size_t count, const GLenum *attachments) Error Framebuffer::discard(size_t count, const GLenum *attachments)
......
...@@ -66,6 +66,8 @@ class Framebuffer ...@@ -66,6 +66,8 @@ class Framebuffer
const std::vector<GLenum> &getDrawBufferStates() const { return mDrawBufferStates; } const std::vector<GLenum> &getDrawBufferStates() const { return mDrawBufferStates; }
const std::vector<FramebufferAttachment> &getColorAttachments() const { return mColorAttachments; } const std::vector<FramebufferAttachment> &getColorAttachments() const { return mColorAttachments; }
bool attachmentsHaveSameDimensions() const;
private: private:
friend class Framebuffer; friend class Framebuffer;
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "angle_gl.h" #include "angle_gl.h"
#include "common/angleutils.h" #include "common/angleutils.h"
#include "libANGLE/angletypes.h"
#include "libANGLE/Error.h" #include "libANGLE/Error.h"
#include "libANGLE/ImageIndex.h" #include "libANGLE/ImageIndex.h"
...@@ -113,6 +114,7 @@ class FramebufferAttachment final ...@@ -113,6 +114,7 @@ class FramebufferAttachment final
GLsizei getWidth() const; GLsizei getWidth() const;
GLsizei getHeight() const; GLsizei getHeight() const;
Extents getSize() const { return Extents(getWidth(), getHeight(), 1); }
GLenum getInternalFormat() const; GLenum getInternalFormat() const;
GLsizei getSamples() const; GLsizei getSamples() const;
GLenum type() const { return mType; } GLenum type() const { return mType; }
......
...@@ -133,4 +133,13 @@ bool Box::operator!=(const Box &other) const ...@@ -133,4 +133,13 @@ bool Box::operator!=(const Box &other) const
return !(*this == other); return !(*this == other);
} }
bool operator==(const Extents &lhs, const Extents &rhs)
{
return lhs.width == rhs.width && lhs.height == rhs.height && lhs.depth == rhs.depth;
}
bool operator!=(const Extents &lhs, const Extents &rhs)
{
return !(lhs == rhs);
}
} }
...@@ -104,6 +104,9 @@ struct Extents ...@@ -104,6 +104,9 @@ struct Extents
bool empty() const { return (width * height * depth) == 0; } bool empty() const { return (width * height * depth) == 0; }
}; };
bool operator==(const Extents &lhs, const Extents &rhs);
bool operator!=(const Extents &lhs, const Extents &rhs);
struct Box struct Box
{ {
int x; int x;
......
...@@ -56,7 +56,7 @@ class FramebufferImpl : angle::NonCopyable ...@@ -56,7 +56,7 @@ class FramebufferImpl : angle::NonCopyable
virtual gl::Error blit(const gl::State &state, const gl::Rectangle &sourceArea, const gl::Rectangle &destArea, virtual gl::Error blit(const gl::State &state, const gl::Rectangle &sourceArea, const gl::Rectangle &destArea,
GLbitfield mask, GLenum filter, const gl::Framebuffer *sourceFramebuffer) = 0; GLbitfield mask, GLenum filter, const gl::Framebuffer *sourceFramebuffer) = 0;
virtual GLenum checkStatus() const = 0; virtual bool checkStatus() const = 0;
const gl::Framebuffer::Data &getData() const { return mData; } const gl::Framebuffer::Data &getData() const { return mData; }
......
...@@ -55,7 +55,7 @@ class MockFramebufferImpl : public rx::FramebufferImpl ...@@ -55,7 +55,7 @@ class MockFramebufferImpl : public rx::FramebufferImpl
GLenum, GLenum,
const gl::Framebuffer *)); const gl::Framebuffer *));
MOCK_CONST_METHOD0(checkStatus, GLenum()); MOCK_CONST_METHOD0(checkStatus, bool());
MOCK_METHOD0(destroy, void()); MOCK_METHOD0(destroy, void());
}; };
......
...@@ -302,14 +302,14 @@ gl::Error FramebufferD3D::blit(const gl::State &state, const gl::Rectangle &sour ...@@ -302,14 +302,14 @@ gl::Error FramebufferD3D::blit(const gl::State &state, const gl::Rectangle &sour
return gl::Error(GL_NO_ERROR); return gl::Error(GL_NO_ERROR);
} }
GLenum FramebufferD3D::checkStatus() const bool FramebufferD3D::checkStatus() const
{ {
// if we have both a depth and stencil buffer, they must refer to the same object // if we have both a depth and stencil buffer, they must refer to the same object
// since we only support packed_depth_stencil and not separate depth and stencil // since we only support packed_depth_stencil and not separate depth and stencil
if (mData.getDepthAttachment() != nullptr && mData.getStencilAttachment() != nullptr && if (mData.getDepthAttachment() != nullptr && mData.getStencilAttachment() != nullptr &&
mData.getDepthStencilAttachment() == nullptr) mData.getDepthStencilAttachment() == nullptr)
{ {
return GL_FRAMEBUFFER_UNSUPPORTED; return false;
} }
// D3D11 does not allow for overlapping RenderTargetViews, so ensure uniqueness // D3D11 does not allow for overlapping RenderTargetViews, so ensure uniqueness
...@@ -326,13 +326,19 @@ GLenum FramebufferD3D::checkStatus() const ...@@ -326,13 +326,19 @@ GLenum FramebufferD3D::checkStatus() const
(attachment.id() == prevAttachment.id() && (attachment.id() == prevAttachment.id() &&
attachment.type() == prevAttachment.type())) attachment.type() == prevAttachment.type()))
{ {
return GL_FRAMEBUFFER_UNSUPPORTED; return false;
} }
} }
} }
} }
return GL_FRAMEBUFFER_COMPLETE; // D3D requires all render targets to have the same dimensions.
if (!mData.attachmentsHaveSameDimensions())
{
return false;
}
return true;
} }
const gl::AttachmentList &FramebufferD3D::getColorAttachmentsForRender( const gl::AttachmentList &FramebufferD3D::getColorAttachmentsForRender(
......
...@@ -79,7 +79,7 @@ class FramebufferD3D : public FramebufferImpl ...@@ -79,7 +79,7 @@ class FramebufferD3D : public FramebufferImpl
gl::Error blit(const gl::State &state, const gl::Rectangle &sourceArea, const gl::Rectangle &destArea, gl::Error blit(const gl::State &state, const gl::Rectangle &sourceArea, const gl::Rectangle &destArea,
GLbitfield mask, GLenum filter, const gl::Framebuffer *sourceFramebuffer) override; GLbitfield mask, GLenum filter, const gl::Framebuffer *sourceFramebuffer) override;
GLenum checkStatus() const override; bool checkStatus() const override;
const gl::AttachmentList &getColorAttachmentsForRender(const WorkaroundsD3D &workarounds) const; const gl::AttachmentList &getColorAttachmentsForRender(const WorkaroundsD3D &workarounds) const;
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include "libANGLE/renderer/gl/StateManagerGL.h" #include "libANGLE/renderer/gl/StateManagerGL.h"
#include "libANGLE/renderer/gl/TextureGL.h" #include "libANGLE/renderer/gl/TextureGL.h"
#include "libANGLE/renderer/gl/WorkaroundsGL.h" #include "libANGLE/renderer/gl/WorkaroundsGL.h"
#include "platform/Platform.h"
namespace rx namespace rx
{ {
...@@ -292,10 +293,15 @@ gl::Error FramebufferGL::blit(const gl::State &state, const gl::Rectangle &sourc ...@@ -292,10 +293,15 @@ gl::Error FramebufferGL::blit(const gl::State &state, const gl::Rectangle &sourc
return gl::Error(GL_NO_ERROR); return gl::Error(GL_NO_ERROR);
} }
GLenum FramebufferGL::checkStatus() const bool FramebufferGL::checkStatus() const
{ {
mStateManager->bindFramebuffer(GL_FRAMEBUFFER, mFramebufferID); mStateManager->bindFramebuffer(GL_FRAMEBUFFER, mFramebufferID);
return mFunctions->checkFramebufferStatus(GL_FRAMEBUFFER); GLenum status = mFunctions->checkFramebufferStatus(GL_FRAMEBUFFER);
if (status != GL_FRAMEBUFFER_COMPLETE)
{
ANGLEPlatformCurrent()->logWarning("GL framebuffer returned incomplete.");
}
return (status == GL_FRAMEBUFFER_COMPLETE);
} }
GLuint FramebufferGL::getFramebufferID() const GLuint FramebufferGL::getFramebufferID() const
......
...@@ -61,7 +61,7 @@ class FramebufferGL : public FramebufferImpl ...@@ -61,7 +61,7 @@ class FramebufferGL : public FramebufferImpl
gl::Error blit(const gl::State &state, const gl::Rectangle &sourceArea, const gl::Rectangle &destArea, gl::Error blit(const gl::State &state, const gl::Rectangle &sourceArea, const gl::Rectangle &destArea,
GLbitfield mask, GLenum filter, const gl::Framebuffer *sourceFramebuffer) override; GLbitfield mask, GLenum filter, const gl::Framebuffer *sourceFramebuffer) override;
GLenum checkStatus() const override; bool checkStatus() const override;
void syncDrawState() const; void syncDrawState() const;
......
...@@ -74,8 +74,8 @@ TEST(ValidationESTest, DrawElementsWithMaxIndexGivesError) ...@@ -74,8 +74,8 @@ TEST(ValidationESTest, DrawElementsWithMaxIndexGivesError)
EXPECT_CALL(*framebufferImpl, onUpdateColorAttachment(_)).Times(1).RetiresOnSaturation(); EXPECT_CALL(*framebufferImpl, onUpdateColorAttachment(_)).Times(1).RetiresOnSaturation();
EXPECT_CALL(*framebufferImpl, checkStatus()) EXPECT_CALL(*framebufferImpl, checkStatus())
.Times(2) .Times(2)
.WillOnce(Return(GL_FRAMEBUFFER_COMPLETE)) .WillOnce(Return(true))
.WillOnce(Return(GL_FRAMEBUFFER_COMPLETE)); .WillOnce(Return(true));
EXPECT_CALL(*framebufferImpl, destroy()).Times(1).RetiresOnSaturation(); EXPECT_CALL(*framebufferImpl, destroy()).Times(1).RetiresOnSaturation();
MockProgramImpl *programImpl = new MockProgramImpl(); MockProgramImpl *programImpl = new MockProgramImpl();
......
...@@ -30,6 +30,9 @@ ...@@ -30,6 +30,9 @@
// TODO(jmadill): Figure out why this fails on the bots, but not locally. // TODO(jmadill): Figure out why this fails on the bots, but not locally.
1108 WIN : dEQP-GLES3.functional.shaders.struct.local.dynamic_loop_struct_array_fragment = FAIL 1108 WIN : dEQP-GLES3.functional.shaders.struct.local.dynamic_loop_struct_array_fragment = FAIL
// We can't support distinct texture sizes in D3D11.
1097 WIN : dEQP-GLES3.functional.fbo.completeness.size.distinct = FAIL
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
// //
// Temprory entries: they should be removed once the bugs are fixed. // Temprory entries: they should be removed once the bugs are fixed.
...@@ -457,7 +460,6 @@ ...@@ -457,7 +460,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.size.distinct = FAIL
1097 WIN : dEQP-GLES3.functional.fbo.completeness.layer.2darr_1_3 = 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.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_1_15 = 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