Early on Advanced Layers did not aggressively use constant buffers, but it had an API to share a single buffer across multiple shader slots and draw calls. It uses a feature called "constant buffer offsetting", provided by ID3D11DeviceContext1. It seemed to work well so I moved a lot of vertex data into constant buffers, similar to what WebRender did when it had tiling. It turns out constant buffer offsetting is either not well specified or not well supported by drivers. bug 1377866 is a good example. We've also had to workaround obscure bugs on our own infrastructure. As a result, I'd like to move vertex shader constant data back into vertex buffers. I think this is more normal anyway, and it will simplify our shaders. On systems where the buffer sharing API is completely broken, we won't have to allocate as many ID3D11Buffers. We'll still use shared buffers for pixel shaders, and for anything complicated like layer transforms.

Currently, our shaders use either a triangle list, or a unit quad with vertex information in a separate instance buffer. The triangle case is not instanced, so each vertex contains all of the data that would normally be in the instance buffer. As we move more fields out of constant buffers and into vertices, triangle buffers will be duplicating more data. This patch converts all the triangle-based shaders to use a unit triangle instead, so we can take advantage of instancing. There is no loss in performance as far as I can tell. It's also a modest code cleanup, eliminating the logic that had to decide how to build and bind a separate kind of buffer.

Build vertex buffers is really annoying, since there is a large matrix of how the data is formatted. For example, a unit quad PaintedLayer needs to attach a Rect to the vertex, but a unit triangle PaintedLayer needs to attach two 3-tuples of coordinates instead. And a PaintedLayer with geometry needs to attach an arbitrary list of texture coordinates. Meanwhile Color layers only have to attach a Color, no matter what the geometry is. Currently each shader must expose this logic through a Traits class, but it is encapsulated too well. Each shader has to re-implement a ton of boilerplate which is evident in ShaderDefinitionsMLGPU.cpp. This patch simplifies everything so shaders only have to expose the exact functionality they need. All input layouts now share a common prefix, so Traits can also share a base vertex structure. Each derived Traits can define its ancillary vertex information, as well as methods that build this information based on the geometry. For example, ColorTraits doesn't care about its geometry, so its "MakeVertexData" function ignores the vertex type. It also does not need to override how polygons are handled. TexturedTraits however must export textured coordinates, so it overloads "MakeVertexData" for every geometry type, as well as the "GenerateTriangles" function. Note that we can now share a generic version of the base vertex logic. This patch is purely a refactoring, which is why ColorTraits still uses a constant buffer index as its ancillary data. That will be removed in the next patch.

This patch expands upon part 2. First, it removes the "items" constant buffer that was used to store ancillary vertex data. Batches can still use a constant buffer if they want to, we just no longer force it. This required some shader updates. Second, we now use Traits to attach unit quad geometry as well. In the ColorTraits case there is no change, since it doesn't care about geometry. In the TexturedTraits case, it attaches the texture uv-rect.

Comment on attachment 8884571 [details] [diff] [review] part 1, use unit triangles Review of attachment 8884571 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/d3d11/MLGDeviceD3D11.cpp @@ +831,5 @@ > + { 1.0f, 0.0f, 0.0f }, { 0.0f, 1.0f, 0.0f }, { 0.0f, 0.0f, 1.0f } > + }; > + mUnitTriangleVB = CreateBuffer( > + MLGBufferType::Vertex, > + sizeof(Vertex3D) * 4, 3? ::: gfx/layers/mlgpu/ShaderDefinitionsMLGPU.cpp @@ +12,5 @@ > > using namespace gfx; > > // Helper function for adding triangle vertices to shader buffers. > struct TriangleVertex Maybe rename this (and the textured version) since it's now all the vertices for a given triangle, rather than a single one?

(In reply to Matt Woodrow (:mattwoodrow) from comment #4) > Comment on attachment 8884571 [details] [diff] [review] > part 1, use unit triangles > > ::: gfx/layers/mlgpu/ShaderDefinitionsMLGPU.cpp > @@ +12,5 @@ > > > > using namespace gfx; > > > > // Helper function for adding triangle vertices to shader buffers. > > struct TriangleVertex > > Maybe rename this (and the textured version) since it's now all the vertices > for a given triangle, rather than a single one? It gets moved around in the second patch, so I'll rename it there.

Same patch, w/ name change.

Same patch, rebased.

Pushed by danderson@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/30e72eadd699 Use unit triangles for full polgyon shaders. (bug 1379314 part 1, r=mattwoodrow) https://hg.mozilla.org/integration/mozilla-inbound/rev/09f2592fdf69 Refactor how vertices are added to unit-triangle shaders. (bug 1379314 part 2, r=mattwoodrow) https://hg.mozilla.org/integration/mozilla-inbound/rev/81f20e3d7ed6 Don't use constant buffers for ancillary vertex data. (bug 1379314 part 3, r=mattwoodrow)

https://hg.mozilla.org/mozilla-central/rev/30e72eadd699 https://hg.mozilla.org/mozilla-central/rev/09f2592fdf69 https://hg.mozilla.org/mozilla-central/rev/81f20e3d7ed6