Skip to content

Commit

Permalink
Cache Framebuffer completeness.
Browse files Browse the repository at this point in the history 
Improves performance on the render-to-texture microbenchmark
by ~3x on the OpenGL back-end. Wipes out several of the top profling
hotspots on that benchmark.

BUG=angleproject:1388

Change-Id: I6a35a0b435b2ed3c83d32acdb9df090df98214ad
Reviewed-on: https://chromium-review.googlesource.com/348957
Reviewed-by: Corentin Wallez <[email protected]>
Commit-Queue: Corentin Wallez <[email protected]>
  • Loading branch information
@null77
null77 authored and Commit Bot committed Jun 17, 2016
1 parent 73d417e commit 362876b
Show file tree
Hide file tree
Showing 12 changed files with 425 additions and 282 deletions.
134 changes: 89 additions & 45 deletions src/libANGLE/Framebuffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,21 @@
#include "libANGLE/renderer/RenderbufferImpl.h"
#include "libANGLE/renderer/SurfaceImpl.h"

using namespace angle;

namespace gl
{

namespace
{
void DetachMatchingAttachment(FramebufferAttachment *attachment, GLenum matchType, GLuint matchId)

void BindResourceChannel(ChannelBinding *binding, FramebufferAttachmentObject *resource)
{
if (attachment->isAttached() &&
attachment->type() == matchType &&
attachment->id() == matchId)
{
attachment->detach();
}
}
binding->bind(resource ? resource->getDirtyChannel() : nullptr);
}

} // anonymous namespace

FramebufferState::FramebufferState()
: mLabel(),
mColorAttachments(1),
Expand Down Expand Up @@ -171,16 +170,35 @@ bool FramebufferState::attachmentsHaveSameDimensions() const
}

Framebuffer::Framebuffer(const Caps &caps, rx::GLImplFactory *factory, GLuint id)
: mState(caps), mImpl(factory->createFramebuffer(mState)), mId(id)
: mState(caps),
mImpl(factory->createFramebuffer(mState)),
mId(id),
mCachedStatus(),
mDirtyDepthAttachmentBinding(this, DIRTY_BIT_DEPTH_ATTACHMENT),
mDirtyStencilAttachmentBinding(this, DIRTY_BIT_STENCIL_ATTACHMENT)
{
ASSERT(mId != 0);
ASSERT(mImpl != nullptr);
ASSERT(mState.mColorAttachments.size() == static_cast<size_t>(caps.maxColorAttachments));

for (size_t colorIndex = 0; colorIndex < mState.mColorAttachments.size(); ++colorIndex)
{
mDirtyColorAttachmentBindings.push_back(ChannelBinding(
this, static_cast<SignalToken>(DIRTY_BIT_COLOR_ATTACHMENT_0 + colorIndex)));
}
}

Framebuffer::Framebuffer(rx::SurfaceImpl *surface)
: mState(), mImpl(surface->createDefaultFramebuffer(mState)), mId(0)
: mState(),
mImpl(surface->createDefaultFramebuffer(mState)),
mId(0),
mCachedStatus(GL_FRAMEBUFFER_COMPLETE),
mDirtyDepthAttachmentBinding(this, DIRTY_BIT_DEPTH_ATTACHMENT),
mDirtyStencilAttachmentBinding(this, DIRTY_BIT_STENCIL_ATTACHMENT)
{
ASSERT(mImpl != nullptr);
mDirtyColorAttachmentBindings.push_back(
ChannelBinding(this, static_cast<SignalToken>(DIRTY_BIT_COLOR_ATTACHMENT_0)));
}

Framebuffer::~Framebuffer()
Expand Down Expand Up @@ -210,13 +228,28 @@ void Framebuffer::detachRenderbuffer(GLuint renderbufferId)

