Skip to content

Commit 3be71ff

Browse files
authored
TSL: Only allow contiguous elements in set methods (#2050)
* TSL: Only allow contiguous elements in set methods * Add test * Add test * Add better reasoning
1 parent c92a632 commit 3be71ff

File tree

4 files changed

+74
-22
lines changed

4 files changed

+74
-22
lines changed

swizzle-generator.js

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,14 @@ for (let i = 0; i < 4; i++) {
1818
}
1919
}
2020

21+
let swizzleOptionsNoRepetitionContiguousOrdered = [];
22+
for (let i = 0; i < 4; i++) {
23+
swizzleOptionsNoRepetitionContiguousOrdered[i] = [];
24+
for (let j = 0; j < 4; j++) {
25+
swizzleOptionsNoRepetitionContiguousOrdered[i][j] = [];
26+
}
27+
}
28+
2129
let swizzleOptionsNoRepetitionOrdered = [];
2230
for (let i = 0; i < 4; i++) {
2331
swizzleOptionsNoRepetitionOrdered[i] = [];
@@ -67,6 +75,16 @@ function setProtoSwizzle(a, b, c, d) {
6775
}
6876
}
6977

78+
if (b == null || a === b - 1) {
79+
if (c == null || b === c - 1) {
80+
if (d == null || c === d - 1) {
81+
swizzleOptionsNoRepetitionContiguousOrdered[numChars - 1][maxCharNum].push(prop);
82+
swizzleOptionsNoRepetitionContiguousOrdered[numChars - 1][maxCharNum].push(altA);
83+
swizzleOptionsNoRepetitionContiguousOrdered[numChars - 1][maxCharNum].push(altB);
84+
}
85+
}
86+
}
87+
7088
if (b == null || a < b) {
7189
if (c == null || a < c && b < c) {
7290
if (d == null || c < d && b < d && a < d) {
@@ -128,7 +146,7 @@ for (let i = 0; i < 4; i++) {
128146
console.log(`interface SetSwizzle${i + 1}<TNumOrBool extends NumOrBoolType> {`);
129147
for (let j = 0; j <= i; j++) {
130148
for (let k = 0; k <= i; k++) {
131-
const arr = swizzleOptionsNoRepetitionOrdered[j][k];
149+
const arr = swizzleOptionsNoRepetitionContiguousOrdered[j][k];
132150
for (const val of arr) {
133151
console.log(` set${val.toUpperCase()}(value: ${assignType[j]}): ${accessType[i]};`);
134152
}

tsl-testing/TSLCore.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ testVec3.setXY(5);
5353
// Allow passing a vec of the right size when setting a vec swizzle property
5454
testVec3.setXY(testVec2);
5555

56-
// Allow elements that are not right next to each other
56+
// Disallow elements that are not right next to each other, they are not handled properly
57+
// @ts-expect-error
5758
testVec3.setXZ(3);
5859

5960
// vec2s don't have a z property
@@ -64,6 +65,10 @@ testVec2.setXZ(testVec2);
6465
// @ts-expect-error
6566
testVec3.setXY(testVec3);
6667

68+
// Disallow elements that are not right next to each other, they are not handled properly
69+
// @ts-expect-error
70+
testVec3.setXZ(testVec2);
71+
6772
// Disallow duplicate identifiers when setting a vec swizzle property
6873
// @ts-expect-error
6974
testVec3.setXX(5);
@@ -87,7 +92,8 @@ testVec3.flipXZ();
8792
// @ts-expect-error
8893
testVec2.flipXZ();
8994

90-
// Disallow out-of-order identifiers, since they are sorted internally
95+
// Disallow out-of-order identifiers. They are functionally identically to the sorted variant, and unnecessarily add to
96+
// the number of available methods
9197
// @ts-expect-error
9298
testVec3.flipZX();
9399

tsl-testing/node-type.test.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect, test } from "vitest";
2-
import { Fn, normalWorldGeometry, vec2, vec3 } from "three/tsl";
2+
import { Fn, normalWorldGeometry, vec2, vec3, vec4 } from "three/tsl";
33
import * as THREE from "three/webgpu";
44

55
const renderer = new THREE.WebGPURenderer();
@@ -112,6 +112,23 @@ test("vec3(1, 0, 2).xz = 6", async () => {
112112
renderer.render(scene, camera);
113113
});
114114

115+
test("vec3(1, 0, 2).xz = vec2(2, 3)", async () => {
116+
const fn = Fn(() => {
117+
const testVec3 = vec3(1, 0, 2);
118+
testVec3.xz = vec2(2, 3);
119+
return testVec3;
120+
});
121+
122+
const result: THREE.Node<"vec3"> = fn();
123+
124+
expect(result.getNodeType(nodeBuilder)).toBe("vec3");
125+
126+
scene.backgroundNode = result.debug();
127+
128+
await renderer.init();
129+
renderer.render(scene, camera);
130+
});
131+
115132
test("vec3(1, 0, 2).setXY(5)", async () => {
116133
const testVec3Set: THREE.Node<"vec3"> = vec3(1, 0, 2).setXY(5);
117134

@@ -123,7 +140,21 @@ test("vec3(1, 0, 2).setXY(5)", async () => {
123140
renderer.render(scene, camera);
124141
});
125142

143+
test("vec4(1, 0, 2, 5).setYZ(vec2(5, 3))", async () => {
144+
const testVec3Set: THREE.Node<"vec4"> = vec4(1, 0, 2, 5).setYZ(vec2(5, 3));
145+
146+
expect(testVec3Set.getNodeType(nodeBuilder)).toBe("vec4");
147+
148+
scene.backgroundNode = testVec3Set.debug();
149+
150+
await renderer.init();
151+
renderer.render(scene, camera);
152+
});
153+
126154
test("vec3(1, 0, 2).setXZ(3)", async () => {
155+
// This doesn't work as expected, the resulting output is:
156+
// vec3<f32>( vec2<f32>( 3.0 ), vec3<f32>( 1.0, 0.0, 2.0 ).z )
157+
// @ts-expect-error
127158
const testVec3Set: THREE.Node<"vec3"> = vec3(1, 0, 2).setXZ(3);
128159

129160
expect(testVec3Set.getNodeType(nodeBuilder)).toBe("vec3");
@@ -134,6 +165,20 @@ test("vec3(1, 0, 2).setXZ(3)", async () => {
134165
renderer.render(scene, camera);
135166
});
136167

168+
test("vec3(1, 0, 2).setXZ(vec2(5, 3))", async () => {
169+
// This doesn't work as expected, the resulting output is:
170+
// vec3<f32>( vec2<f32>( 5.0, 3.0 ), vec3<f32>( 1.0, 0.0, 2.0 ).z )
171+
// @ts-expect-error
172+
const testVec3Set: THREE.Node<"vec3"> = vec3(1, 0, 2).setXZ(vec2(5, 3));
173+
174+
expect(testVec3Set.getNodeType(nodeBuilder)).toBe("vec3");
175+
176+
scene.backgroundNode = testVec3Set.debug();
177+
178+
await renderer.init();
179+
renderer.render(scene, camera);
180+
});
181+
137182
test("vec3(1, 0, 2).setYZ(3)", async () => {
138183
const testVec3Set: THREE.Node<"vec3"> = vec3(1, 0, 2).setYZ(3);
139184

@@ -179,6 +224,7 @@ test("vec3(1, 0, 2).flipXZ()", async () => {
179224
});
180225

181226
test("vec3(1, 0, 2).flipZX()", async () => {
227+
// This is functions correctly, but we don't include it to cut down on the number of methods
182228
// @ts-expect-error
183229
const testVec3Set: THREE.Node<"vec3"> = vec3(1, 0, 2).flipZX();
184230

types/three/src/nodes/tsl/TSLCore.d.ts

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,9 +1371,6 @@ interface SetSwizzle3<TNumOrBool extends NumOrBoolType> {
13711371
setXY(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec3<TNumOrBool>>;
13721372
setRG(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec3<TNumOrBool>>;
13731373
setST(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec3<TNumOrBool>>;
1374-
setXZ(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec3<TNumOrBool>>;
1375-
setRB(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec3<TNumOrBool>>;
1376-
setSP(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec3<TNumOrBool>>;
13771374
setYZ(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec3<TNumOrBool>>;
13781375
setGB(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec3<TNumOrBool>>;
13791376
setTP(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec3<TNumOrBool>>;
@@ -1398,30 +1395,15 @@ interface SetSwizzle4<TNumOrBool extends NumOrBoolType> {
13981395
setXY(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
13991396
setRG(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
14001397
setST(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1401-
setXZ(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1402-
setRB(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1403-
setSP(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
14041398
setYZ(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
14051399
setGB(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
14061400
setTP(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1407-
setXW(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1408-
setRA(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1409-
setSQ(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1410-
setYW(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1411-
setGA(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1412-
setTQ(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
14131401
setZW(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
14141402
setBA(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
14151403
setPQ(value: Node<NumOrBoolToVec2<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
14161404
setXYZ(value: Node<NumOrBoolToVec3<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
14171405
setRGB(value: Node<NumOrBoolToVec3<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
14181406
setSTP(value: Node<NumOrBoolToVec3<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1419-
setXYW(value: Node<NumOrBoolToVec3<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1420-
setRGA(value: Node<NumOrBoolToVec3<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1421-
setSTQ(value: Node<NumOrBoolToVec3<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1422-
setXZW(value: Node<NumOrBoolToVec3<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1423-
setRBA(value: Node<NumOrBoolToVec3<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
1424-
setSPQ(value: Node<NumOrBoolToVec3<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
14251407
setYZW(value: Node<NumOrBoolToVec3<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
14261408
setGBA(value: Node<NumOrBoolToVec3<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;
14271409
setTPQ(value: Node<NumOrBoolToVec3<TNumOrBool>> | NumOrBool<TNumOrBool>): Node<NumOrBoolToVec4<TNumOrBool>>;

0 commit comments

Comments
 (0)