gpujs / gpu.js

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

Setting constant sampler2D override the input sampler2D #634

Open poonwu opened 4 years ago

poonwu commented 4 years ago

Setting constants somehow override the input in GPU mode. This behavior, of course, does not happen in CPU mode.

const {GPU, Kernel} = require('gpu.js')

const gpu = new GPU({mode: 'gpu'})
const cpu = new GPU({mode: 'cpu'})

function identity(m) {
  return [m[this.thread.x][0], m[this.thread.x][1], m[this.thread.x][2]];
}

let gkernel = gpu.createKernel(identity, {
  output: [1]
})
let ckernel = cpu.createKernel(identity, {
  output: [1]
})

gkernel.setConstants({
  p: [[0,1,1]]
})
ckernel.setConstants({
  p: [[0,1,1]]
})

const data = [[111,222,333]];

const result0 = gkernel(data);
const result1 = ckernel(data);

console.log('GPU', result0);
console.log('CPU', result1);

/**
$ node gputest.js
GPU [ Float32Array(3) [ 0, 1, 1 ] ]
CPU [ Float32Array(3) [ 111, 222, 333 ] ]
*/

I am using Node v14.4.0 and GPU.js 2.9.5

How important is this (1-5)?

4 or 5. This is related to the core function of gpu.js

Expected behavior

Should return same values as CPU counterpart.

Other Comments

I checked the compiled shader code, and nothing much different between no setConstants and setConstants other than extra uniform sampler field. That field isn't even accessed, so I can only suspect that the lib might be sending constant texture to the wrong channel...

poonwu commented 4 years ago

Nevermind, it's my mistake. I believe the matrices are intended to be passed as input only, not constants. Apparently, we really need better constants documentation.

So, to solve above case, just don't set texture as constants and correct the kernel to something like identity(m, n) where m,n are matrices.

robertleeplummerjr commented 4 years ago

I believe the matrices are intended to be passed as input only, not constants.

That is incorrect, this is a bug.