Commit a15472a3 by Geoff Lang

Fix bugs with drawbuffer state.

* IsAttachmentEnabled was checking the wrong draw buffer state. Instead of checking that drawbuffer[colorAttachment] is in use, it should have been scanning for a drawbuffer state that points to colorAttachment. * Allow for maxDrawBuffer != maxColorAttachments. Tested by the GL backend on some systems that don't have the draw buffers extension. Fixed by updating the helpers and adding a new getDrawBuffer helper. BUG=angleproject:1121 Change-Id: Idd1b0a9ec4a3f944d332c708364408bf5d59e1fd Reviewed-on: https://chromium-review.googlesource.com/292740Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org> Tryjob-Request: Geoff Lang <geofflang@chromium.org> Tested-by: 's avatarGeoff Lang <geofflang@chromium.org>
parent 722b0d31
......@@ -54,6 +54,7 @@ Framebuffer::Data::Data(const Caps &caps)
mDrawBufferStates(caps.maxDrawBuffers, GL_NONE),
mReadBufferState(GL_COLOR_ATTACHMENT0_EXT)
{
ASSERT(mDrawBufferStates.size() > 0);
mDrawBufferStates[0] = GL_COLOR_ATTACHMENT0_EXT;
}
......@@ -287,10 +288,15 @@ const FramebufferAttachment *Framebuffer::getAttachment(GLenum attachment) const
}
}
GLenum Framebuffer::getDrawBufferState(unsigned int colorAttachment) const
size_t Framebuffer::getDrawbufferStateCount() const
{
ASSERT(colorAttachment < mData.mDrawBufferStates.size());
return mData.mDrawBufferStates[colorAttachment];
return mData.mDrawBufferStates.size();
}
GLenum Framebuffer::getDrawBufferState(size_t drawBuffer) const
{
ASSERT(drawBuffer < mData.mDrawBufferStates.size());
return mData.mDrawBufferStates[drawBuffer];
}
void Framebuffer::setDrawBuffers(size_t count, const GLenum *buffers)
......@@ -303,6 +309,36 @@ void Framebuffer::setDrawBuffers(size_t count, const GLenum *buffers)
mDirtyBits.set(DIRTY_BIT_DRAW_BUFFERS);
}
const FramebufferAttachment *Framebuffer::getDrawBuffer(size_t drawBuffer) const
{
ASSERT(drawBuffer < mData.mDrawBufferStates.size());
if (mData.mDrawBufferStates[drawBuffer] != GL_NONE)
{
// ES3 spec: "If the GL is bound to a draw framebuffer object, the ith buffer listed in bufs
// must be COLOR_ATTACHMENTi or NONE"
ASSERT(mData.mDrawBufferStates[drawBuffer] == GL_COLOR_ATTACHMENT0 + drawBuffer ||
(drawBuffer == 0 && mData.mDrawBufferStates[drawBuffer] == GL_BACK));
return getAttachment(mData.mDrawBufferStates[drawBuffer]);
}
else
{
return nullptr;
}
}
bool Framebuffer::hasEnabledDrawBuffer() const
{
for (size_t drawbufferIdx = 0; drawbufferIdx < mData.mDrawBufferStates.size(); ++drawbufferIdx)
{
if (getDrawBuffer(drawbufferIdx) != nullptr)
{
return true;
}
}
return false;
}
GLenum Framebuffer::getReadBufferState() const
{
return mData.mReadBufferState;
......@@ -317,26 +353,6 @@ void Framebuffer::setReadBuffer(GLenum buffer)
mDirtyBits.set(DIRTY_BIT_READ_BUFFER);
}
bool Framebuffer::isEnabledColorAttachment(size_t colorAttachment) const
{
ASSERT(colorAttachment < mData.mColorAttachments.size());
return (mData.mColorAttachments[colorAttachment].isAttached() &&
mData.mDrawBufferStates[colorAttachment] != GL_NONE);
}
bool Framebuffer::hasEnabledColorAttachment() const
{
for (size_t colorAttachment = 0; colorAttachment < mData.mColorAttachments.size(); ++colorAttachment)
{
if (isEnabledColorAttachment(static_cast<unsigned int>(colorAttachment)))
{
return true;
}
}
return false;
}
size_t Framebuffer::getNumColorBuffers() const
{
return mData.mColorAttachments.size();
......@@ -354,9 +370,9 @@ bool Framebuffer::hasStencil() const
bool Framebuffer::usingExtendedDrawBuffers() const
{
for (size_t colorAttachment = 1; colorAttachment < mData.mColorAttachments.size(); ++colorAttachment)
for (size_t drawbufferIdx = 1; drawbufferIdx < mData.mDrawBufferStates.size(); ++drawbufferIdx)
{
if (isEnabledColorAttachment(static_cast<unsigned int>(colorAttachment)))
if (getDrawBuffer(drawbufferIdx) != nullptr)
{
return true;
}
......
......@@ -117,14 +117,15 @@ class Framebuffer final : public LabeledObject
const FramebufferAttachment *getAttachment(GLenum attachment) const;
GLenum getDrawBufferState(unsigned int colorAttachment) const;
size_t getDrawbufferStateCount() const;
GLenum getDrawBufferState(size_t drawBuffer) const;
void setDrawBuffers(size_t count, const GLenum *buffers);
const FramebufferAttachment *getDrawBuffer(size_t drawBuffer) const;
bool hasEnabledDrawBuffer() const;
GLenum getReadBufferState() const;
void setReadBuffer(GLenum buffer);
bool isEnabledColorAttachment(size_t colorAttachment) const;
bool hasEnabledColorAttachment() const;
size_t getNumColorBuffers() const;
bool hasDepth() const;
bool hasStencil() const;
......
......@@ -54,7 +54,7 @@ ClearParameters GetClearParameters(const gl::State &state, GLbitfield mask)
const gl::Framebuffer *framebufferObject = state.getDrawFramebuffer();
if (mask & GL_COLOR_BUFFER_BIT)
{
if (framebufferObject->hasEnabledColorAttachment())
if (framebufferObject->hasEnabledDrawBuffer())
{
for (unsigned int i = 0; i < ArraySize(clearParams.clearColor); i++)
{
......
......@@ -244,6 +244,9 @@ void GenerateCaps(const FunctionsGL *functions, gl::Caps *caps, gl::TextureCapsM
}
else
{
// Framebuffer is required to have at least one drawbuffer even if the extension is not
// supported
caps->maxDrawBuffers = 1;
LimitVersion(maxSupportedESVersion, gl::Version(2, 0));
}
......
......@@ -682,11 +682,14 @@ bool ValidateBlitFramebufferParameters(gl::Context *context,
GLenum readInternalFormat = readColorBuffer->getInternalFormat();
const InternalFormat &readFormatInfo = GetInternalFormatInfo(readInternalFormat);
for (size_t i = 0; i < drawFramebuffer->getNumColorBuffers(); i++)
for (size_t drawbufferIdx = 0;
drawbufferIdx < drawFramebuffer->getDrawbufferStateCount(); ++drawbufferIdx)
{
if (drawFramebuffer->isEnabledColorAttachment(i))
const FramebufferAttachment *attachment =
drawFramebuffer->getDrawBuffer(drawbufferIdx);
if (attachment)
{
GLenum drawInternalFormat = drawFramebuffer->getColorbuffer(i)->getInternalFormat();
GLenum drawInternalFormat = attachment->getInternalFormat();
const InternalFormat &drawFormatInfo = GetInternalFormatInfo(drawInternalFormat);
// The GL ES 3.0.2 spec (pg 193) states that:
......
......@@ -1602,15 +1602,13 @@ bool ValidateBlitFramebufferANGLE(Context *context,
return false;
}
for (size_t colorAttachment = 0;
colorAttachment < drawFramebuffer->getNumColorBuffers(); ++colorAttachment)
for (size_t drawbufferIdx = 0;
drawbufferIdx < drawFramebuffer->getDrawbufferStateCount(); ++drawbufferIdx)
{
if (drawFramebuffer->isEnabledColorAttachment(colorAttachment))
const FramebufferAttachment *attachment =
drawFramebuffer->getDrawBuffer(drawbufferIdx);
if (attachment)
{
const FramebufferAttachment *attachment =
drawFramebuffer->getColorbuffer(colorAttachment);
ASSERT(attachment);
if (!(attachment->type() == GL_TEXTURE &&
attachment->getTextureImageIndex().type == GL_TEXTURE_2D) &&
attachment->type() != GL_RENDERBUFFER &&
......
......@@ -38,7 +38,8 @@ class DrawBuffersTest : public ANGLETest
for (size_t texIndex = 0; texIndex < ArraySize(mTextures); texIndex++)
{
glBindTexture(GL_TEXTURE_2D, mTextures[texIndex]);
glTexStorage2DEXT(GL_TEXTURE_2D, 1, GL_RGBA8, getWindowWidth(), getWindowHeight());
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, getWindowWidth(), getWindowHeight(), 0, GL_RGBA,
GL_UNSIGNED_BYTE, nullptr);
}
GLfloat data[] =
......@@ -231,6 +232,13 @@ TEST_P(DrawBuffersTest, VerifyD3DLimits)
TEST_P(DrawBuffersTest, Gaps)
{
if (getClientVersion() < 3 && !extensionEnabled("GL_EXT_draw_buffers"))
{
std::cout << "Test skipped because ES3 or GL_EXT_draw_buffers is not available."
<< std::endl;
return;
}
glBindTexture(GL_TEXTURE_2D, mTextures[0]);
glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT1, GL_TEXTURE_2D, mTextures[0], 0);
......@@ -255,6 +263,13 @@ TEST_P(DrawBuffersTest, Gaps)
TEST_P(DrawBuffersTest, FirstAndLast)
{
if (getClientVersion() < 3 && !extensionEnabled("GL_EXT_draw_buffers"))
{
std::cout << "Test skipped because ES3 or GL_EXT_draw_buffers is not available."
<< std::endl;
return;
}
glBindTexture(GL_TEXTURE_2D, mTextures[0]);
glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, mTextures[0], 0);
......@@ -288,6 +303,13 @@ TEST_P(DrawBuffersTest, FirstAndLast)
TEST_P(DrawBuffersTest, FirstHalfNULL)
{
if (getClientVersion() < 3 && !extensionEnabled("GL_EXT_draw_buffers"))
{
std::cout << "Test skipped because ES3 or GL_EXT_draw_buffers is not available."
<< std::endl;
return;
}
bool flags[8] = { false };
GLenum bufs[8] = { GL_NONE };
......@@ -320,6 +342,13 @@ TEST_P(DrawBuffersTest, FirstHalfNULL)
TEST_P(DrawBuffersTest, UnwrittenOutputVariablesShouldNotCrash)
{
if (getClientVersion() < 3 && !extensionEnabled("GL_EXT_draw_buffers"))
{
std::cout << "Test skipped because ES3 or GL_EXT_draw_buffers is not available."
<< std::endl;
return;
}
// Bind two render targets but use a shader which writes only to the first one.
glBindTexture(GL_TEXTURE_2D, mTextures[0]);
glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, mTextures[0], 0);
......
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