docs: add fixes for sprite performance issues

Co-authored-by: codegirl007 <s.raide@gmail.com>
2026-08-09 06:04:34 +00:00
parent c49302d90c
commit 7a04cdaa8b
6 changed files with 412 additions and 15 deletions
@@ -93,10 +93,29 @@ These results are from a software Vulkan backend. Hardware drivers may have a
 different balance, which is another reason to keep the benchmark reproducible
 and report backend details.

-## Proposed change
+## Suggested fix

 Add a non-interactive benchmark target with two explicitly separate workloads.

+Add dedicated Makefile targets that always build optimized benchmark code:
+
+```make
+PERF_ITERATIONS ?= 2000000
+PERF_FRAMES ?= 1000
+
+perf-draw:
+	odin run examples/draw_bench \
+		-collection:pkg=. \
+		-debug -o:speed \
+		-define:PERF_ITERATIONS=$(PERF_ITERATIONS)
+
+perf-frame:
+	odin run examples/frame_bench \
+		-collection:pkg=. \
+		-debug -o:speed \
+		-define:PERF_FRAMES=$(PERF_FRAMES)
+```
+
 ### CPU queue benchmark

 - Construct `App`, `Character_Data`, and `Sprite` with real baked metadata.
@@ -108,6 +127,28 @@ Add a non-interactive benchmark target with two explicitly separate workloads.
 - Run at least one million calls and report nanoseconds per draw.
 - Build with `-o:speed` by default.

+The measured loop should clear the queue at its cap, vary input to prevent
+compiler hoisting, and report time per draw:
+
+```odin
+PERF_ITERATIONS :: #config(PERF_ITERATIONS, 2_000_000)
+
+start := sdl.GetTicksNS()
+for i in 0 ..< PERF_ITERATIONS {
+	if len(app.draw_list) == eng.MAX_SPRITES {
+		clear(&app.draw_list)
+	}
+	sprite.position.x = f32(i & 1023)
+	eng.draw_sprite(&app, &sprite)
+}
+elapsed := sdl.GetTicksNS() - start
+
+fmt.printfln(
+	"%.3f ns/draw",
+	f64(elapsed) / f64(PERF_ITERATIONS),
+)
+```
+
 ### Full-frame benchmark

 - Use a real SDL GPU device and baked texture.
@@ -117,6 +158,27 @@ Add a non-interactive benchmark target with two explicitly separate workloads.
 - Record GPU backend, present mode, compiler version, compiler flags, and sprite
  count.

+Use a fixed frame count rather than an interactive quit time:
+
+```odin
+PERF_FRAMES :: #config(PERF_FRAMES, 1_000)
+
+for _ in 0 ..< 100 {
+	draw_benchmark_frame(&app, sprites[:]) // warm-up
+}
+
+start := sdl.GetTicksNS()
+for _ in 0 ..< PERF_FRAMES {
+	draw_benchmark_frame(&app, sprites[:])
+}
+elapsed := sdl.GetTicksNS() - start
+
+fmt.printfln(
+	"%.3f ms/frame",
+	f64(elapsed) / f64(PERF_FRAMES) / 1_000_000.0,
+)
+```
+
 The CPU benchmark should be available without a display or GPU. The full-frame
 benchmark may remain opt-in where a suitable GPU backend is unavailable.

