CesiumGS / cesium-unreal

Bringing the 3D geospatial ecosystem to Unreal Engine
https://cesium.com/platform/cesium-for-unreal/
Apache License 2.0
886 stars 286 forks source link

Create RHI vertex and index buffers asynchronously #982

Open nithinp7 opened 1 year ago

nithinp7 commented 1 year ago

In #975, we implemented asynchronous RHI texture creation after discovering the RHIAsyncCreateTexture2D function in Unreal. Similarly, there exist RHIAsyncCreateIndexBuffer and RHIAsyncCreateVertexBuffer that seem to suggest we can do a similar optimization for index and vertex buffers. We may in the process even find ways to directly upload quantized buffers. With run-of-the mill tilesets, this optimization may not improve much - but high-quality photogrammetry tilesets (e.g., centimeter-precise models) will almost certainly bottleneck during render thread vertex buffer creation.

As with the RHI-level texture creation PR, we will have to determine how to "link" the Unreal user-level static mesh object to the RHI-level resource. There will probably be a bunch of edge cases and varying support across platforms, but generally I think this should be possible.

RaiaN commented 1 year ago

I thought my comment might be useful for your future work. Previously I dealt with partial Vertex & Index buffer uploads in UE4. Apparently UE RHI layer didn't really use offset for static buffers so I ended up modifying that part of RHI (D3D11 only) i.e. source code. Hopefully no more changes required these days to implement direct upload of quantized buffers.

kring commented 1 year ago

Thanks for the note @RaiaN! Just to be sure I'm understanding... you're saying you needed to modify the UE source code itself?

RaiaN commented 1 year ago

Hey @kring. Yeah, what I mean is that in case you wanted to partially update vertex/index buffer X in 4.2X, which is created by the engine itself (at PostLoad time), you would need to modify D3D11 Lock_ functions as Epic didn't implement support for partial updates of BUF_Static buffers.

However, I assume you would be using BUF_Dynamic instead of BUF_Static so there should be no issues with partial updates in this case.

csciguy8 commented 3 months ago

For reference, here's some of the latest related profiles in Unreal Insights... image from this PR.

Setting up the mesh and material look to be around half the time spent in ::LoadPrimitive, so if we can benefit from more asynchronous operation, fantastic.

Curiously, while we are using RHIAsyncCreateTexture2D for textures, it looks like we are using it in a function that waits for it to complete anyway? (kind of defeating the point) https://github.com/CesiumGS/cesium-unreal/blob/95e3b227fc4138f9f3307c40c74493f6a4bbb218/Source/CesiumRuntime/Private/CesiumTextureUtility.cpp#L34

kring commented 3 months ago

Curiously, while we are using RHIAsyncCreateTexture2D for textures, it looks like we are using it in a function that waits for it to complete anyway? (kind of defeating the point)

Waiting in a worker thread is much better than waiting in the game thread.

But in old versions of UE, the wait was "hidden", so it's possible we didn't even realize it was happening. It'd be interesting to explore whether we can eliminate it. But it doesn't defeat the point, in any case.

csciguy8 commented 3 months ago

Waiting in a worker thread is much better than waiting in the game thread.... But it doesn't defeat the point, in any case.

Ah, I think I see.

So using RHIAsyncCreateTexture2D allows for a completion event later (which we don't currently take advantage of), but the other benefit is it can be called from a worker thread, where RHICreateTexture2D cannot?

Apologies for the naive questions, the UE docs on these calls are a bit sparse, and equally sparse.

kring commented 3 months ago

but the other benefit is it can be called from a worker thread, where RHICreateTexture2D cannot?

Right. It is my understanding that all of the RHI* functions, unless explicitly documented otherwise, can only be called from the render thread.