bkaradzic / bgfx

Cross-platform, graphics API agnostic, "Bring Your Own Engine/Framework" style rendering library.
https://bkaradzic.github.io/bgfx/overview.html
BSD 2-Clause "Simplified" License
15.02k stars 1.95k forks source link

Proper VBO streaming for OpenGL #1119

Open jimon opened 7 years ago

jimon commented 7 years ago

(creating meta issue to track progress, instead of random PR's and chat)

So in #706 we added a hotfix to VBO streaming, because naive implementation is stalling pipeline. But apparently just recreating buffers is not really an orthodox way to do it (I've checked original link that goes to here https://gamedev.stackexchange.com/questions/87074/for-vertex-buffer-steaming-multiple-glbuffersubdata-vs-orphaning and also tried googling some other solutions, no one is ever mentioning recreating buffers every frame).

In https://github.com/bkaradzic/bgfx/pull/1118 we tried to hot fix it, but doesn't look like it's a proper fix. Also we've tried doing suggested fixes with adding orphaning to workarounds here https://github.com/jimon/bgfx/commit/f2546b3171017036dd1cc3492fa741d6d64576a1 but looks like it's not a proper fix, because how do we know which way would be faster?

I would suggest to revisit how #706 was fixed instead, and do double buffering of transient buffers instead. Sure proper async streaming everywhere would be great, but a good pragmatic solution now is also awesome :)

attilaz commented 7 years ago

FYI. our "solution" is this. We shipped games with this, but I don't claim that this is the perfect solution. I wonder how well it works on your devices.

void update(uint32_t _offset, uint32_t _size, void* _data)
        {
            BX_CHECK(0 != m_id, "Updating invalid vertex buffer.");
            GL_CHECK(glBindBuffer(m_target, m_id) );

#if BX_PLATFORM_ANDROID
            GL_CHECK(glBufferData(m_target
                , _size
                , NULL
                , GL_DYNAMIC_DRAW
                ) );
#endif

            GL_CHECK(glBufferSubData(m_target
                , _offset
                , _size
                , _data
                ) );
            GL_CHECK(glBindBuffer(m_target, 0) );
        }

not the one that is in bkaradzic/bgfx:

        void update(uint32_t _offset, uint32_t _size, void* _data, bool _discard = false)
        {
            BX_CHECK(0 != m_id, "Updating invalid index buffer.");

            if (_discard)
            {
                // orphan buffer...
                destroy();
                create(m_size, NULL, m_flags);
            }

            GL_CHECK(glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, m_id) );
            GL_CHECK(glBufferSubData(GL_ELEMENT_ARRAY_BUFFER
                , _offset
                , _size
                , _data
                ) );
            GL_CHECK(glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0) );
        }

we are not using the discard flag because we are using an older bgfx, but that could be added so glBufferData is only called when _discard is true.