void Framebuffer::detachResourceById(GLenum resourceType, GLuint resourceId)
{
for (auto &colorAttachment : mState.mColorAttachments)
for (size_t colorIndex = 0; colorIndex < mState.mColorAttachments.size(); ++colorIndex)
{
DetachMatchingAttachment(&colorAttachment, resourceType, resourceId);
detachMatchingAttachment(&mState.mColorAttachments[colorIndex], resourceType, resourceId,
DIRTY_BIT_COLOR_ATTACHMENT_0 + colorIndex);
}

DetachMatchingAttachment(&mState.mDepthAttachment, resourceType, resourceId);
DetachMatchingAttachment(&mState.mStencilAttachment, resourceType, resourceId);
detachMatchingAttachment(&mState.mDepthAttachment, resourceType, resourceId,
DIRTY_BIT_DEPTH_ATTACHMENT);
detachMatchingAttachment(&mState.mStencilAttachment, resourceType, resourceId,
DIRTY_BIT_STENCIL_ATTACHMENT);
}

void Framebuffer::detachMatchingAttachment(FramebufferAttachment *attachment,
GLenum matchType,
GLuint matchId,
size_t dirtyBit)
{
if (attachment->isAttached() && attachment->type() == matchType && attachment->id() == matchId)
{
attachment->detach();
mDirtyBits.set(dirtyBit);
}
}

const FramebufferAttachment *Framebuffer::getColorbuffer(size_t colorAttachment) const
Expand Down Expand Up @@ -397,6 +430,18 @@ GLenum Framebuffer::checkStatus(const ContextState &state)
return GL_FRAMEBUFFER_COMPLETE;
}

if (hasAnyDirtyBit() || !mCachedStatus.valid())
{
mCachedStatus = checkStatusImpl(state);
}

return mCachedStatus.value();
}

