From f33a98b1a044c8ac7b12778cbca6c4a124875e4a Mon Sep 17 00:00:00 2001 From: Mikko Rasa Date: Sat, 21 Dec 2013 11:36:40 +0200 Subject: [PATCH] Rewrite Bind as two different classes Boolean flag parameters are not too clear without seeing the declaration. This new implementation also doesn't use dynamic allocation. --- source/ambientocclusion.cpp | 6 +- source/bindable.h | 134 ++++++++++++++++++------------------ source/bloom.cpp | 6 +- source/font.cpp | 2 +- source/framebuffer.cpp | 4 +- source/pipeline.cpp | 4 +- source/renderbuffer.cpp | 4 +- source/shadowmap.cpp | 2 +- source/texture2d.cpp | 6 +- source/texture3d.cpp | 6 +- source/texturecube.cpp | 4 +- 11 files changed, 87 insertions(+), 91 deletions(-) diff --git a/source/ambientocclusion.cpp b/source/ambientocclusion.cpp index 4ef37625..f42069be 100644 --- a/source/ambientocclusion.cpp +++ b/source/ambientocclusion.cpp @@ -135,11 +135,11 @@ void AmbientOcclusion::render(const Texture2D &color, const Texture2D &depth) combine_texturing.attach(0, depth); combine_texturing.attach(1, color); - Bind unbind_dtest(static_cast(0), true); - Bind unbind_blend(static_cast(0), true); + BindRestore unbind_dtest(static_cast(0)); + BindRestore unbind_blend(static_cast(0)); { - Bind bind_fbo(fbo, true); + BindRestore bind_fbo(fbo); Bind bind_tex(occlude_texturing); Bind bind_shader(occlude_shader); occlude_shdata.apply(); diff --git a/source/bindable.h b/source/bindable.h index bde5e85a..f534d7af 100644 --- a/source/bindable.h +++ b/source/bindable.h @@ -66,95 +66,93 @@ public: /** RAII class for binding things. Binds the thing upon construction and unbinds it upon destruction. If a null pointer is given, unbinds upon construction and -does nothing upon destruction. Optionally can restore the previous binding. +does nothing upon destruction. */ class Bind { private: - struct Base - { - virtual ~Base() { } - }; + typedef void CleanupFunc(); + + CleanupFunc *cleanup; +public: template - struct Binder1: Base - { - const T *obj; - - Binder1(const T *o): - obj(o) - { - if(obj) - obj->bind(); - else - T::unbind(); - } - - ~Binder1() - { - if(obj) - obj->unbind(); - } - }; - - template - struct Binder2: Base + Bind(T *o) { init(o); } + + template + Bind(const T *o) { init(o); } + + template + Bind(const T &o) { init(&o); } + +private: + template + void init(const T *o) { - const T *obj; - const U *old; - - Binder2(const T *o, const U *l): - obj(o), - old(l) - { - if(obj) - obj->bind(); - else - T::unbind(); - } - - ~Binder2() - { - if(old) - old->bind(); - else if(obj) - obj->unbind(); - } - }; - - Base *binder; + cleanup = (o ? &unbind : 0); + if(o) + o->bind(); + else + T::unbind(); + } + +public: + ~Bind() + { if(cleanup) cleanup(); } + +private: + template + static void unbind() + { T::unbind(); } +}; + + +/** +Similar to Bind, but restores previous binding upon destruction. +*/ +class BindRestore +{ +private: + typedef void CleanupFunc(const void *); + + const void *old; + CleanupFunc *cleanup; public: template - Bind(const T &o, bool r = false): - binder(r ? create(&o, T::current()) : create(&o)) - { } + BindRestore(T *o) { init(o); } template - Bind(const T *o, bool r = false): - binder(r ? create(o, T::current()) : create(o)) - { } + BindRestore(const T *o) { init(o); } template - Bind(T *o, bool r = false): - binder(r ? create(o, T::current()) : create(o)) - { } + BindRestore(const T &o) { init(&o); } private: - Bind(const Bind &); - Bind &operator=(const Bind &); + template + void init(T *o) + { + old = T::current(); + cleanup = (o!=old ? &restore : 0); + if(o) + o->bind(); + else if(old) + T::unbind(); + } public: - ~Bind() { delete binder; } + ~BindRestore() + { if(cleanup) cleanup(old); } private: template - Base *create(const T *o) - { return new Binder1(o); } - - template - Base *create(const T *o, const U *l) - { return new Binder2(o, l); } + static void restore(const void *o) + { + if(o) + reinterpret_cast(o)->bind(); + else + T::unbind(); + } }; } // namespace GL diff --git a/source/bloom.cpp b/source/bloom.cpp index ad7ec269..5ffa82f6 100644 --- a/source/bloom.cpp +++ b/source/bloom.cpp @@ -100,15 +100,15 @@ void Bloom::set_strength(float s) void Bloom::render(const Texture2D &src, const Texture2D &) { - Bind unbind_dtest(static_cast(0), true); - Bind unbind_blend(static_cast(0), true); + BindRestore unbind_dtest(static_cast(0)); + BindRestore unbind_blend(static_cast(0)); { Bind bind_shader(blur_shader); blur_shdata_common.apply(); for(unsigned i=0; i<2; ++i) { - Bind bind_fbo(fbo[i], true); + BindRestore bind_fbo(fbo[i]); Bind bind_tex(i ? tex[0] : src); blur_shdata[i].apply(); quad.draw(); diff --git a/source/font.cpp b/source/font.cpp index 1639f4a3..1005609e 100644 --- a/source/font.cpp +++ b/source/font.cpp @@ -64,7 +64,7 @@ float Font::get_string_width(const string &str, StringCodec::Decoder &dec) const void Font::draw_string(const string &str, StringCodec::Decoder &dec, const Color &color) const { - Bind bind_tex(get_texture(), true); + BindRestore bind_tex(get_texture()); Immediate imm((TEXCOORD2, COLOR4_UBYTE, VERTEX2)); imm.color(color); build_string(str, dec, imm); diff --git a/source/framebuffer.cpp b/source/framebuffer.cpp index ae8f4dd4..67249518 100644 --- a/source/framebuffer.cpp +++ b/source/framebuffer.cpp @@ -213,7 +213,7 @@ void Framebuffer::detach(FramebufferAttachment attch) FramebufferStatus Framebuffer::check_status() const { - Bind _bind(this, true); + BindRestore _bind(this); return static_cast(glCheckFramebufferStatus(GL_FRAMEBUFFER)); } @@ -226,7 +226,7 @@ void Framebuffer::require_complete() const void Framebuffer::clear(BufferBits bits) { - Bind _bind(this, true); + BindRestore _bind(this); glClear(bits); } diff --git a/source/pipeline.cpp b/source/pipeline.cpp index 8229515f..0546a4e8 100644 --- a/source/pipeline.cpp +++ b/source/pipeline.cpp @@ -147,9 +147,7 @@ void Pipeline::render(Renderer &renderer, const Tag &tag) const setup_frame(); const Framebuffer *out_fbo = Framebuffer::current(); - /* Binding the current object is a no-op, but this will restore the original - FBO in case an exception is thrown. */ - Bind restore_fbo(out_fbo, true); + // XXX Make sure the correct FBO is restored if an exception is thrown if(target[0]) { diff --git a/source/renderbuffer.cpp b/source/renderbuffer.cpp index 89b25500..b2b25a08 100644 --- a/source/renderbuffer.cpp +++ b/source/renderbuffer.cpp @@ -20,7 +20,7 @@ Renderbuffer::~Renderbuffer() void Renderbuffer::storage(PixelFormat fmt, unsigned wd, unsigned ht) { require_pixelformat(fmt); - Bind _bind(this, true); + BindRestore _bind(this); width = wd; height = ht; glRenderbufferStorage(GL_RENDERBUFFER, fmt, width, height); @@ -31,7 +31,7 @@ void Renderbuffer::storage_multisample(unsigned samples, PixelFormat fmt, unsign static Require _req(EXT_framebuffer_multisample); require_pixelformat(fmt); - Bind _bind(this, true); + BindRestore _bind(this); width = wd; height = ht; glRenderbufferStorageMultisample(GL_RENDERBUFFER, samples, fmt, width, height); diff --git a/source/shadowmap.cpp b/source/shadowmap.cpp index 8151a7cf..21e67c38 100644 --- a/source/shadowmap.cpp +++ b/source/shadowmap.cpp @@ -104,7 +104,7 @@ void ShadowMap::setup_frame() const shadow_matrix.translate(-0.5, -0.5, depth_bias/size-0.5); shadow_matrix.invert(); - Bind bind_fbo(fbo, true); + BindRestore bind_fbo(fbo); Bind bind_depth(DepthTest::lequal()); fbo.clear(DEPTH_BUFFER_BIT); renderable.render("shadow"); diff --git a/source/texture2d.cpp b/source/texture2d.cpp index 7a9701f5..973afebf 100644 --- a/source/texture2d.cpp +++ b/source/texture2d.cpp @@ -46,7 +46,7 @@ void Texture2D::image(unsigned level, PixelFormat fmt, DataType type, const void unsigned h = height; get_level_size(level, w, h); - Bind _bind(this, true); + BindRestore _bind(this); glTexImage2D(target, level, ifmt, w, h, 0, fmt, type, data); allocated |= 1<