Make the line renderer spec compliant (#160)

crates/vsvg-viewer/src/painters/line_painter.rs

@@ -1,13 +1,140 @@
-use crate::engine::EngineRenderObjects;
-use crate::painters::{Painter, Vertex};
-use std::mem;
-use vsvg::{FlattenedPath, PathTrait};
-use wgpu::util::DeviceExt;
+//! Scale-aware line painter with arbitrary color and width.
+//!
+//! ### Design goal
+//!
+//! We want to render many (1+ million) poly-lines each with an arbitrary number of segments, for
+//! a total of 10+ million vertices.
+//!
+//! Since the line count may be very large (e.g. if each has a single segment), we must draw all
+//! lines in a single draw call. This means that our buffer must somehow encode end-of-line
+//! information.
+//!
+//! Lines ending with a vertex that is equal to the first one must be drawn as a closed shape.
+//!
+//!
+//! ### Shader input
+//!
+//! The VS is triggered 4 times per instance, to generate a triangle strip for each line segment.
+//! Each line segment corresponds to an instance.
+//!
+//! For each instance, the shader expects the following data:
+//! - 4 points (`p0`, `p1`, `p2`, and `p3`)
+//! - color and width attributes
+//!
+//! The point corresponds to:
+//! - `p0`: the previous point
+//! - `p1`: the starting point of the segment
+//! - `p2`: the ending point of the segment
+//! - `p3`: the point after the segment
+//!
+//! If `p0` and `p1` are equal, this means that this segment is the first of a line. Likewise, if
+//! `p2` and `p3` are equal, this means that this segment is the last of a line.
+//!
+//! With this information, the VS can compute the exact portion of the start/end capsule to be drawn
+//!  and send that information to the FS.
+//!
+//!
+//! ### Buffer preparation
+//!
+//! Consider the following example comprising three lines A, B, and C. A has one segment, B has
+//! three segments, and C has three segments and is closed.
+//!
+//! ```text
+//!       1 segment                3 segments                   3 segments,
+//!                                                             closed line
+//!
+//!                                                               C0, C3
+//! A0 ────────────── A1      B0 ───────┐B1                         ╱╲
+//!                                     │                          ╱  ╲
+//!                                   B2└────────B3               ╱    ╲
+//!                                                             C2──────C1
+//!
+//! points buffer       │                             │
+//! ┌────┬────┬────┬────┼────┬────┬────┬────┬────┬────┼────┬────┬────┬────┬────┬────┐
+//! │ A0 │ A0 │ A1 │ A1 │ B0 │ B0 │ B1 │ B2 │ B3 │ B3 │ C2 │ C0 │ C1 │ C2 │ C3 │ C1 │
+//! └────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┘
+//!           │                   │                             │                   │
+//!           └────────┬──────────┘                             └────────┬──────────┘
+//!      │             │     │                                           │
+//!      └────────┬────┼─────┘                ....                       │
+//! │             │    ││                                                │
+//! └────────┬────┼────┼┘                                                │
+//!          │    │    │                                                 │
+//!          ▼    ▼    ▼                                                 ▼
+//!        ┌────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┐
+//!        │ A0 │    │    │    │ B0 │ B1 │ B2 │    │    │    │ C0 │ C1 │ C2 │
+//!        └────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┘
+//!        attributes buffer (one per segment + padding)
+//!```
+//!
+//! We construct the point buffer line-by-line, with each line consisting of:
+//! - The sequence of points defining its segments.
+//! - Repeating the first and last point.
+//! - For closed shape, we pad the point list with the last and 2nd points instead, such that the
+//!   shader is always presented with 4 sequential points.
+//!
+//! Finally, we concatenate all these buffers together in one large point buffer.
+//!
+//! The key observation is that, for such a buffer topology, a sliding window of 4 points exactly
+//! corresponds with the required shader input, _except_ when the window spans two lines.
+//!
+//! To address that, we create a second buffer where each item is the attributes (color and width)
+//! for a single segment. Between each line, we insert three "empty" attributes (actually fully
+//! transparent color). The shader uses these empty attributes to detect and ignore point
+//! quadruplets which span two lines.
+//!
+//! The length of the attributes buffer (including the empty attributes) corresponds to the number
+//! of instances passed to the draw call.
+//!
+//!
+//! ### Buffer binding
+//!
+//! Given the above, the "naive" approach would be to bind the point buffer with a stride of one
+//! vertex but passing four vertices at a time. This used to work but, it turns out, is not
+//! compliant with the [WebGPU spec](https://gpuweb.github.io/gpuweb/#dictdef-gpuvertexbufferlayout).
+//! As such, it is rejected by (recent versions of) Chrome and wgpu/metal since (23.0.0).
+//!
+//! To work around this limitation, we bind the point buffer four times, with an offset of one to
+//! three vertices, respectively. Each of these bindings gets a stride of one vertex and exposes one
+//! vertex to the shader, which is spec compliant.
+//!
+//! ```text
+//!                   first                                                         last
+//!                  instance                                                     instance
+//!                     │                                                            │
+//!                     ▼                                                            ▼
+//!                   ┌────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬ ─ ─ ─ ─ ┬ ─ ─
+//! p0                │ A0 │ A0 │ A1 │ A1 │ B0 │ B0 │ B1 │ B2 │ B3 │ B3 │ C2 │ C0 │ C1 │ C2 │ C0   C1 │
+//!                   └────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴ ─ ─ ─ ─ ┴ ─ ─
+//!              ┌ ─ ─┌────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬ ─ ─ ─ ─ ┐
+//! p1             A0 │ A0 │ A1 │ A1 │ B0 │ B0 │ B1 │ B2 │ B3 │ B3 │ C2 │ C0 │ C1 │ C2 │ C0 │ C1    offset = 1
+//!              └ ─ ─└────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴ ─ ─ ─ ─ ┘
+//!          ─ ─ ┬ ─ ─┌────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬ ─ ─
+//! p2      │ A0   A0 │ A1 │ A1 │ B0 │ B0 │ B1 │ B2 │ B3 │ B3 │ C2 │ C0 │ C1 │ C2 │ C0 │ C1 │       offset = 2
+//!          ─ ─ ┴ ─ ─└────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴ ─ ─
+//!    ┌ ─ ─ ─ ─ ┬ ─ ─┌────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┐
+//! p3   A0 │ A0   A1 │ A1 │ B0 │ B0 │ B1 │ B2 │ B3 │ B3 │ C2 │ C0 │ C1 │ C2 │ C0 │ C1 │            offset = 3
+//!    └ ─ ─ ─ ─ ┴ ─ ─└────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┘
+//!                   point buffer (bound four times)
+//!
+//!
+//!
+//!                   ┌────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┬────┐
+//!                   │ A0 │    │    │    │ B0 │ B1 │ B2 │    │    │    │ C0 │ C1 │ C2 │
+//!                   └────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┴────┘
+//!                   attributes buffer
+//! ```
+
 use wgpu::{
-    include_wgsl, vertex_attr_array, Buffer, ColorTargetState, PrimitiveTopology, RenderPass,
-    RenderPipeline,
+    include_wgsl, util::DeviceExt, vertex_attr_array, Buffer, ColorTargetState, PrimitiveTopology,
+    RenderPass, RenderPipeline,
 };
 
+use vsvg::{FlattenedPath, PathTrait};
+
+use crate::engine::EngineRenderObjects;
+use crate::painters::{Painter, Vertex};
+
 #[repr(C)]
 #[derive(Copy, Clone, Debug, bytemuck::Pod, bytemuck::Zeroable)]
 struct Attribute {
@@ -153,33 +280,45 @@ impl LinePainterData {
 
 /// Renders paths as scale-aware lines with variable width and color.
 ///
-/// TODO: explain how this works
+/// See module documentation for details.
 pub(crate) struct LinePainter {
     render_pipeline: RenderPipeline,
 }
 
 impl LinePainter {
     pub(crate) fn new(render_objects: &EngineRenderObjects) -> Self {
-        // key insight: the stride is one point, but we expose 4 points at once!
-        let points_buffer_layout = wgpu::VertexBufferLayout {
-            array_stride: mem::size_of::<Vertex>() as wgpu::BufferAddress,
-            step_mode: wgpu::VertexStepMode::Instance,
-            attributes: &vertex_attr_array![
-                0 => Float32x2,
-                1 => Float32x2,
-                2 => Float32x2,
-                3 => Float32x2,
-            ],
-        };
+        // This is where we prepare the 4x binding of the same point buffer. Each binding has a
+        // stride of one vertex but a different starting offset.
+
+        let vertex_attributes = (0..4)
+            .map(|i| wgpu::VertexAttribute {
+                format: wgpu::VertexFormat::Float32x2,
+                offset: 0,
+                shader_location: i,
+            })
+            .collect::<Vec<_>>();
+
+        let mut buffer_layouts = vertex_attributes
+            .iter()
+            .map(|vertex_attrb| wgpu::VertexBufferLayout {
+                array_stride: size_of::<Vertex>() as wgpu::BufferAddress,
+                step_mode: wgpu::VertexStepMode::Instance,
+                attributes: std::slice::from_ref(vertex_attrb),
+            })
+            .collect::<Vec<_>>();
+
+        // add the color and width attributes
 
-        let attributes_buffer_layout = wgpu::VertexBufferLayout {
-            array_stride: mem::size_of::<Attribute>() as wgpu::BufferAddress,
+        let vertex_attrib_color_width = vertex_attr_array![
+            4 => Uint32,
+            5 => Float32,
+        ];
+
+        buffer_layouts.push(wgpu::VertexBufferLayout {
+            array_stride: size_of::<Attribute>() as wgpu::BufferAddress,
             step_mode: wgpu::VertexStepMode::Instance,
-            attributes: &vertex_attr_array![
-                4 => Uint32,
-                5 => Float32,
-            ],
-        };
+            attributes: &vertex_attrib_color_width,
+        });
 
         let shader = render_objects
             .device
@@ -222,7 +361,7 @@ impl LinePainter {
                         module: &shader,
                         entry_point: "vs_main",
                         compilation_options: Default::default(),
-                        buffers: &[points_buffer_layout, attributes_buffer_layout],
+                        buffers: &buffer_layouts,
                     },
                     fragment: Some(wgpu::FragmentState {
                         module: &shader,
@@ -255,8 +394,14 @@ impl Painter for LinePainter {
     ) {
         rpass.set_pipeline(&self.render_pipeline);
         rpass.set_bind_group(0, camera_bind_group, &[]);
+
+        let offset = size_of::<Vertex>() as u64;
         rpass.set_vertex_buffer(0, data.points_buffer.slice(..));
-        rpass.set_vertex_buffer(1, data.attributes_buffer.slice(..));
+        rpass.set_vertex_buffer(1, data.points_buffer.slice(offset..));
+        rpass.set_vertex_buffer(2, data.points_buffer.slice((2 * offset)..));
+        rpass.set_vertex_buffer(3, data.points_buffer.slice((3 * offset)..));
+
+        rpass.set_vertex_buffer(4, data.attributes_buffer.slice(..));
         rpass.draw(0..4, 0..data.instance_count);
     }
 }

crates/vsvg-viewer/src/shaders/line.wgsl

@@ -193,4 +193,7 @@ fn fs_main(in: VertexOutput) -> @location(0) vec4<f32> {
              discard;
         }
     }
+
+    // should never happen, appease Chrome and/or the spec
+    return vec4<f32>(0.0, 0.0, 0.0, 0.0);
 }

crates/vsvg-viewer/src/shaders/point.wgsl

@@ -75,9 +75,14 @@ fn fs_main(in: VertexOutput) -> @location(0) vec4<f32> {
         return in.color;
     } else if (distance < in.w2 + aa) {
         var alpha = (distance - in.w2) / aa;
-        alpha = smoothstep(1.0, 0.0, alpha);
+
+        // spec requires first arg to be lower than second
+        alpha = 1.0 - smoothstep(0.0, 1.0, alpha);
         return vec4<f32>(in.color.rgb, in.color.a * alpha);
     } else {
         discard;
     }
+
+    // should never happen, appease Chrome and/or the spec
+    return vec4<f32>(0.0, 0.0, 0.0, 0.0);
 }