From 8f6ff99136d1c47745bd62ce3f97b6abbf4ef7e7 Mon Sep 17 00:00:00 2001 From: falsycat Date: Thu, 3 Nov 2022 10:59:26 +0900 Subject: [PATCH] improve ugly codes in locking OpenGL objects --- common/gl_obj.cc | 89 +++++++++++++++++++++++------------------------- common/gl_obj.hh | 53 +++++++++++++++------------- file/gl_obj.cc | 69 +++++++++++++------------------------ 3 files changed, 96 insertions(+), 115 deletions(-) diff --git a/common/gl_obj.cc b/common/gl_obj.cc index ed71a03..f58e56f 100644 --- a/common/gl_obj.cc +++ b/common/gl_obj.cc @@ -19,9 +19,9 @@ namespace nf7::gl { template -auto LockAndValidate(const std::shared_ptr& ctx, - nf7::AsyncFactory>>& factory, - std::function&& validator) noexcept { +auto LockAndValidate(const std::shared_ptr& ctx, + typename T::Factory& factory, + std::function&& validator) noexcept { typename nf7::Future>>::Promise pro {ctx}; factory.Create().Chain(pro, [validator](auto& v) { validator(**v); @@ -30,7 +30,7 @@ auto LockAndValidate(const std::shared_ptr& return pro.future(); } static auto LockAndValidate(const std::shared_ptr& ctx, - nf7::gl::BufferFactory& factory, + nf7::gl::Buffer::Factory& factory, nf7::gl::BufferTarget target, size_t required) noexcept { return LockAndValidate(ctx, factory, [target, required](auto& buf) { @@ -46,7 +46,7 @@ static auto LockAndValidate(const std::shared_ptr& ctx, }); } static auto LockAndValidate(const std::shared_ptr& ctx, - nf7::gl::TextureFactory& factory, + nf7::gl::Texture::Factory& factory, nf7::gl::TextureTarget target) noexcept { return LockAndValidate(ctx, factory, [target](auto& tex) { if (tex.meta().target != target) { @@ -148,7 +148,7 @@ nf7::Future>> Obj_ProgramMeta::Create( std::vector>>> shs; for (auto shader : shaders) { shs.emplace_back(ctx->env().GetFileOrThrow(shader). - interfaceOrThrow().Create()); + interfaceOrThrow().Create()); apro.Add(shs.back()); } @@ -212,21 +212,20 @@ try { } nf7::Future>>::Promise pro {ctx}; - LockBuffers(ctx).Chain( + LockAttachments(ctx).Chain( nf7::Env::kGL, ctx, pro, [*this, ctx, pro](auto& bufs) mutable { // check all buffers if (index) { - assert(bufs.size() == attrs.size()+1); - const auto& m = (*bufs.back().value()).meta(); + assert(bufs.index); + const auto& m = (***bufs.index).meta(); if (m.target != gl::BufferTarget::ElementArray) { throw nf7::Exception {"index buffer is not ElementArray"}; } - } else { - assert(bufs.size() == attrs.size()); } + assert(bufs.attrs.size() == attrs.size()); for (size_t i = 0; i < attrs.size(); ++i) { - if ((*bufs[i].value()).meta().target != gl::BufferTarget::Array) { + if ((**bufs.attrs[i]).meta().target != gl::BufferTarget::Array) { throw nf7::Exception {"buffer is not Array"}; } } @@ -236,7 +235,7 @@ try { glBindVertexArray(id); for (size_t i = 0; i < attrs.size(); ++i) { const auto& attr = attrs[i]; - const auto& buf = *bufs[i].value(); + const auto& buf = **bufs.attrs[i]; glBindBuffer(GL_ARRAY_BUFFER, buf.id()); glEnableVertexAttribArray(attr.location); @@ -250,8 +249,7 @@ try { reinterpret_cast(static_cast(attr.offset))); } if (index) { - const auto& buf = *bufs.back().value(); - glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, buf.id()); + glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, (***bufs.index).id()); } glBindBuffer(GL_ARRAY_BUFFER, 0); glBindVertexArray(0); @@ -264,17 +262,27 @@ try { return {std::current_exception()}; } -Obj_VertexArrayMeta::LockedBuffersFuture Obj_VertexArrayMeta::LockBuffers( +Obj_VertexArrayMeta::LockedAttachmentsFuture Obj_VertexArrayMeta::LockAttachments( const std::shared_ptr& ctx, const ValidationHint& vhint) const noexcept try { + const auto Lock = [&](nf7::File::Id id, gl::BufferTarget target, size_t req) { + auto& factory = ctx->env(). + GetFileOrThrow(id).interfaceOrThrow(); + return LockAndValidate(ctx, factory, target, req); + }; + nf7::AggregatePromise apro {ctx}; - std::vector fus; - fus.reserve(1+attrs.size()); + auto ret = std::make_shared(); + LockedAttachmentsFuture::Promise pro {ctx}; // lock array buffers - for (const auto& attr : attrs) { + std::vector attrs_fu; + attrs_fu.reserve(attrs.size()); + for (size_t i = 0; i < attrs.size(); ++i) { + const auto& attr = attrs[i]; + const size_t required = // when non-instanced and no-index-buffer drawing attr.divisor == 0 && vhint.vertices > 0 && !index? @@ -284,35 +292,24 @@ try { static_cast(attr.stride) * (vhint.instances-1) + attr.offset: size_t {0}; - auto& factory = ctx->env(). - GetFileOrThrow(attr.buffer). - interfaceOrThrow(); - auto fu = LockAndValidate(ctx, factory, gl::BufferTarget::Array, required); - apro.Add(fu); - fus.emplace_back(fu); + attrs_fu.push_back(Lock(attr.buffer, gl::BufferTarget::Array, required)); + apro.Add(attrs_fu.back()); } // lock index buffers (it must be the last element in `fus`) if (index) { const auto required = gl::GetByteSize(index->numtype) * vhint.vertices; - - auto& factory = ctx->env(). - GetFileOrThrow(index->buffer). - interfaceOrThrow(); - auto fu = LockAndValidate(ctx, factory, gl::BufferTarget::ElementArray, required); - apro.Add(fu); - fus.emplace_back(fu); + apro.Add(Lock(index->buffer, gl::BufferTarget::ElementArray, required). + Chain(pro, [ret](auto& buf) { ret->index = buf; })); } - // wait for all registered futures - nf7::Future>>>::Promise pro {ctx}; - apro.future().Chain(pro, [fus = std::move(fus)](auto&) { - std::vector>> ret; - ret.reserve(fus.size()); - for (auto& fu : fus) { - ret.emplace_back(fu.value()); + // return ret + apro.future().Chain(pro, [ret, attrs_fu = std::move(attrs_fu)](auto&) { + ret->attrs.reserve(attrs_fu.size()); + for (auto& fu : attrs_fu) { + ret->attrs.push_back(fu.value()); } - return ret; + return std::move(*ret); }); return pro.future(); } catch (nf7::Exception&) { @@ -371,34 +368,34 @@ try { auto ret = std::make_shared(); nf7::AggregatePromise apro {ctx}; + LockedAttachmentsFuture::Promise pro {ctx}; - const auto lock = [&](nf7::File::Id id) { + const auto Lock = [&](nf7::File::Id id) { auto& factory = ctx->env(). GetFileOrThrow(id). - interfaceOrThrow(); + interfaceOrThrow(); return LockAndValidate(ctx, factory, gl::TextureTarget::Tex2D); }; for (size_t i = 0; i < colors.size(); ++i) { const auto& color = colors[i]; if (color && color->tex) { - apro.Add(lock(color->tex).ThenIf([i, ret](auto& res) { + apro.Add(Lock(color->tex).Chain(pro, [i, ret](auto& res) { ret->colors[i] = res; })); } } if (depth && depth->tex) { - apro.Add(lock(depth->tex).ThenIf([ret](auto& res) { + apro.Add(Lock(depth->tex).Chain(pro, [ret](auto& res) { ret->depth = res; })); } if (stencil && stencil->tex) { - apro.Add(lock(stencil->tex).ThenIf([ret](auto& res) { + apro.Add(Lock(stencil->tex).Chain(pro, [ret](auto& res) { ret->stencil = res; })); } - LockedAttachmentsFuture::Promise pro {ctx}; apro.future().Chain(pro, [ret](auto&) { return std::move(*ret); }); return pro.future(); } catch (nf7::Exception&) { diff --git a/common/gl_obj.hh b/common/gl_obj.hh index 681f311..7731685 100644 --- a/common/gl_obj.hh +++ b/common/gl_obj.hh @@ -22,8 +22,9 @@ namespace nf7::gl { template class Obj final { public: - using Meta = T; - using Param = typename Meta::Param; + using Meta = T; + using Param = typename Meta::Param; + using Factory = nf7::AsyncFactory>>>; Obj(const std::shared_ptr& ctx, GLuint id, const Meta& meta) noexcept : ctx_(ctx), id_(id), meta_(meta) { @@ -60,14 +61,12 @@ struct Obj_BufferMeta final { glDeleteBuffers(1, &id); } - // must be called from main or sub task nf7::Future>> Create( const std::shared_ptr& ctx) const noexcept; gl::BufferTarget target; }; -using Buffer = Obj; -using BufferFactory = AsyncFactory>>; +using Buffer = Obj; struct Obj_TextureMeta final { @@ -78,7 +77,6 @@ struct Obj_TextureMeta final { glDeleteTextures(1, &id); } - // must be called from main or sub task nf7::Future>> Create( const std::shared_ptr& ctx) const noexcept; @@ -86,8 +84,7 @@ struct Obj_TextureMeta final { gl::InternalFormat format; std::array size; }; -using Texture = Obj; -using TextureFactory = AsyncFactory>>; +using Texture = Obj; struct Obj_ShaderMeta final { @@ -98,7 +95,6 @@ struct Obj_ShaderMeta final { glDeleteShader(id); } - // must be called from main or sub task nf7::Future>> Create( const std::shared_ptr& ctx, const std::string& src) const noexcept; @@ -106,7 +102,6 @@ struct Obj_ShaderMeta final { gl::ShaderType type; }; using Shader = Obj; -using ShaderFactory = AsyncFactory>>; struct Obj_ProgramMeta final { @@ -124,7 +119,6 @@ struct Obj_ProgramMeta final { glDeleteProgram(id); } - // must be called from main or sub task nf7::Future>> Create( const std::shared_ptr& ctx, const std::vector& shaders) noexcept; @@ -135,15 +129,17 @@ struct Obj_ProgramMeta final { std::optional depth; }; using Program = Obj; -using ProgramFactory = AsyncFactory>>; struct Obj_VertexArrayMeta final { public: struct Param { }; - using LockedBuffersFuture = - nf7::Future>>>; + struct LockedAttachments { + std::optional>> index; + std::vector>> attrs; + }; + using LockedAttachmentsFuture = nf7::Future; struct Index { nf7::File::Id buffer; @@ -161,8 +157,6 @@ struct Obj_VertexArrayMeta final { }; struct ValidationHint { - ValidationHint() noexcept { } - size_t vertices = 0; size_t instances = 0; }; @@ -171,21 +165,18 @@ struct Obj_VertexArrayMeta final { glDeleteVertexArrays(1, &id); } - // must be called from main or sub task nf7::Future>> Create( const std::shared_ptr& ctx) const noexcept; - // must be called from main or sub task // it's guaranteed that the last element of the returned vector is an index buffer if index != std::nullopt - LockedBuffersFuture LockBuffers( + LockedAttachmentsFuture LockAttachments( const std::shared_ptr& ctx, - const ValidationHint& vhint = {}) const noexcept; + const ValidationHint& vhint = {0, 0}) const noexcept; std::optional index; std::vector attrs; }; using VertexArray = Obj; -using VertexArrayFactory = AsyncFactory>>; struct Obj_FramebufferMeta final { @@ -212,7 +203,6 @@ struct Obj_FramebufferMeta final { glDeleteFramebuffers(1, &id); } - // must be called from main or sub task nf7::Future>> Create( const std::shared_ptr& ctx) const noexcept; @@ -224,6 +214,23 @@ struct Obj_FramebufferMeta final { std::optional stencil; }; using Framebuffer = Obj; -using FramebufferFactory = AsyncFactory>>; + + + +// acquires locks of the object and all of its attachments +template +auto LockRecursively(typename T::Factory& factory, + const std::shared_ptr& ctx, + Args&&... args) noexcept { + typename nf7::Future>, + typename T::Meta::LockedAttachments>>::Promise pro {ctx}; + factory.Create().Chain(pro, [=, ...args = std::forward(args)](auto& obj) mutable { + (**obj).meta().LockAttachments(ctx, std::forward(args)...). + Chain(pro, [obj](auto& att) { + return std::make_pair(obj, att); + }); + }); + return pro.future(); +} } // namespace nf7::gl diff --git a/file/gl_obj.cc b/file/gl_obj.cc index a69b0ce..b95c96c 100644 --- a/file/gl_obj.cc +++ b/file/gl_obj.cc @@ -911,66 +911,41 @@ struct Program { nf7::AggregatePromise apro {p.la}; // find, fetch and lock FBO - std::optional fbo_fu; - std::optional fbo_lock_fu; - { - fbo_fu = v.tuple("fbo").file(base). - interfaceOrThrow().Create(); - - nf7::gl::Framebuffer::Meta::LockedAttachmentsFuture::Promise fbo_lock_pro; - fbo_fu->ThenIf([la = p.la, fbo_lock_pro](auto& fbo) mutable { - (**fbo).meta().LockAttachments(la).Chain(fbo_lock_pro); - }); - - fbo_lock_fu = fbo_lock_pro.future(); - apro.Add(*fbo_lock_fu); - } + auto fbo_fu = gl::LockRecursively( + v.tuple("fbo"). + file(base). + interfaceOrThrow(), + p.la); + apro.Add(fbo_fu); // find, fetch and lock VAO - std::optional vao_fu; - std::optional vao_lock_fu; - { - vao_fu = v.tuple("vao").file(base). - interfaceOrThrow().Create(); - - nf7::gl::VertexArray::Meta::ValidationHint vhint; - vhint.vertices = static_cast(count); - vhint.instances = static_cast(inst); - - nf7::gl::VertexArray::Meta::LockedBuffersFuture::Promise vao_lock_pro; - vao_fu->ThenIf([la = p.la, vao_lock_pro, vhint](auto& vao) mutable { - (**vao).meta().LockBuffers(la, vhint).Chain(vao_lock_pro); - }); - - vao_lock_fu = vao_lock_pro.future(); - apro.Add(*vao_lock_fu); - } + auto vao_fu = gl::LockRecursively( + v.tuple("vao"). + file(base). + interfaceOrThrow(), + p.la, + gl::VertexArray::Meta::ValidationHint { + .vertices = static_cast(count), + .instances = static_cast(inst), + }); + apro.Add(vao_fu); // find, fetch and lock textures - std::vector> tex_fu; + std::vector> tex_fu; tex_fu.reserve(tex->size()); for (auto& pa : *tex) { auto fu = base. ResolveOrThrow(pa.second.string()). - interfaceOrThrow(). + interfaceOrThrow(). Create(); tex_fu.emplace_back(pa.first, fu); apro.Add(fu); } // execute drawing after successful locking - apro.future().Then(nf7::Env::kGL, p.la, [=, tex_fu = std::move(tex_fu)](auto&) { - assert(fbo_lock_fu); - assert(vao_lock_fu); - try { - if (fbo_lock_fu->error()) fbo_lock_fu->value(); - if (vao_lock_fu->error()) vao_lock_fu->value(); - } catch (nf7::Exception&) { - p.log->Error("failed to acquire lock of VAO or FBO"); - return; - } - const auto& fbo = *fbo_fu->value(); - const auto& vao = *vao_fu->value(); + apro.future().ThenIf(nf7::Env::kGL, p.la, [=, tex_fu = std::move(tex_fu)](auto&) { + const auto& fbo = *fbo_fu.value().first; + const auto& vao = *vao_fu.value().first; const auto& prog = *p.obj; // bind objects @@ -1027,6 +1002,8 @@ struct Program { if (status != GL_FRAMEBUFFER_COMPLETE) { p.log->Warn("framebuffer is broken"); } + }).Catch([p](auto& e) { + p.log->Error(e); }); return false; } else {