fix: Choose f32 when trying to find a type common with abstractFloat and i32 (or u32)

Author: iwoplaza

packages/typegpu/src/tgsl/conversion.ts

@@ -1,6 +1,7 @@
 import { stitch } from '../core/resolve/stitch.ts';
 import { UnknownData } from '../data/dataTypes.ts';
 import { undecorate } from '../data/dataTypes.ts';
+import { f32 } from '../data/numeric.ts';
 import { derefSnippet, RefOperator } from '../data/ref.ts';
 import { schemaCallWrapperGPU } from '../data/schemaCallWrapper.ts';
 import { snip, type Snippet } from '../data/snippet.ts';
@@ -120,12 +121,18 @@ function getImplicitConversionRank(src: BaseData, dest: BaseData): ConversionRan
     }
   }
 
+  if ((trueSrc.type === 'u32' || trueSrc.type === 'i32') && trueDst.type === 'abstractFloat') {
+    // When one of the types is a float (abstract or not), we don't want to cast it to a non-float type,
+    // which would cause it to lose precision. We instead choose the common type to be f32.
+    return { rank: 1, action: 'cast', targetType: f32 };
+  }
+
   if (trueSrc.type === 'abstractFloat') {
     if (trueDst.type === 'u32') {
-      return { rank: 2, action: 'cast', targetType: trueDst };
+      return { rank: 3, action: 'cast', targetType: trueDst };
     }
     if (trueDst.type === 'i32') {
-      return { rank: 1, action: 'cast', targetType: trueDst };
+      return { rank: 2, action: 'cast', targetType: trueDst };
     }
   }
 
@@ -167,6 +174,10 @@ function findBestType(
   let bestResult: { type: BaseData; details: ConversionRankInfo[]; sum: number } | undefined;
 
   for (const targetType of uniqueTypes) {
+    /**
+     * The type we end up converting to. Will be different than `targetType` if `targetType === abstractFloat`
+     */
+    let destType = targetType;
     const details: ConversionRankInfo[] = [];
     let sum = 0;
     for (const sourceType of types) {
@@ -176,9 +187,12 @@ function findBestType(
         break;
       }
       details.push(conversion);
+      if (conversion.action === 'cast') {
+        destType = conversion.targetType;
+      }
     }
     if (sum < (bestResult?.sum ?? Number.POSITIVE_INFINITY)) {
-      bestResult = { type: targetType, details, sum };
+      bestResult = { type: destType, details, sum };
     }
   }
   if (!bestResult) {

packages/typegpu/tests/indent.test.ts

@@ -333,7 +333,7 @@ describe('indents', () => {
     })((input) => {
       'use gpu';
       const uniBoid = layout.$.boids;
-      for (let i = d.u32(); i < std.floor(std.sin(Math.PI / 2)); i++) {
+      for (let i = d.u32(); i < d.u32(std.sin(Math.PI / 2)); i++) {
         const sampled = std.textureSample(layout.$.sampled, layout.$.sampler, d.vec2f(0.5, 0.5), i);
         const someVal = std.textureLoad(layout.$.smoothRender, d.vec2i(), 0);
         if (someVal.x + sampled.x > 0.5) {

packages/typegpu/tests/tgsl/conversion.test.ts

@@ -260,6 +260,12 @@ describe('convertToCommonType', () => {
     expect(result).toBeUndefined();
   });
 
+  it('chooses abstractFloat over i32', () => {
+    const result = convertToCommonType(ctx, [snippetI32, snippetAbsFloat]);
+    expect(result).toBeDefined();
+    expect(result?.[0]?.dataType.type).toBe('f32');
+  });
+
   it('respects restrictTo types', () => {
     // [abstractInt, i32] -> common type i32
     // Restrict to f32: requires cast for i32

packages/typegpu/tests/tgsl/multiplication.test.ts

@@ -0,0 +1,129 @@
+import { test } from 'typegpu-testing-utility';
+import { expect, vi } from 'vitest';
+import tgpu, { d } from 'typegpu';
+
+import { expectSnippetOf } from '../utils/parseResolved.ts';
+
+test('multiplying i32 with a float literal should implicitly convert to an f32', () => {
+  using consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+
+  function main() {
+    'use gpu';
+    const a = d.i32(1) * 0.001;
+    const int = d.i32(1);
+    return int * 0.001;
+  }
+
+  expect(tgpu.resolve([main])).toMatchInlineSnapshot(`
+    "fn main() -> f32 {
+      const a = 1e-3f;
+      const int = 1i;
+      return (f32(int) * 1e-3f);
+    }"
+  `);
+
+  expectSnippetOf(() => {
+    'use gpu';
+    return d.i32(1) * 0.001;
+  }).toStrictEqual([0.001, d.f32, 'constant']);
+
+  expect(consoleWarnSpy.mock.calls).toMatchInlineSnapshot(`
+    [
+      [
+        "Implicit conversions from [
+      1: i32
+    ] to f32 are supported, but not recommended.
+    Consider using explicit conversions instead.",
+      ],
+      [
+        "Implicit conversions from [
+      int: i32
+    ] to f32 are supported, but not recommended.
+    Consider using explicit conversions instead.",
+      ],
+      [
+        "Implicit conversions from [
+      1: i32
+    ] to f32 are supported, but not recommended.
+    Consider using explicit conversions instead.",
+      ],
+    ]
+  `);
+});
+
+test('multiplying u32 with a float literal should implicitly convert to an f32', () => {
+  using consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+
+  function main() {
+    'use gpu';
+    const a = d.u32(10) * 0.001;
+    const int = d.u32(100);
+    return int * 0.001;
+  }
+
+  expect(tgpu.resolve([main])).toMatchInlineSnapshot(`
+    "fn main() -> f32 {
+      const a = 0.01f;
+      const int = 100u;
+      return (f32(int) * 1e-3f);
+    }"
+  `);
+
+  expectSnippetOf(() => {
+    'use gpu';
+    return d.u32(1) * 0.001;
+  }).toStrictEqual([0.001, d.f32, 'constant']);
+
+  expect(consoleWarnSpy.mock.calls).toMatchInlineSnapshot(`
+    [
+      [
+        "Implicit conversions from [
+      10: u32
+    ] to f32 are supported, but not recommended.
+    Consider using explicit conversions instead.",
+      ],
+      [
+        "Implicit conversions from [
+      int: u32
+    ] to f32 are supported, but not recommended.
+    Consider using explicit conversions instead.",
+      ],
+      [
+        "Implicit conversions from [
+      1: u32
+    ] to f32 are supported, but not recommended.
+    Consider using explicit conversions instead.",
+      ],
+    ]
+  `);
+});
+
+test('multiplying u32 with an i32 should implicitly convert to an i32', () => {
+  using consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+
+  function main() {
+    'use gpu';
+    const uint = d.u32(5);
+    const int = d.i32(3);
+    return uint * int;
+  }
+
+  expect(tgpu.resolve([main])).toMatchInlineSnapshot(`
+    "fn main() -> i32 {
+      const uint = 5u;
+      const int = 3i;
+      return (i32(uint) * int);
+    }"
+  `);
+
+  expect(consoleWarnSpy.mock.calls).toMatchInlineSnapshot(`
+    [
+      [
+        "Implicit conversions from [
+      uint: u32
+    ] to i32 are supported, but not recommended.
+    Consider using explicit conversions instead.",
+      ],
+    ]
+  `);
+});