GLenum Framebuffer::checkStatusImpl(const ContextState &state)
{
ASSERT(mId != 0);

unsigned int colorbufferSize = 0;
int samples = -1;
bool missingAttachment = true;
Expand Down Expand Up @@ -644,7 +689,7 @@ Error Framebuffer::clear(rx::ContextImpl *context, GLbitfield mask)
{
if (context->getGLState().isRasterizerDiscardEnabled())
{
return gl::Error(GL_NO_ERROR);
return gl::NoError();
}

return mImpl->clear(context, mask);
Expand All @@ -657,7 +702,7 @@ Error Framebuffer::clearBufferfv(rx::ContextImpl *context,
{
if (context->getGLState().isRasterizerDiscardEnabled())
{
return gl::Error(GL_NO_ERROR);
return gl::NoError();
}

return mImpl->clearBufferfv(context, buffer, drawbuffer, values);
Expand All @@ -670,7 +715,7 @@ Error Framebuffer::clearBufferuiv(rx::ContextImpl *context,
{
if (context->getGLState().isRasterizerDiscardEnabled())
{
return gl::Error(GL_NO_ERROR);
return gl::NoError();
}

return mImpl->clearBufferuiv(context, buffer, drawbuffer, values);
Expand All @@ -683,7 +728,7 @@ Error Framebuffer::clearBufferiv(rx::ContextImpl *context,
{
if (context->getGLState().isRasterizerDiscardEnabled())
{
return gl::Error(GL_NO_ERROR);
return gl::NoError();
}

return mImpl->clearBufferiv(context, buffer, drawbuffer, values);
Expand All @@ -697,7 +742,7 @@ Error Framebuffer::clearBufferfi(rx::ContextImpl *context,
{
if (context->getGLState().isRasterizerDiscardEnabled())
{
return gl::Error(GL_NO_ERROR);
return gl::NoError();
}

return mImpl->clearBufferfi(context, buffer, drawbuffer, depth, stencil);
Expand All @@ -719,19 +764,15 @@ Error Framebuffer::readPixels(rx::ContextImpl *context,
GLenum type,
GLvoid *pixels) const
{
Error error = mImpl->readPixels(context, area, format, type, pixels);
if (error.isError())
{
return error;
}
ANGLE_TRY(mImpl->readPixels(context, area, format, type, pixels));

Buffer *unpackBuffer = context->getGLState().getUnpackState().pixelBuffer.get();
if (unpackBuffer)
{
unpackBuffer->onPixelUnpack();
}

return Error(GL_NO_ERROR);
return NoError();
}

Error Framebuffer::blit(rx::ContextImpl *context,
Expand All @@ -745,16 +786,15 @@ Error Framebuffer::blit(rx::ContextImpl *context,

int Framebuffer::getSamples(const ContextState &state)
{
if (checkStatus(state) == GL_FRAMEBUFFER_COMPLETE)
if (complete(state))
{
// for a complete framebuffer, all attachments must have the same sample count
// in this case return the first nonzero sample size
for (const FramebufferAttachment &colorAttachment : mState.mColorAttachments)
// For a complete framebuffer, all attachments must have the same sample count.
// In this case return the first nonzero sample size.
const auto *firstColorAttachment = mState.getFirstColorAttachment();
if (firstColorAttachment)
{
if (colorAttachment.isAttached())
{
return colorAttachment.getSamples();
}
ASSERT(firstColorAttachment->isAttached());
return firstColorAttachment->getSamples();
}
}

Expand Down Expand Up @@ -791,6 +831,8 @@ void Framebuffer::setAttachment(GLenum type,
mState.mStencilAttachment.attach(type, binding, textureIndex, attachmentObj);
mDirtyBits.set(DIRTY_BIT_DEPTH_ATTACHMENT);
mDirtyBits.set(DIRTY_BIT_STENCIL_ATTACHMENT);
BindResourceChannel(&mDirtyDepthAttachmentBinding, resource);
BindResourceChannel(&mDirtyStencilAttachmentBinding, resource);
}
else
{
Expand All @@ -800,22 +842,26 @@ void Framebuffer::setAttachment(GLenum type,
case GL_DEPTH_ATTACHMENT:
mState.mDepthAttachment.attach(type, binding, textureIndex, resource);
mDirtyBits.set(DIRTY_BIT_DEPTH_ATTACHMENT);
break;
BindResourceChannel(&mDirtyDepthAttachmentBinding, resource);
break;
case GL_STENCIL:
case GL_STENCIL_ATTACHMENT:
mState.mStencilAttachment.attach(type, binding, textureIndex, resource);
mDirtyBits.set(DIRTY_BIT_STENCIL_ATTACHMENT);
break;
BindResourceChannel(&mDirtyStencilAttachmentBinding, resource);
break;
case GL_BACK:
mState.mColorAttachments[0].attach(type, binding, textureIndex, resource);
mDirtyBits.set(DIRTY_BIT_COLOR_ATTACHMENT_0);
break;
// No need for a resource binding for the default FBO, it's always complete.
break;
default:
{
size_t colorIndex = binding - GL_COLOR_ATTACHMENT0;
ASSERT(colorIndex < mState.mColorAttachments.size());
mState.mColorAttachments[colorIndex].attach(type, binding, textureIndex, resource);
mDirtyBits.set(DIRTY_BIT_COLOR_ATTACHMENT_0 + colorIndex);
BindResourceChannel(&mDirtyColorAttachmentBindings[colorIndex], resource);
}
break;
}
Expand All @@ -827,27 +873,25 @@ void Framebuffer::resetAttachment(GLenum binding)
setAttachment(GL_NONE, binding, ImageIndex::MakeInvalid(), nullptr);
}

void Framebuffer::syncState() const
void Framebuffer::syncState()
{
if (mDirtyBits.any())
{
mImpl->syncState(mDirtyBits);
mDirtyBits.reset();
mCachedStatus.reset();
}
}

int Framebuffer::getCachedSamples(const ContextState &state) const
void Framebuffer::signal(SignalToken token)
{
// TODO(jmadill): Framebuffer samples caching.
ASSERT(mDirtyBits.none());
return const_cast<Framebuffer *>(this)->getSamples(state);
// TOOD(jmadill): Make this only update individual attachments to do less work.
mCachedStatus.reset();
}

GLenum Framebuffer::getCachedStatus(const ContextState &state) const
bool Framebuffer::complete(const ContextState &state)
{
// TODO(jmadill): Framebuffer status caching.
ASSERT(mDirtyBits.none());
return const_cast<Framebuffer *>(this)->checkStatus(state);
return (checkStatus(state) == GL_FRAMEBUFFER_COMPLETE);
}

} // namespace gl
32 changes: 22 additions & 10 deletions src/libANGLE/Framebuffer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@

#include <vector>

#include "common/Optional.h"
#include "common/angleutils.h"
#include "libANGLE/Constants.h"
#include "libANGLE/Debug.h"
#include "libANGLE/Error.h"
#include "libANGLE/FramebufferAttachment.h"
#include "libANGLE/RefCountObject.h"
#include "libANGLE/signal_utils.h"

namespace rx
{
Expand Down Expand Up @@ -86,7 +88,7 @@ class FramebufferState final : angle::NonCopyable
GLenum mReadBufferState;
};

class Framebuffer final : public LabeledObject
class Framebuffer final : public LabeledObject, public angle::SignalReceiver
{
public:
Framebuffer(const Caps &caps, rx::GLImplFactory *factory, GLuint id);
Expand Down Expand Up @@ -140,10 +142,8 @@ class Framebuffer final : public LabeledObject
int getSamples(const ContextState &state);
GLenum checkStatus(const ContextState &state);

// These methods do not change any state.
// TODO(jmadill): Remove ContextState parameter when able.
int getCachedSamples(const ContextState &state) const;
GLenum getCachedStatus(const ContextState &state) const;
// Helper for checkStatus == GL_FRAMEBUFFER_COMPLETE.
bool complete(const ContextState &state);

bool hasValidDepthStencil() const;

Expand Down Expand Up @@ -200,19 +200,31 @@ class Framebuffer final : public LabeledObject
typedef std::bitset<DIRTY_BIT_MAX> DirtyBits;
bool hasAnyDirtyBit() const { return mDirtyBits.any(); }

void syncState() const;
void syncState();

protected:
// angle::SignalReceiver implementation
void signal(angle::SignalToken token) override;

private:
void detachResourceById(GLenum resourceType, GLuint resourceId);
void detachMatchingAttachment(FramebufferAttachment *attachment,
GLenum matchType,
GLuint matchId,
size_t dirtyBit);
GLenum checkStatusImpl(const ContextState &state);

FramebufferState mState;
rx::FramebufferImpl *mImpl;
GLuint mId;

// TODO(jmadill): See if we can make this non-mutable.
mutable DirtyBits mDirtyBits;
Optional<GLenum> mCachedStatus;
std::vector<angle::ChannelBinding> mDirtyColorAttachmentBindings;
angle::ChannelBinding mDirtyDepthAttachmentBinding;
angle::ChannelBinding mDirtyStencilAttachmentBinding;

DirtyBits mDirtyBits;
};

}
} // namespace gl

#endif // LIBANGLE_FRAMEBUFFER_H_
5 changes: 5 additions & 0 deletions src/libANGLE/FramebufferAttachment.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,4 +233,9 @@ Error FramebufferAttachmentObject::getAttachmentRenderTarget(
return getAttachmentImpl()->getAttachmentRenderTarget(target, rtOut);
}

angle::BroadcastChannel *FramebufferAttachmentObject::getDirtyChannel()
{
return &mDirtyChannel;
}

} // namespace gl
5 changes: 5 additions & 0 deletions src/libANGLE/FramebufferAttachment.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "libANGLE/angletypes.h"
#include "libANGLE/Error.h"
#include "libANGLE/ImageIndex.h"
#include "libANGLE/signal_utils.h"

namespace egl
{
Expand Down Expand Up @@ -165,8 +166,12 @@ class FramebufferAttachmentObject
Error getAttachmentRenderTarget(const FramebufferAttachment::Target &target,
rx::FramebufferAttachmentRenderTarget **rtOut) const;

angle::BroadcastChannel *getDirtyChannel();

protected:
virtual rx::FramebufferAttachmentObjectImpl *getAttachmentImpl() const = 0;

angle::BroadcastChannel mDirtyChannel;
};

inline Extents FramebufferAttachment::getSize() const
Expand Down
Loading

0 comments on commit 362876b

Please sign in to comment.