google / bigwheels

BigWheels is a cross-platform, API agnostic framework to build graphics applications.
Apache License 2.0
94 stars 35 forks source link

Geometry: split geometry class into static and mutable geometry #31

Open Keenuts opened 1 year ago

Keenuts commented 1 year ago

Current geometry (and tri_mesh) support appending new vertices/attributes, etc. This is useful when building a mesh on the fly, or when parsing some stream.

If https://github.com/google/bigwheels/issues/28 is accepted, buffer will be decoupled from the allocated memory block. Allowing import of GLTF meshes with multiple buffers using the same underlying allocation block, while still supporting those mutable geometry/buffers APIs could become complex as appending to a buffer will imply changing neighbor buffer offsets, and underlying allocation size.

For this reason, I'd suggest having 2 kind of buffer/geometry:

An alternative solution would be to keep the current geometry class, and handle it like the interleaved vs linear layout: return a failure in some cases. But it might be harder to maintain, and also become harder to use as we would need to know the layout, and how an object was created to use it correctly.

chaoticbob commented 1 year ago

The current Geometry class is uncessarily complicated. It was designed around the notion to enable benchmarking between interleaved and planar data when we cared about testing those things. We probably don't anymore. I don't know if we even need it to be dynamic. Right now Geometry acts as a proxy between TriMesh and Mesh so that it can configure that memory layouts in Mesh based on Geometry's configuration. The mutability thing was just a nice to have. Do we still need these?

For non-GLTF cases here's the configuration we care about: IndexBuffer [optional but recommended] PositionBuffer - position data only AttributesBuffer - vertex colors, normals, tangents, etc..

The main course (as in food) of the rendering is going to be PBR and the appetizer (as in food) is going to be simple debug. We know that this configuration is well suited for both desktop and mobile.

An alternative solution would be to keep the current geometry class, and handle it like the interleaved vs linear layout: return a failure in some cases. But it might be harder to maintain, and also become harder to use as we would need to know the layout, and how an object was created to use it correctly.

I've been wanting to replace the current Geometry class for a long time. The incoming geometry should be able to tell the mesh class how it wants the memory to be laid out.

Annoying distinction: Do you want to a single memory allocation or an offsetable buffer? (See #28).

I don't think it's unreasonable to accomodate what you're asking for. However, I would like to know if the ask is for generalized supporting of GLTF data or specifc case of GLTF? That is, do you want to be able to load in any configuration of a GLTF's geometry data or just the nominal/popular cases (e.g. ready to go PBR models)?

Keenuts commented 1 year ago

It was designed around the notion to enable benchmarking between interleaved and planar data when we cared about testing those things

@wangra-google do we still need it? I'd say we could probably do without, and having 2 meshes ready, and render one or the other and obtain the same effect no?

I'll expand my thought on #28 but it was more about allowing creating a mesh by giving buffers already set (and decoupling the memory from the buffer). This request was heavily biased toward GLTF, as gltf file format does that, but also because I have the impression than decoupling the memory from the buffer is useful (as vulkan does), but maybe the simplicity of BigWheels is better.

wangra-google commented 1 year ago

Yes, we still want to test the performance impact of interleaved and planar data in the benchmark. But as you said, it is not necessary to keep the current implementation of appending data since we know the layout for both cases.