Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions packages/typegpu-gl/src/glslGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
type ResolutionCtx,
type TgpuShaderStage,
type FunctionDefinitionOptions,
type Snippet,
snip,
} from 'typegpu/~internal';

// ----------
Expand Down Expand Up @@ -68,6 +70,21 @@ ${Object.entries(struct.propTypes)
return id;
}

function correspondingBooleanVectorSchema(dataType: d.BaseData) {
if (dataType.type.includes('2')) {
return d.vec2b;
}
if (dataType.type.includes('3')) {
return d.vec3b;
}
if (dataType.type.includes('4')) {
return d.vec4b;
}
throw new Error(
`Internal error: schema of type '${dataType.type}' does not have a corresponding boolean vector.`,
);
}

const gl_PositionSnippet = tgpu['~unstable'].rawCodeSnippet('gl_Position', d.vec4f, 'private');

interface EntryFnState {
Expand Down Expand Up @@ -105,6 +122,84 @@ export class GlslGenerator extends WgslGenerator {
return super.typeAnnotation(data);
}

override call(
name: string,
templateParams: readonly Snippet[],
args: readonly Snippet[],
): string {
if (name === 'bitcast') {
const [target] = templateParams;
if (!target || !d.isWgslData(target.value)) {
throw new Error(`Expected bitcast() to be called with a data type template parameter`);
}
const [source] = args;
if (!source || source.dataType === UnknownData) {
throw new Error(`Invalid argument passed to bitcast()`);
}
const targetSchema = target.value;
const sourceSchema = source.dataType;
const targetPrimitive = targetSchema.type.startsWith('vec')
? (targetSchema as d.Vec3f).primitive
: targetSchema;
const sourcePrimitive = sourceSchema.type.startsWith('vec')
? (sourceSchema as d.Vec3f).primitive
: sourceSchema;

if (sourcePrimitive.type === 'u32' && targetPrimitive.type === 'f32') {
return super.call('uintBitsToFloat', [], [source]);
}
if (sourcePrimitive.type === 'i32' && targetPrimitive.type === 'f32') {
return super.call('intBitsToFloat', [], [source]);
}
if (sourcePrimitive.type === 'f32' && targetPrimitive.type === 'u32') {
return super.call('floatBitsToUint', [], [source]);
}
if (sourcePrimitive.type === 'f32' && targetPrimitive.type === 'i32') {
return super.call('floatBitsToInt', [], [source]);
}
if (sourceSchema.type === targetSchema.type) {
return this.ctx.resolveSnippet(source).value;
}

throw new Error(`Cannot bitcast from ${String(sourceSchema)} to ${String(targetSchema)}`);
}

if (name === 'select') {
const [falsy, truthy, cond] = args;
if (!falsy || !truthy || !cond) {
throw new Error(`Invalid number of arguments for 'select'`);
}

if (falsy.dataType !== UnknownData && falsy.dataType.type.startsWith('vec')) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GLSL ES 3.0 only supports mix(genType, genType, genBType) for floating-point vectors, not for genIType or genUType. This means integer-vector std.select will still emit invalid GLSL here (it was already invalid before this PR, just via select(...)). Consider guarding non-float vector selects or adding a dedicated integer/unsigned path.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

???

Image

if (cond.dataType !== UnknownData && cond.dataType.type.startsWith('vec')) {
return super.call('mix', templateParams, args);
}
return super.call('mix', templateParams, [
falsy,
truthy,
this.typeInstantiation(correspondingBooleanVectorSchema(falsy.dataType), [cond]),
]);
}

// Generating a ternary expression, which is supported in GLSL (scalar condition only)
if (cond.dataType !== UnknownData && cond.dataType.type.startsWith('vec')) {
throw new Error(`GLSL select() with scalar branches requires a scalar boolean condition`);
}

return `(${this.ctx.resolveSnippet(cond).value} ? ${this.ctx.resolveSnippet(truthy).value} : ${this.ctx.resolveSnippet(falsy).value})`;
}

if (name === 'saturate') {
const [arg] = args;
if (!arg) {
throw new Error(`Invalid number of arguments for 'saturate'`);
}
return super.call('clamp', [], [arg, snip(0, d.f32, 'constant'), snip(1, d.f32, 'constant')]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This emits clamp(x, 0f, 1f), but 0f/1f are not valid GLSL ES 3.0 literals — GLSL float constants require a decimal point (or exponent). Verified with glslang (Khronos reference): both clamp(float(v), 0f, 1f) and clamp(vec3, 0f, 1f) fail with "float literal needs a decimal point or exponent"; with 0.0/1.0 the same calls compile. The new snapshot test pins exactly this broken output, so std.saturate via @typegpu/gl will fail at shader-compile time at runtime.

Suggested change
return super.call('clamp', [], [arg, snip(0, d.f32, 'constant'), snip(1, d.f32, 'constant')]);
const [arg] = args;
if (!arg) {
throw new Error(`Invalid number of arguments for 'saturate'`);
}
return super.call(
'clamp',
[],
[arg, snip(0.0, d.f32, 'constant'), snip(1.0, d.f32, 'constant')],
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

frog is right about 0f and 1f constants, but sadly its fix won't work

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix for proper GLSL constants is in a PR that's stacked on top of this one

}

return super.call(name, templateParams, args);
}

override _emitVarDecl(
_keyword: 'var' | 'let' | 'const',
name: string,
Expand Down
120 changes: 120 additions & 0 deletions packages/typegpu-gl/tests/glslGenerator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,126 @@ describe('GlslGenerator - variable declarations', () => {
});
});

describe('GlslGenerator - standard function calls', () => {
it('translates scalar `select()` to ternary expression', () => {
function foo() {
'use gpu';
const cond = false;
return std.select(0, 1, cond);
}

expect(tgpu.resolve([foo], glOptions())).toMatchInlineSnapshot(`
"int foo() {
bool cond = false;
return (cond ? 1i : 0i);
}"
`);
});

it('translates vector `select()` to mix()', () => {
function foo() {
'use gpu';
const cond = false;
const vecCond = d.vec3b(false, true, false);
const bar = std.select(d.vec3f(0), d.vec3f(1), cond); // `cond` should be coerced to a boolean vector
const baz = std.select(d.vec3f(1), d.vec3f(0), vecCond);
}

expect(tgpu.resolve([foo], glOptions())).toMatchInlineSnapshot(`
"void foo() {
bool cond = false;
bvec3 vecCond = bvec3(false, true, false);
vec3 bar = mix(vec3(), vec3(1), bvec3(cond));
vec3 baz = mix(vec3(1), vec3(), vecCond);
}"
`);
});

it('should throw on select() with vector cond and scalar branches', () => {
function foo() {
'use gpu';
const cond = d.vec3b(false, true, false);
// @ts-ignore
return std.select(0, 1, cond);
}

expect(() => tgpu.resolve([foo], glOptions())).toThrowErrorMatchingInlineSnapshot(`
[Error: Resolution of the following tree failed:
- <root>
- fn*:foo
- fn*:foo()
- fn:select: GLSL select() with scalar branches requires a scalar boolean condition]
`);
});

it('translates `saturate(v)` to `clamp(v, 0.0, 1.0)`', () => {
function foo() {
'use gpu';
const scalar = 2;
const vec3 = d.vec3f(1, 2, 3);
std.saturate(scalar);
std.saturate(vec3);
}

expect(tgpu.resolve([foo], glOptions())).toMatchInlineSnapshot(`
"void foo() {
int scalar = 2;
vec3 vec3_1 = vec3(1, 2, 3);
clamp(float(scalar), 0f, 1f);
clamp(vec3_1, 0f, 1f);
}"
`);
});

it('translates bitcast', () => {
function foo() {
'use gpu';
const f = d.f32(1.5);
const f2 = d.vec2f(1.5);
const u = d.u32(15);
const u2 = d.vec2u(15);
const i = d.i32(-5);
const i2 = d.vec2i(-5);

std.bitcast(d.f32, d.f32)(f); //no-op
std.bitcast(d.u32, d.u32)(u); //no-op
std.bitcast(d.i32, d.i32)(i); //no-op

std.bitcast(d.f32, d.u32)(f);
std.bitcast(d.f32, d.i32)(f);
std.bitcast(d.u32, d.f32)(u);
std.bitcast(d.i32, d.f32)(i);

std.bitcast(d.vec2f, d.vec2u)(f2);
std.bitcast(d.vec2f, d.vec2i)(f2);
std.bitcast(d.vec2u, d.vec2f)(u2);
std.bitcast(d.vec2i, d.vec2f)(i2);
}

expect(tgpu.resolve([foo], glOptions())).toMatchInlineSnapshot(`
"void foo() {
float f = 1.5f;
vec2 f2 = vec2(1.5);
uint u = 15u;
uvec2 u2 = uvec2(15);
int i = -5i;
ivec2 i2 = ivec2(-5);
f;
u;
i;
floatBitsToUint(f);
floatBitsToInt(f);
uintBitsToFloat(u);
intBitsToFloat(i);
floatBitsToUint(f2);
floatBitsToInt(f2);
uintBitsToFloat(u2);
intBitsToFloat(i2);
}"
`);
});
});

describe('GlslGenerator - function definitions', () => {
it('generates proper function signatures', () => {
function add(a: number, b: number) {
Expand Down
11 changes: 8 additions & 3 deletions packages/typegpu/src/core/function/dualImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ type AnyFn = (...args: never[]) => unknown;
interface DualImplOptions<T extends AnyFn> {
readonly name: string | undefined;
readonly normalImpl: T | string;
readonly codegenImpl: (ctx: ResolutionCtx, args: MapValueToSnippet<Parameters<T>>) => string;
readonly codegenImpl: (
ctx: ResolutionCtx,
args: MapValueToSnippet<Parameters<T>>,
returnType: BaseData,
) => string;
readonly signature:
| {
argTypes: (BaseData | BaseData[])[];
Expand Down Expand Up @@ -117,9 +121,10 @@ export function dualImpl<T extends AnyFn>(options: DualImplOptions<T>): DualFn<T

const possibleSideEffects = options.sideEffects || args.some((a) => a.possibleSideEffects);

const concreteReturnType = concretize(returnType);
return snip(
options.codegenImpl(ctx, converted),
concretize(returnType),
options.codegenImpl(ctx, converted, concreteReturnType),
concreteReturnType,
// Functions give up ownership of their return value
/* origin */ 'runtime',
possibleSideEffects,
Expand Down
5 changes: 2 additions & 3 deletions packages/typegpu/src/std/array.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { dualImpl } from '../core/function/dualImpl.ts';
import { stitch } from '../core/resolve/stitch.ts';
import { abstractInt, u32 } from '../data/numeric.ts';
import { ptrFn } from '../data/ptr.ts';
import { type _ref as ref, isRef } from '../data/ref.ts';
Expand All @@ -18,9 +17,9 @@ export const arrayLength = dualImpl({
};
},
normalImpl: (a: unknown[] | ref<unknown[]>) => (isRef(a) ? a.$.length : a.length),
codegenImpl(_ctx, [a]) {
codegenImpl(ctx, [a]) {
const length = sizeOfPointedToArray(a.dataType);
return length > 0 ? `${length}` : stitch`arrayLength(${a})`;
return length > 0 ? `${length}` : ctx.gen.call('arrayLength', [], [a]);
},
sideEffects: false,
});
22 changes: 8 additions & 14 deletions packages/typegpu/src/std/bitcast.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { dualImpl } from '../core/function/dualImpl.ts';
import { stitch } from '../core/resolve/stitch.ts';
import {
bitcastF32toU32Impl,
bitcastU32toF32Impl,
Expand Down Expand Up @@ -44,6 +43,7 @@ import { SignatureNotSupportedError } from '../errors.ts';
import { getName } from '../shared/meta.ts';
import type { Infer } from '../shared/repr.ts';
import { comptime } from '../core/function/comptime.ts';
import { coerceToSnippet } from '../tgsl/generationHelpers.ts';

type BitcastU32toF32Overload = <T extends number | v2u | v3u | v4u>(
value: T,
Expand All @@ -64,10 +64,8 @@ export const bitcastU32toF32 = dualImpl({
}
return VectorOps.bitcastU32toF32[value.kind](value);
}) as BitcastU32toF32Overload,
codegenImpl: (_ctx, [n]) => {
return isVec(n.dataType)
? stitch`bitcast<vec${n.dataType.componentCount}f>(${n})`
: stitch`bitcast<f32>(${n})`;
codegenImpl: (ctx, [n], returnType) => {
return ctx.gen.call('bitcast', [coerceToSnippet(returnType)], [n]);
},
signature: (...arg) => {
const uargs = unifyStrict(arg, u32AllowedSchemas);
Expand Down Expand Up @@ -103,10 +101,8 @@ export const bitcastU32toI32 = dualImpl({
}
return VectorOps.bitcastU32toI32[value.kind](value);
}) as BitcastU32toI32Overload,
codegenImpl: (_ctx, [n]) => {
return isVec(n.dataType)
? stitch`bitcast<vec${n.dataType.componentCount}i>(${n})`
: stitch`bitcast<i32>(${n})`;
codegenImpl: (ctx, [n], returnType) => {
return ctx.gen.call('bitcast', [coerceToSnippet(returnType)], [n]);
},
signature: (...arg) => {
const uargs = unifyStrict(arg, u32AllowedSchemas);
Expand Down Expand Up @@ -144,10 +140,8 @@ export const bitcastF32toU32 = dualImpl({
}
return VectorOps.bitcastF32toU32[value.kind](value);
}) as BitcastF32toU32Overload,
codegenImpl: (_ctx, [n]) => {
return isVec(n.dataType)
? stitch`bitcast<vec${n.dataType.componentCount}u>(${n})`
: stitch`bitcast<u32>(${n})`;
codegenImpl: (ctx, [n], returnType) => {
return ctx.gen.call('bitcast', [coerceToSnippet(returnType)], [n]);
},
signature: (...arg) => {
const uargs = unifyStrict(arg, f32AllowedSchemas);
Expand Down Expand Up @@ -283,7 +277,7 @@ function bitcastFor<In extends BitcastAllowedTypes, Out extends BitcastAllowedTy
return dualImpl({
name: 'bitcast',
normalImpl: getCpuBitcast<In, Out>(inType, outType),
codegenImpl: (_ctx, [n]) => stitch`bitcast<${outType.type}>(${n})`,
codegenImpl: (ctx, [n]) => ctx.gen.call('bitcast', [coerceToSnippet(outType)], [n]),
signature: (arg) => {
const uarg = unifyStrict([arg], [inType]);
if (!uarg) {
Expand Down
2 changes: 1 addition & 1 deletion packages/typegpu/src/std/boolean.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ export const select = dualImpl({
},
normalImpl: cpuSelect,
codegenImpl: (ctx, [f, t, cond]) => {
const result = stitch`select(${f}, ${t}, ${cond})`;
const result = ctx.gen.call('select', [], [f, t, cond]);
Comment thread
iwoplaza marked this conversation as resolved.
if (
!validSelectBranchTypes.includes(f.dataType as AnyWgslData) ||
!validSelectBranchTypes.includes(t.dataType as AnyWgslData)
Expand Down
2 changes: 1 addition & 1 deletion packages/typegpu/src/std/numeric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,7 @@ export const saturate = dualImpl({
name: 'saturate',
signature: unifyRestrictedSignature(anyFloat),
normalImpl: cpuSaturate,
codegenImpl: (_ctx, [value]) => stitch`saturate(${value})`,
codegenImpl: (ctx, [value]) => ctx.gen.call('saturate', [], [value]),
Comment thread
iwoplaza marked this conversation as resolved.
sideEffects: false,
});

Expand Down
3 changes: 2 additions & 1 deletion packages/typegpu/src/tgsl/shaderGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export interface ShaderGenerator {
functionDefinition(options: FunctionDefinitionOptions): string;

typeInstantiation(schema: BaseData, args: readonly Snippet[]): ResolvedSnippet;
typeAnnotation(schema: BaseData): string;
numericLiteral(value: number, schema: BaseData): ResolvedSnippet;
typeAnnotation(schema: BaseData): string;
call(name: string, templateParams: readonly Snippet[], args: readonly Snippet[]): string;
}
12 changes: 12 additions & 0 deletions packages/typegpu/src/tgsl/wgslGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,18 @@ ${this.ctx.pre}}`;
return snip(base, schema, /* origin */ 'constant', false);
}

public call(name: string, templateParams: readonly Snippet[], args: readonly Snippet[]): string {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we plan on routing all calls through here? Is there an issue for tracking that?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just made one #2829

const resolvedTemplateParams = templateParams
.map((arg) => this.ctx.resolveSnippet(arg).value)
.join(', ');
const resolvedArgs = args.map((arg) => this.ctx.resolveSnippet(arg).value).join(', ');

if (resolvedTemplateParams.length > 0) {
return `${name}<${resolvedTemplateParams}>(${resolvedArgs})`;
}
return `${name}(${resolvedArgs})`;
}

protected _return(statement: tinyest.Return): string {
const returnNode = statement[1];

Expand Down
Loading