gpujs / gpu.js

GPU Accelerated JavaScript
https://gpu.rocks
MIT License
15.07k stars 650 forks source link

Math.max and Math.min only accept two parameters #618

Open TomWyllie opened 4 years ago

TomWyllie commented 4 years ago

A GIF or MEME to give some spice of the internet

What is wrong?

Using Math.max with more than two parameters causes the GPU kernel build to fail.

How do we replicate the issue?

const gpu = new GPU({ mode: 'gpu' });
const kernel = gpu.createKernel(function() {
    return Math.max(0, 1, 2);
}).setOutput([1]);
kernel();

And observe the output error gpu-browser.js:14913 Uncaught Error: Error compiling fragment shader: ERROR: 0:457: 'max' : no matching overloaded function found. Replacing 0, 1, 2 with 0, 1 (or any other two numbers) leads to the function behaving as expected. The same is true of Math.min. JS Fiddle: https://jsfiddle.net/TomWyllie/opdg95kb/

GPU.js 2.9.4 GPU: NVIDIA GeForce GTX 1060 3GB Nvidia Driver 446.14 Google Chrome Version 83.0.4103.97 (Official Build) (64-bit) Windows 10

How important is this (1-5)?

2, it's very easy (but pretty messy) to workaround by using Math.max(x, Math.max(y, Math.max(z, t))); or similar, but that is pretty ugly. My understanding is that the spec allows arbitrarily many parameters.

Other Comments

I'm beginning to think there might be something deeper wrong with GPU.js on my machine given this issue and the other I filed several days ago (#617) must be very common use cases and surely lots of other people would be having these same problems... any thoughts appreciated.

TomWyllie commented 4 years ago

This is probably due to the ESSL specifying that the max and min functions can only take two variables. See page 66 of the spec. Perhaps when parsing the kernel code GPU.js could automatically expand Math.max(x, y, z) to Math.max(Math.max(x, y), z); or similar? I spent quite a bit of time debugging this reckoning it was the types of my variables that were wrong rather than that only two parameters were supported...