Import native vertex/index buffers into buffer objects
Is your feature request related to a problem? Please describe. We have been using Filament in our CAD app for PBR and reusing the (native) vertex and index buffers between our renderer and Filament deemed impossible without changing Filament. We would like to propose extending the Filament driver API with importing native buffers into Filament buffer objects just like you can import native textures. This can come in handy for other projects which mix their own rendering engine with Filament's.
Describe the solution you'd like
Driver API
// 'id' is an id<MTLBuffer> bridged as void* in Metal, GLuint in OpenGL
DECL_DRIVER_API_R_N(backend::BufferObjectHandle, importBufferObject,
intptr_t, id,
backend::BufferObjectBinding, bindingType,
backend::BufferUsage, usage,
uint32_t, byteCount)
// setupExternalImage() has to be more generic to support buffers, not just texture images
// DECL_DRIVER_API_SYNCHRONOUS_N(void, setupExternalImage, void*, image)
DECL_DRIVER_API_SYNCHRONOUS_N(void, setupExternalResource, intptr_t, externalResource)
// This was never used, at least in the git revision we forked from. May be kept if necessary
//DECL_DRIVER_API_SYNCHRONOUS_N(void, cancelExternalImage, void*, image)
Before investing the time, I'd like to hear your opinion about this feature. If you are interested, I'd deliver it in (at least) 2 PRs, 1 PR per backend. Since our solution was based on a 2 year old Filament revision, I need time to prepare each.
Describe alternatives you've considered We considered attaching/wrapping an optional 'external buffer' to/in Filament vertex and index buffers (see here). This solution worked, but we weren't satisfied because we had to modify the buffer creation code, and callers would have to specify in advance if they are going to pass in external buffers or not.
OS and backend Our proposed solution is Metal and OpenGL only. We have zero Vulkan knowledge and no engineering budget to implement it in Vulkan (even though it probably is not so hard). The biggest question to us is: is it worth anything to you if we only add such a feature to one or two backends?
Quality? Our solution has been used by 40k daily users on iOS, macOS (with MacCatalyst) and Windows (via OpenGL ES3 backend) on various hardware configurations for 2 years. We never tried it on Android and I know that's your main platform. Therefore I think the OpenGL part would need more attention from your part than the Metal part (we have deeper knowledge in Metal anyway).
Meanwhile I found that you removed cancelExternalImage too
For illustration I added:
- a branch with the proposed API changes, but no implementation: https://github.com/google/filament/compare/main...shapr3d:filament:import_native_buffers
- another branch with a Metal implementation and a unit test: https://github.com/shapr3d/filament/compare/import_native_buffers...shapr3d:filament:import_native_buffers_mtl
Now adding a unit test was the hardest part. I may be doing something wrong but I faced several obstacles:
mainbranch didn't compile with Xcode 14.2, so my branches didn't either.-Wallends up after-Wno-conversion. I tweaked filament/backend/CMakeLists.txt to unblock myself. Is this a known issue, or am I doing sth wrong on my local dev setup?MTLGPUFamilyMac1,MTLResourceUsageSampleand one of theMTLRenderCommandEncoder::useResources()overloads got deprecated and I had to comment out some code.
- I was extending the
BackendTest::VertexBufferUpdateunit test and found that it fails when ran with the Metal backend onmainbranch because:- The
Programinstance has no name soMetalShaderCompiler::createProgram()runs onto a nullptr.p.diagnostics("whatevs", {});solves this, just wanted to let you know. - A driver purge was missing at the end of the test, so the dtor of the driver hit an assertion. Understandable since the scheduled callbacks were not drained by a
purge(). - Something is mixed up with the UBO in this test, so I completely removed it. My assumption is that both te VB and UBO are bound to slot 0 of the vertex shader, but I couldn't confirm this 😞 .
- The
My point is that I got the new unit test working on my machine, but I had to cut some nasty corners on main, to even run the existing ones, so I will need a little help with ironing out the above problems before it can land in a PR.