From c103be50de6715d9e32093ae72d179e24b447030 Mon Sep 17 00:00:00 2001
From: Jesper van den Ende <jespertheend@gmail.com>
Date: Thu, 22 Aug 2024 15:59:20 +0200
Subject: [PATCH] fix(VertexState): Don't throw when preferred shader locations
 matches the vertex state

---
 src/rendering/VertexState.js                | 10 +--
 test/unit/src/rendering/VertexState.test.js | 82 +++++++++++++++++++++
 2 files changed, 87 insertions(+), 5 deletions(-)

diff --git a/src/rendering/VertexState.js b/src/rendering/VertexState.js
index 4fbe2f07f..e087927b8 100644
--- a/src/rendering/VertexState.js
+++ b/src/rendering/VertexState.js
@@ -72,12 +72,12 @@ export class VertexState {
 
 		for (const buffer of this.buffers) {
 			for (const attribute of buffer.attributes) {
-				const loc = preferredShaderLocationsMap.get(attribute.attributeType);
-				if (loc !== undefined) {
-					if (takenShaderLocations.has(loc)) {
-						throw new Error(`Preferred shader location ${loc} is already taken by an attribute in the VertexState.`);
+				const location = preferredShaderLocationsMap.get(attribute.attributeType);
+				if (location !== undefined && location != attribute.shaderLocation) {
+					if (takenShaderLocations.has(location)) {
+						throw new Error(`Preferred shader location ${location} is already taken by an attribute in the VertexState.`);
 					}
-					takenShaderLocations.add(loc);
+					takenShaderLocations.add(location);
 				}
 			}
 		}
diff --git a/test/unit/src/rendering/VertexState.test.js b/test/unit/src/rendering/VertexState.test.js
index 7f35a15e2..6f1dbaa84 100644
--- a/test/unit/src/rendering/VertexState.test.js
+++ b/test/unit/src/rendering/VertexState.test.js
@@ -248,6 +248,88 @@ Deno.test({
 	},
 });
 
+Deno.test({
+	name: "getDescriptor() prioritizes the shader location from the vertex state over that of the preferredShaderLocations",
+	fn() {
+		/** @type {import("../../../../src/rendering/VertexState.js").VertexStateOptions} */
+		const options = {
+			buffers: [
+				{
+					attributes: [
+						{
+							attributeType: Mesh.AttributeType.POSITION,
+							componentCount: 3,
+							format: Mesh.AttributeFormat.FLOAT32,
+							shaderLocation: 1,
+						},
+						{
+							attributeType: Mesh.AttributeType.COLOR,
+							componentCount: 3,
+							format: Mesh.AttributeFormat.FLOAT32,
+							shaderLocation: 2,
+						},
+					],
+				},
+			],
+		};
+		const vertexState = new VertexState(options);
+
+		const result = vertexState.getDescriptor({
+			preferredShaderLocations: [
+				{ attributeType: Mesh.AttributeType.POSITION, location: 3 },
+				{ attributeType: Mesh.AttributeType.COLOR, location: 4 },
+			],
+		});
+
+		assertEquals(result.buffers.length, 1);
+		const attributes = /** @type {GPUVertexAttribute[]} */ (result.buffers[0].attributes);
+		assertEquals(attributes.length, 2);
+		assertEquals(attributes[0].shaderLocation, 1);
+		assertEquals(attributes[1].shaderLocation, 2);
+	},
+});
+
+Deno.test({
+	name: "getDescriptor() doesn't throw when preferred shader locations are the same as that in the vertexstate",
+	fn() {
+		/** @type {import("../../../../src/rendering/VertexState.js").VertexStateOptions} */
+		const options = {
+			buffers: [
+				{
+					attributes: [
+						{
+							attributeType: Mesh.AttributeType.POSITION,
+							componentCount: 3,
+							format: Mesh.AttributeFormat.FLOAT32,
+							shaderLocation: 1,
+						},
+						{
+							attributeType: Mesh.AttributeType.COLOR,
+							componentCount: 3,
+							format: Mesh.AttributeFormat.FLOAT32,
+							shaderLocation: 2,
+						},
+					],
+				},
+			],
+		};
+		const vertexState = new VertexState(options);
+
+		const result = vertexState.getDescriptor({
+			preferredShaderLocations: [
+				{ attributeType: Mesh.AttributeType.POSITION, location: 1 },
+				{ attributeType: Mesh.AttributeType.COLOR, location: 2 },
+			],
+		});
+
+		assertEquals(result.buffers.length, 1);
+		const attributes = /** @type {GPUVertexAttribute[]} */ (result.buffers[0].attributes);
+		assertEquals(attributes.length, 2);
+		assertEquals(attributes[0].shaderLocation, 1);
+		assertEquals(attributes[1].shaderLocation, 2);
+	},
+});
+
 Deno.test({
 	name: "getDescriptor() uses the first available shader location for automatic shader location attributes",
 	fn() {