[Impeller] Support non-interleaved vertex data. #42415
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
| auto vertex_layout = descriptor.layouts[vertex_buffer_index]; | ||
| vertex_layout.stride = input.length; | ||
| vertex_layout.stepRate = 1u; | ||
| vertex_layout.stepFunction = MTLVertexStepFunctionPerVertex; |
There was a problem hiding this comment.
Can we research a design where the vertex layout is always provided to Impeller's HAL rather than having any implicit/hardcoded layouts enforced by the backends?
I think we'll inevitably need fine control over layout in some situations and I'm a bit concerned that adding a second hardcoded layout style will increase the difficulty of backing out of this.
For example, something like:
- The pipeline stores an attribute list.
- ImpellerC generates a default attribute list in the vertex stage structs with the staggered layout.
- This default layout is copied to the pipeline unless overridden in the ctor.
- When using a custom layout, the user is entirely responsible for packing the set of vertex buffers correctly and binding them on the raster Command. When not using a custom layout, they can safely use the generated header utilities and everything works fine.
|
I have something hacky that works OK for specifying basic layouts, and I see a path to making it less hacky... However, that still leaves a big open question with how we're caching pipelines. Constructing a new vertex description, and including that in the cache keys for the content_context variants seems like it will add a decent amount of overhead for basically no benefit most of the time - since of the pipelines we're caching we're likely only ever going to use a single variant. If we did need lots of different layouts, then I could make this a bit more efficient by having a secondary map where each vertex layout gets registered, and then is refered to by key/index in the pipeline cache. But that feels like overhead for what we need, so instead I'm thinking about making CreateDefaultPipeline accept a customization that applies everywhere. I do worry a bit about discovery though, so this also feels like it would be a bit magical. |
|
Or ... I guess I could essentially construct the vertex description as a static const and then do ptr comparisons. That would be cheap. |
|
actually I don't think this is a huge issue, as long as I keep the automagic interleaved vs non-interleaved logic out of renderer, content_context/pipeline cache can have a bit of magic still. |
|
ahh, no, its all in renderer lol. |
chinmaygarde
changed the title
…rs. (#42628) Working on #42415 , I found that it was difficult to customize the buffer layout as the interleaved layout is implicitly confiured in the backends. Rather than bake another built in layout, I've pulled (most) information about buffer layout into the generated headers so it is explicitly confiured, which should allow easier customization as the backend has fewer choices to make. TBD whether or not we need to do something weird for GLES since stride has a different meaning there... Work towards flutter/flutter#116168
jonahwilliams
changed the title
| size_t vertex_index, | ||
| size_t vertex_offset) const { | ||
| for (const auto& array : vertex_attrib_arrays_) { | ||
| if (true) { |
|
Working for everything but openGL. There I need to do a bit more work to deduce stride computations and how to map indexes. |
chinmaygarde
changed the title
|
This is expected to remain open until later this year. |
|
This is extremely stale, and unfortunately I never finished it for GLES. I'm going to close, but this might serve as a reference when we pick it back up in the future. |
5 participants
By default Impeller interleaves all vertex data into a sinigle buffer. This is generally convienent, except when the vertex data is coming from disperate buffers (drawVertices, drawAtlas). We should support configuring the pipeline so that this vertex data can be supplied from different buffers.
in the future, this may allow further simplifications of the geometry class, which is currently a bit more complicated than it needs to be due to having to apply uv mapping as well.
Also updates the dl_vertices geometry to special case per color vertex data. Though this still needs to be converted as dl_color and color have different representations.
Work towards flutter/flutter#116168