[Fix] (MG_Impl/Framebuffer, MG_State/Framebuffer, MG_Backend/DirectGLES): redone fbo attachment management. remap GL -> ES attachment names

MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp

@@ -137,15 +137,23 @@ namespace MobileGL::MG_Backend::DirectGLES {
 
     namespace FramebufferImpl {
         void SyncCurrentFBO() {
-            auto currentFBO = MG_State::pGLContext->GetFramebufferBindingSlot(FramebufferTarget::Draw).GetBoundObject();
+            const FramebufferTarget fboTargets[] = {FramebufferTarget::Draw, FramebufferTarget::Read};
+
+            MG_State::GLState::FramebufferObject* lastUpdatedFBO = nullptr;
+
+            for (auto target: fboTargets) {
+                auto currentFBO =
+                        MG_State::pGLContext->GetFramebufferBindingSlot(target).GetBoundObject();
+
                 if (!currentFBO) {
                     MGLOG_E("No FBO is currently bound, cannot sync current FBO.");
-                return;
+                    continue;
                 }
 
-            if (currentFBO == MG_Impl::GLImpl::FramebufferImpl::pDefaultFramebufferInfo->defaultFBO) {
+                if (currentFBO ==
+                    MG_Impl::GLImpl::FramebufferImpl::pDefaultFramebufferInfo->defaultFBO) {
                     // Default FBO, nothing to sync
-                return;
+                    continue;
                 }
 
                 const auto &backendFBOIt = g_backendFramebufferObjects.find(currentFBO);
@@ -156,7 +164,17 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 } else {
                     backendFBOObject = backendFBOIt->second;
                 }
-            backendFBOObject->SyncToBackend(currentFBO);
+
+                if (currentFBO.get() == lastUpdatedFBO) {
+                    MGLOG_I("Draw FBO and read FBO are the same, skipping sync.");
+                } else {
+                    backendFBOObject->SyncToBackend(currentFBO, target);
+                }
+
+                backendFBOObject->Bind(target);
+
+                lastUpdatedFBO = currentFBO.get();
+            }
         }
     } // namespace FramebufferImpl
 

MobileGL/MG_Backend/DirectGLES/Managers.cpp

@@ -4,6 +4,7 @@
 #include "MG_Util/Types.h"
 #include "Utils.h"
 #include "DirectGLES.h"
+#include "MG_Util/Converters/GLToMG/FramebufferEnumConverter.h"
 #include <MG_Util/BackendLoaders/OpenGL/Loader.h>
 #include <MG_Util/Converters/GLToStr/GLEnumConverter.h>
 #include <MG_Util/Converters/MGToGL/DataTypeConverter.h>
@@ -383,25 +384,64 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 MG_External::GLES::glBindFramebuffer(GL_DRAW_FRAMEBUFFER, m_backendFBOId);
         }
 
