gpujs / gpu.js

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

The GPU property is assigned and defined resulting in a TypeError in Chrome #639

Open WesselKroos opened 3 years ago

WesselKroos commented 3 years ago

First of all, great work on gpu.js! It's crazy to be able to apply video sharpening in realtime in a browser on 1080p60 videos. 👍

What is wrong?

The following TypeError is thrown in the developer console when I load gpu.js in Chrome via the gpu-browser.js script: image TypeError: Cannot set property GPU of #<Window> which has only a getter Documentation about the TypeError: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Getter_only

Because the GPU property is defined and assigned to the window object. First via de defineProperty method:

Object.defineProperty(target, 'GPU', {
    get() {
        return GPU;
    }
});

Source: gpu-browser.js#L18060 - npm version 2.10.4

And then again via an assignment to window.GPU:

if (typeof window !== "undefined") {
    g = window;
} else if (typeof global !== "undefined") {
    g = global;
} else if (typeof self !== "undefined") {
    g = self;
} else {
    g = this;
}
g.GPU = f(); // <-- Where the error is thrown

Source: gpu-browser.js#L14 - npm version 2.10.4

Where does it happen?

When loading the gpu.js/dist/gpu-browser.js file in Chrome via the following javascript:

const s = document.createElement('script');
s.src = 'gpu-browser.js';
s.onload = () => {
    s.parentNode.removeChild(s)
};
s.onerror = (e) => {
    console.error('Adding script failed:', e.target.src, e);
};
document.head.appendChild(s);

How do we replicate the issue?

  1. Install gpu.js version 2.10.4 via npm.
  2. Host the node_modules/gpu.js/dist/gpu-browser.js file on a webserver.(https://github.com/gpujs/gpu.js/blob/develop/dist/gpu-browser.js).
  3. Load the script via the javascript code in the "Where does it happen?" section.

How important is this (1-5)?

3 - It still works for my usecase, but the error is in the developer console. I don't know if something else is failing because the script execution stops when this error is thrown.

Expected behavior (i.e. solution)

Define or assign the GPU property to the window object, but don't do both.

WesselKroos commented 2 years ago

To be more clear, this only happens when the script goes through a compiler like rollup/webpack and adds the "use strict"; line to the beginning of the file.

caewok commented 2 years ago

Yes, this fails in my use case. I am using it in a module where I don't have complete control of the compiler, so I think it is adding "use strict" on the backend. My work-around was to rename g.GPU:

if (typeof window !== "undefined") {
    g = window;
} else if (typeof global !== "undefined") {
    g = global;
} else if (typeof self !== "undefined") {
    g = self;
} else {
    g = this;
}
g.myGPU = f(); // <-- Rename to avoid error

The result is two properties added to window: window.GPU pointing to the GPU class, and window.myGPU pointing to all the classes. Appears to work in limited testing.

I don't know enough about the intention here to figure out whether my work-around is sufficient or if something else would be preferable...