@@ -0,0 +1,101 @@
+# Add opt-in order-safe texture batching
+
+## Summary
+
+`end_frame` batches only consecutive sprites that use the same texture.
+Alternating two textures therefore produces one sampler bind and draw call per
+sprite even when some sprites could safely be regrouped.
+
+Globally sorting transparent sprites by texture is not correct: overlapping
+sprites may blend differently when submission order changes. Batching should
+therefore be opt-in within explicit groups whose members are safe to reorder.
+
+## Evidence
+
+A temporary paired benchmark used:
+
+- Odin `dev-2026-05-nightly:ea5175d` with `-debug -o:speed`
+- SDL 3.4.12 and Vulkan/Lavapipe
+- 128 animated sprites alternating between two equivalent textures
+- Sorting cost included in the measured frame
+- Ten order-alternated samples of 400 frames per mode
+
+| Mode | Texture runs | Median frame time |
+| --- | ---: | ---: |
+| Submission order | 128 | 5.765 ms |
+| Texture grouped | 2 | 5.654 ms |
+
+Sorting and grouping improved median frame time by approximately **1.9%**.
+Hardware drivers with higher draw-call overhead may show a different result.
+
+## Suggested fix
+
+Add an explicit batch group to queued sprites. Group `0` keeps strict submission
+order; nonzero groups may be reordered only when the caller guarantees that
+their members are order-independent.
+
+```odin
+Queued_Sprite :: struct {
+	texture:     ^sdl.GPUTexture,
+	verts:       [SPRITE_VERT_COUNT]Vertex,
+	batch_group: u32, // 0 = strict order; nonzero = caller permits regrouping
+}
+```
+
+Sort each contiguous, nonzero group by texture immediately before upload:
+
+```odin
+group_texture_runs :: proc(list: []Queued_Sprite) {
+	start := 0
+	for start < len(list) {
+		group := list[start].batch_group
+		if group == 0 {
+			start += 1
+			continue
+		}
+
+		end := start + 1
+		for end < len(list) && list[end].batch_group == group {
+			end += 1
+		}
+
+		// Stable insertion sort is sufficient while MAX_SPRITES is 128.
+		for i in start + 1 ..< end {
+			item := list[i]
+			j := i
+			for j > start {
+				if uintptr(list[j - 1].texture) <= uintptr(item.texture) {
+					break
+				}
+				list[j] = list[j - 1]
+				j -= 1
+			}
+			list[j] = item
+		}
+		start = end
+	}
+}
+```
+
+Call it after all sprites are queued and before the transfer-buffer copy:
+
+```odin
+group_texture_runs(app.draw_list[:])
+```
+
+Expose batching through a separate API or explicit parameter so existing
+`draw_sprite` calls remain strict-order by default:
+
+```odin
+draw_sprite_batched(&app, &sprite, batch_group = 1)
+```
+
+## Acceptance criteria
+
+- Existing `draw_sprite` behavior preserves exact submission order.
+- Reordering requires an explicit nonzero batch group.
+- Sorting never moves a sprite across a strict-order entry or group boundary.
+- Add tests for strict order, group boundaries, stable same-texture ordering,
+  and reduced texture-run count.
+- Add a visual overlap test confirming default alpha compositing is unchanged.
+- Benchmark sorting cost and draw-call reduction on a hardware GPU.
@@ -0,0 +1,69 @@
+# Enable SDL GPU buffer cycling for per-frame sprite uploads
+
+## Summary
+
+`end_frame` overwrites the same transfer buffer and vertex buffer every frame,
+but both SDL calls currently pass `cycle = false`:
+
+```odin
+sdl.MapGPUTransferBuffer(app.device, app.transfer_buffer, false)
+sdl.UploadToGPUBuffer(copy_pass, src, dst, false)
+```
+
+SDL documents cycling as the mechanism that rotates to an unbound internal
+resource when the previous frame still references the current one. Enabling it
+avoids an unnecessary resource dependency and makes the overwrite pattern
+explicitly safe.
+
+## Evidence
+
+A temporary paired benchmark used:
+
+- Odin `dev-2026-05-nightly:ea5175d` with `-debug -o:speed`
+- SDL 3.4.12 and Vulkan/Lavapipe
+- 128 animated sprites
+- Ten order-alternated samples of 400 frames per mode
+
+| Mode | Median frame time |
+| --- | ---: |
+| Cycling disabled | 4.869 ms |
+| Cycling enabled | 4.834 ms |
+
+Cycling improved median frame time by approximately **0.7%**. This is a small
+performance change, but it also follows SDL's documented resource-reuse model.
+
+## Suggested fix
+
+Cycle both resources that are fully overwritten each frame:
+
+```odin
+map_ptr := sdl.MapGPUTransferBuffer(
+	app.device,
+	app.transfer_buffer,
+	true, // rotate if the previous frame still binds this transfer buffer
+)
+
+// Write the complete [0, n * SPRITE_VERTS_SIZE) range, then unmap.
+sdl.UnmapGPUTransferBuffer(app.device, app.transfer_buffer)
+
+copy_pass := sdl.BeginGPUCopyPass(cmd)
+sdl.UploadToGPUBuffer(
+	copy_pass,
+	src,
+	dst,
+	true, // rotate the destination vertex buffer if it is still bound
+)
+sdl.EndGPUCopyPass(copy_pass)
+```
+
+Cycling makes previous contents undefined, so this remains correct only because
+the renderer writes the complete vertex range used by the frame before drawing.
+Do not enable cycling for partial updates that depend on untouched data.
+
+## Acceptance criteria
+
+- Both `MapGPUTransferBuffer` and `UploadToGPUBuffer` use `cycle = true`.
+- The complete submitted vertex range is rewritten every frame.
+- Existing engine tests and examples continue to pass.
+- A full-frame benchmark confirms no regression on a hardware GPU backend.
+- Add a comment explaining why cycling is safe for this full-overwrite path.
@@ -47,7 +47,7 @@ Environment:
 - SDL 3.4.12
 - Linux x86-64