-        void BackendFramebufferObject::SyncToBackend(SharedPtr<MG_State::GLState::FramebufferObject>& stateFBOObject) {
+        void BackendFramebufferObject::SyncToBackend(SharedPtr<MG_State::GLState::FramebufferObject>& stateFBOObject, FramebufferTarget asTarget) {
             if (!stateFBOObject) {
                 MGLOG_E("State FBO object is null, cannot sync to backend.");
                 return;
             }
 
-            MGLOG_D("Syncing FBO object with ID: %u to backend for state: %p", m_backendFBOId, stateFBOObject.get());
+            MGLOG_D("Syncing FBO object with ID: %u to backend for state: %p, as %s FBO", m_backendFBOId, stateFBOObject.get(),
+                    (asTarget == FramebufferTarget::Draw ? "DRAW" : "READ"));
 
-            BackendFramebufferBindingProtector backendFBOBindingProtector(GL_FRAMEBUFFER);
+            GLenum glFBOTarget = MG_Util::ConvertFramebufferTargetToGLEnum(asTarget);
-            // TODO: do i really need to bind here?
+            BackendFramebufferBindingProtector backendFBOBindingProtector(glFBOTarget);
-            Bind(FramebufferTarget::Read);
+
-            Bind(FramebufferTarget::Draw);
+            Bind(asTarget);
+
+            if (asTarget == FramebufferTarget::Draw) {
+                int nBuffers = 0;
+                // Attach attachment to FBO, realize `glDrawBuffers`
+                if (stateFBOObject->DrawBuffersIsDirty()) {
+                    std::fill(m_frontendBuffers,
+                              m_frontendBuffers + MG_State::GLState::FramebufferObject::MAX_DRAW_BUFFERS,
+                              GL_NONE);
+                    std::fill(m_backendBuffers,
+                              m_backendBuffers + MG_State::GLState::FramebufferObject::MAX_DRAW_BUFFERS,
+                              GL_NONE);
+                    auto &stateDrawBuffers = stateFBOObject->GetDrawBuffers();
+                    for (GLint i = 0; i < stateDrawBuffers.size(); ++i) {
+                        m_frontendBuffers[i] =
+                                MG_Util::ConvertFramebufferAttachmentTypeToGLEnum(stateDrawBuffers[i]);
+                    }
+
+                    MGLOG_D("%s: mapping draw buffers gl -> es:", __func__);
+                    for (int i = 0;
+                         i < MG_State::GLState::FramebufferObject::MAX_DRAW_BUFFERS; ++i) {
+                        if (m_frontendBuffers[i] == GL_NONE) {
+                            MGLOG_D("(m_frontendBuffers[%d] = GL_NONE), skipped", i);
+                            continue;
+                        }
+                        m_backendBuffers[nBuffers] = GL_COLOR_ATTACHMENT0 + nBuffers;
+                        m_compactedFrontendBuffers[nBuffers] = m_frontendBuffers[i];
+                        MGLOG_D("(m_frontendBuffers[%d] = %s) -> %s",
+                                i, MG_Util::ConvertGLEnumToString(m_frontendBuffers[i]).c_str(),
+                                MG_Util::ConvertGLEnumToString(m_backendBuffers[nBuffers]).c_str());
+                        nBuffers++;
+                    }
+
+                    MG_External::GLES::glDrawBuffers(nBuffers, m_backendBuffers);
+
+                    stateFBOObject->ClearDrawBuffersDirtyState();
+                }
 
-            // TODO: add dirty check
                 // Sync all attachments
                 const auto& attachments = stateFBOObject->GetAllAttachments();
-            for (SizeT index = 0; index < attachments.size(); ++index) {
+                for (SizeT i = 0; i < nBuffers; ++i) {
-                FramebufferAttachmentType attachmentType = static_cast<FramebufferAttachmentType>(index);
+                    FramebufferAttachmentType frontendAttachmentType =
-                const auto& attachment = attachments[index];
+                            MG_Util::ConvertGLEnumToFramebufferAttachmentType(m_compactedFrontendBuffers[i]);
+                    GLenum glBackendAttachmentType = m_backendBuffers[i];
+
+                    const auto& attachment = attachments[(SizeT)frontendAttachmentType];
                     if (!attachment.IsComplete()) {
                         continue;
                     }
@@ -414,31 +454,57 @@ namespace MobileGL::MG_Backend::DirectGLES {
                             continue;
                         }
                         const auto& backendTextureObject = backendTextureIt->second;
-                    backendTextureObject->Bind(MG_Util::ConvertTextureTargetToGLEnum(textureObject->GetTarget()));
+                        auto glTextureTarget = MG_Util::ConvertTextureTargetToGLEnum(textureObject->GetTarget());
+                        backendTextureObject->Bind(glTextureTarget);
                         MG_External::GLES::glFramebufferTexture2D(
-                        GL_FRAMEBUFFER, MG_Util::ConvertFramebufferAttachmentTypeToGLEnum(attachmentType),
+                                glFBOTarget, glBackendAttachmentType, glTextureTarget,
-                        MG_Util::ConvertTextureTargetToGLEnum(textureObject->GetTarget()),
                                 backendTextureObject->GetBackendTextureId(), static_cast<GLint>(attachment.GetTextureLevel()));
                     } else if (attachment.IsRenderbuffer()) {
                         // TODO
                     }
                 }
-
+            } else {
-            if (stateFBOObject->DrawBuffersIsDirty()) {
+                // Attach attachment to FBO, realize `glReadBuffer`
-                static GLenum drawbufs[MG_State::GLState::FramebufferObject::MAX_DRAW_BUFFERS];
+                // TODO: do we actually need "virtualization" here? I assume not?
-                std::fill(drawbufs, drawbufs + MG_State::GLState::FramebufferObject::MAX_DRAW_BUFFERS, GL_NONE);
+                GLenum frontendAtt = MG_Util::ConvertFramebufferAttachmentTypeToGLEnum(stateFBOObject->GetReadBuffer());
-                auto& stateDrawBuffers = stateFBOObject->GetDrawBuffers();
+                GLenum backendAtt = GL_NONE;
-                GLint i = 0;
+                // Attach attachment to FBO, fast path
-                for (; i < stateDrawBuffers.size(); ++i) {
+                for (SizeT i = 0; i < MG_State::GLState::FramebufferObject::MAX_DRAW_BUFFERS; ++i) {
-                    if (stateDrawBuffers[i] == FramebufferAttachmentType::None) {
+                    if (m_compactedFrontendBuffers[i] == frontendAtt)
-                        break;
+                        backendAtt = m_backendBuffers[i];
                 }
-                    drawbufs[i] = MG_Util::ConvertFramebufferAttachmentTypeToGLEnum(stateDrawBuffers[i]);
+                if (backendAtt != GL_NONE) {
+                    MG_External::GLES::glReadBuffer(backendAtt);
+                } else {
+                    // Maybe we should properly do `glFramebufferTexture2D` here
+                    // don't need remap / virtualization
+                    MG_External::GLES::glReadBuffer(frontendAtt);
+                    const auto &attachments = stateFBOObject->GetAllAttachments();
+                    const auto &attachment = attachments[(SizeT) frontendAtt];
+                    if (!attachment.IsComplete()) {
+                        return;
                     }
 
-                MG_External::GLES::glDrawBuffers(i, drawbufs);
+                    if (attachment.IsTexture()) {
-
+                        const auto &textureObject = attachment.GetTexture();
-                stateFBOObject->ClearDrawBuffersDirtyState();
+                        const auto &backendTextureIt =
+                                TextureImpl::g_backendTextureObjects.find(textureObject);
+                        if (backendTextureIt == TextureImpl::g_backendTextureObjects.end()) {
+                            MGLOG_E("No backend texture found for FBO attachment, cannot bind texture.");
+                            return;
+                        }
+                        const auto &backendTextureObject = backendTextureIt->second;
+                        auto glTextureTarget = MG_Util::ConvertTextureTargetToGLEnum(
+                                textureObject->GetTarget());
+                        backendTextureObject->Bind(glTextureTarget);
+                        MG_External::GLES::glFramebufferTexture2D(
+                                glFBOTarget, frontendAtt, glTextureTarget,
+                                backendTextureObject->GetBackendTextureId(),
+                                static_cast<GLint>(attachment.GetTextureLevel()));
+                    } else if (attachment.IsRenderbuffer()) {
+                        // TODO
+                    }
+                }
             }
         }
 

MobileGL/MG_Backend/DirectGLES/Managers.h

@@ -83,12 +83,32 @@ namespace MobileGL::MG_Backend::DirectGLES {
         class BackendFramebufferObject {
         public:
             BackendFramebufferObject();
-            void SyncToBackend(SharedPtr<MG_State::GLState::FramebufferObject>& stateFBOObject);
+            void SyncToBackend(SharedPtr<MG_State::GLState::FramebufferObject>& stateFBOObject, FramebufferTarget asTarget);
             Uint GetBackendFramebufferId() { return m_backendFBOId; }
             void Bind(FramebufferTarget target);
 
         private:
             Uint m_backendFBOId = 0;
+
+            /* this will save buffers in its original form,
+               reversion, absence or not consecutive are all allowed, as long as GL spec allows it
+               i.e. it could be like [COLOR_ATTACHMENT0, COLOR_ATTACHMENT5, NONE, COLOR_ATTACHMENT4]
+               Probably useful to re-link shader output according to this.
+               aka. realizing `glBindFragDataLocation`
+             */
+            GLenum m_frontendBuffers[MG_State::GLState::FramebufferObject::MAX_DRAW_BUFFERS] = {GL_NONE};
+            /* this will save buffers in its compacted GL form,
+               not consecutive is not allowed
+               i.e. it could be like [COLOR_ATTACHMENT0, COLOR_ATTACHMENT5, COLOR_ATTACHMENT4]
+               (no GL_NONE among those)
+             */
+            GLenum m_compactedFrontendBuffers[MG_State::GLState::FramebufferObject::MAX_DRAW_BUFFERS] = {GL_NONE};
+            /* this will save buffers in stricter ES rules
+               reversion, absence or not consecutive are not allowed, according to ES spec
+               i.e. it could be like [COLOR_ATTACHMENT0, COLOR_ATTACHMENT1, NONE, NONE, ...]
+               this array could be provided as data directly to ES `glDrawBuffers` function
+             */
+            GLenum m_backendBuffers[MG_State::GLState::FramebufferObject::MAX_DRAW_BUFFERS] = {GL_NONE};
         };
 
         extern UnorderedMap<SharedPtr<MG_State::GLState::FramebufferObject>, SharedPtr<BackendFramebufferObject>>

MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.cpp

@@ -67,23 +67,24 @@ namespace MobileGL {
                 m_externalIndex(externalIndex) {
                 m_attachments.fill(FramebufferAttachment(false));
                 m_drawBuffers.fill(FramebufferAttachmentType::None);
+                m_drawBuffers[0] = FramebufferAttachmentType::Color0;
             }
 
             void FramebufferObject::AttachTexture(FramebufferAttachmentType type, SharedPtr<ITextureObject> texture,
                                                   int level) {
                 m_attachments[static_cast<SizeT>(type)] = FramebufferAttachment(std::move(texture), level);
-//                m_drawBuffersDirty = true;
+                m_drawBuffersDirty = true;
             }
 
             void FramebufferObject::AttachRenderbuffer(FramebufferAttachmentType type,
                                                        std::shared_ptr<RenderbufferObjectStub> renderbuffer) {
                 m_attachments[static_cast<SizeT>(type)] = FramebufferAttachment(renderbuffer);
-//                m_drawBuffersDirty = true;
+                m_drawBuffersDirty = true;
             }
 
             void FramebufferObject::Detach(FramebufferAttachmentType type) {
                 m_attachments[static_cast<SizeT>(type)] = FramebufferAttachment(false);
-//                m_drawBuffersDirty = true;
+                m_drawBuffersDirty = true;
             }
 
             const FramebufferAttachment& FramebufferObject::GetAttachment(FramebufferAttachmentType type) const {

MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.h

@@ -103,11 +103,12 @@ namespace MobileGL {
                             static_cast<SizeT>(FramebufferAttachmentType::FramebufferAttachmentTypeCount)>&
                 GetAllAttachments() const;
                 Bool CheckCompleteness() const;
-//                void SetDrawBuffers(const Vector<FramebufferAttachmentType>& buffers);
+                // aka. `buffer` as in glDrawBuffers/glReadBuffers
                 void SetDrawBuffer(Uint index, FramebufferAttachmentType buffer);
                 bool DrawBuffersIsDirty() const { return m_drawBuffersDirty; }
                 void ClearDrawBuffersDirtyState() { m_drawBuffersDirty = false; }
                 const Array<FramebufferAttachmentType, MAX_DRAW_BUFFERS>& GetDrawBuffers() const;
+                FramebufferAttachmentType GetReadBuffer() const { return m_readBuffer; }
                 Uint GetExternalIndex() const;
 
             private:
@@ -117,6 +118,7 @@ namespace MobileGL {
                     m_attachments;
                 Bool m_drawBuffersDirty = false;
                 Array<FramebufferAttachmentType, MAX_DRAW_BUFFERS> m_drawBuffers;
+                FramebufferAttachmentType m_readBuffer = FramebufferAttachmentType::None;
             };
 
         } // namespace GLState

MobileGL/MG_Util/Converters/MGToGL/FramebufferEnumConverter.cpp

@@ -26,7 +26,7 @@ namespace MobileGL {
             case FramebufferAttachmentType::Stencil:
                 return GL_STENCIL_ATTACHMENT;
             default:
-                return GL_COLOR_ATTACHMENT0;
+                return GL_NONE;
             }
         }