-## Proposed change
+## Suggested fix

 Compile profiling binaries with optimization while retaining symbols:

@@ -55,13 +55,25 @@ Compile profiling binaries with optimization while retaining symbols:
 FLAME_ODIN_FLAGS ?= -debug -o:speed

 flame-build:
-	odin build examples/$(FLAME_EXAMPLE) -collection:pkg=. \
-		-out:$(FLAME_BIN) $(FLAME_ODIN_FLAGS)
+	odin build examples/$(FLAME_EXAMPLE) \
+		-collection:pkg=. \
+		-out:$(FLAME_BIN) \
+		$(FLAME_ODIN_FLAGS)
 ```

 Keeping the flags configurable allows an explicitly unoptimized diagnostic run
 without making it the performance default.

+Example usage:
+
+```bash
+# Representative performance profile: optimized code with debug symbols.
+make flame FLAME_EXAMPLE=crowd
+
+# Explicitly profile unoptimized code when investigating debug-only behavior.
+make flame FLAME_EXAMPLE=crowd FLAME_ODIN_FLAGS="-debug -o:none"
+```
+
 Consider applying an explicit optimization mode to performance-oriented example
 runs as well. Plain `odin run` currently uses Odin's unoptimized default.

@@ -0,0 +1,123 @@
+# Prototype instanced sprite rendering for larger batches
+
+## Summary
+
+The renderer currently generates and uploads six complete vertices per sprite:
+
+```text
+6 vertices × (position float2 + UV float2) = 96 bytes/sprite/frame
+```
+
+Instancing can keep one immutable six-corner unit quad on the GPU and upload one
+rectangle/UV record per sprite:
+
+```text
+clip rectangle float4 + UV rectangle float4 = 32 bytes/sprite/frame
+```
+
+This reduces dynamic upload volume by two thirds, but requires another pipeline
+and backend-specific vertex shader. At the current 128-sprite cap, the measured
+gain is too small to justify enabling it unconditionally.
+
+## Evidence
+
+A complete temporary instanced path was implemented with:
+
+- A static six-corner vertex buffer
+- A 32-byte per-instance buffer
+- Vertex-rate and instance-rate pipeline inputs
+- One instanced draw per texture run
+- Validated SPIR-V and SDL's debug GPU device
+
+The paired benchmark used ten order-alternated samples of 400 frames:
+
+| Mode | Median frame time |
+| --- | ---: |
+| Six dynamic vertices per sprite | 5.527 ms |
+| 32-byte instance per sprite | 5.467 ms |
+
+Instancing improved median frame time by approximately **1.1%** on
+Vulkan/Lavapipe with 128 sprites.
+
+## Suggested fix
+
+Treat this as a prototype gated by larger sprite counts or a demonstrated
+hardware bottleneck, not as an immediate replacement.
+
+Define the compact instance payload:
+
+```odin
+Sprite_Instance :: struct {
+	clip_rect: [4]f32, // left, top, right, bottom
+	uv_rect:   [4]f32, // u0, v0, u1, v1
+}
+```
+
+Queue one record after the existing quad and UV calculations:
+
+```odin
+instance := Sprite_Instance {
+	clip_rect = {p0.x, p0.y, p2.x, p2.y},
+	uv_rect   = {u0, v0, u1, v1},
+}
+append(&app.instance_list, instance)
+```
+
+Use a static unit quad:
+
+```odin
+UNIT_QUAD := [6]Vec2 {
+	{0, 0}, {1, 0}, {1, 1},
+	{0, 0}, {1, 1}, {0, 1},
+}
+```
+
+The instanced vertex shader reconstructs position and UV:
+
+```glsl
+#version 450
+
+layout(location = 0) in vec2 in_corner;
+layout(location = 1) in vec4 in_clip_rect;
+layout(location = 2) in vec4 in_uv_rect;
+
+layout(location = 0) out vec2 v_uv;
+
+void main() {
+    vec2 position = mix(in_clip_rect.xy, in_clip_rect.zw, in_corner);
+    v_uv = mix(in_uv_rect.xy, in_uv_rect.zw, in_corner);
+    gl_Position = vec4(position, 0.0, 1.0);
+}
+```
+
+Configure slot 0 as vertex-rate and slot 1 as instance-rate, then draw each
+texture run with six vertices and `run` instances:
+
+```odin
+vb_descs := [2]sdl.GPUVertexBufferDescription {
+	{slot = 0, pitch = u32(size_of(Vec2)), input_rate = .VERTEX},
+	{slot = 1, pitch = u32(size_of(Sprite_Instance)), input_rate = .INSTANCE},
+}
+
+sdl.DrawGPUPrimitives(
+	app.render_pass,
+	6,        // unit-quad vertices
+	u32(run), // sprite instances in this texture run
+	0,
+	0,
+)
+```
+
+Keep the current path as a fallback until the instanced implementation exists
+for Vulkan, D3D12, and Metal and demonstrates a meaningful hardware win.
+
+## Acceptance criteria
+
+- Benchmark at 128, 512, 2,048, and 10,000 sprites or the highest supported
+  counts.
+- Report CPU queue time, bytes uploaded, and complete frame time separately.
+- Require a meaningful hardware improvement before changing the default path.
+- Supply equivalent Vulkan, D3D12, and Metal shaders.
+- Preserve texture-run batching and sprite flip/trim behavior.
+- Add visual equivalence tests for position, UVs, animation frames, and flip.
+- Retain the existing six-vertex path as a fallback during evaluation.
@@ -55,28 +55,58 @@ Environment:
 - Optimized with `-debug -o:speed`
 - Linux x86-64

-## Proposed change
+## Suggested fix

 Compute clip scaling once and construct corners from the unique coordinates:

 ```odin
-sx := 2.0 / f32(app.swapchain_w)
-sy := 2.0 / f32(app.swapchain_h)
+sprite_quad_to_clip :: proc(x0, y0, x1, y1, sw, sh: f32) -> [4]Vec2 {
+	sx := 2.0 / sw
+	sy := 2.0 / sh

-left   := x0_px * sx - 1
-right  := x1_px * sx - 1
-top    := 1 - y0_px * sy
-bottom := 1 - y1_px * sy
+	left   := x0 * sx - 1
+	right  := x1 * sx - 1
+	top    := 1 - y0 * sy
+	bottom := 1 - y1 * sy

-p0 := Vec2{left, top}
-p1 := Vec2{right, top}
-p2 := Vec2{right, bottom}
-p3 := Vec2{left, bottom}
+	return {
+		{left, top},
+		{right, top},
+		{right, bottom},
+		{left, bottom},
+	}
+}
+
+points := sprite_quad_to_clip(x0_px, y0_px, x1_px, y1_px, sw, sh)
+p0, p1, p2, p3 := points[0], points[1], points[2], points[3]
 ```

 Keep `to_clip` for general callers and its existing tests; this change only
 specializes quad construction inside `draw_sprite`.

+Add an equivalence test before replacing the current calls:
+
+```odin
+@(test)
+sprite_quad_clip_math_matches_to_clip :: proc(t: ^testing.T) {
+	x0, y0 := f32(125), f32(80)
+	x1, y1 := f32(325), f32(280)
+	sw, sh := f32(800), f32(600)
+
+	expected := [4]Vec2 {
+		to_clip(x0, y0, sw, sh),
+		to_clip(x1, y0, sw, sh),
+		to_clip(x1, y1, sw, sh),
+		to_clip(x0, y1, sw, sh),
+	}
+	actual := sprite_quad_to_clip(x0, y0, x1, y1, sw, sh)
+
+	for i in 0 ..< 4 {
+		testing.expect_value(t, actual[i], expected[i])
+	}
+}
+```
+
 ## Acceptance criteria

 - Existing sprite geometry, camera, UV, and batching tests pass.