greggman / twgl.js

A Tiny WebGL helper Library
http://twgljs.org
MIT License
2.67k stars 260 forks source link

helper.isTexture() gives false-negative result since v4.9.3 when running in node #159

Closed cwill closed 4 years ago

cwill commented 4 years ago

After upgrading past this commit: https://github.com/greggman/twgl.js/commit/c1aebd79c456d34ee6cf306741ee1bef9501dfea I've found that the following code stopped working while trying to set up a framebuffer in a headless nodejs server:

function makeFramebuffer(gl, width, height) {
  const attachments = [{
      width,
      height,
      format: gl.RGBA,
      internalFormat: gl.RGBA32F,
      minMag: gl.NEAREST,
      wrap: gl.CLAMP_TO_EDGE,
      type: gl.FLOAT,
      auto: false,
      src: new Float32Array(width*height*4),
  }];
  return twgl.createFramebufferInfo(gl, attachments, width, height);
}

Because Node.js doesn't supply the same DOM & WebGL class constructors that are available in browsers, helper.isTexture() is returning false even though the object passed to it really is a WebGLTexture instance, because the global WebGLTexture isn't defined.

I think a workaround using gl.isTexture(), gl.isRenderbuffer(), etc. directly could potentially work as a fallback when the constructor is undefined, but don't have quite enough context to know for certain if that's compatible with twgl's approach. If I have a chance to experiment I'll report back here.

greggman commented 4 years ago

I'm not sure what the solutions is yet but FYI, gl.isTexture and gl.isRenderbuffer don't tell you something is a WebGLTexture or a WebGLRenderBufffer. They only tell you at an OpenGL level if the id is a valid texture id or renderbuffer id. In that API you can have ids that are valid ids but are not valid textures or valid renderbuffers. You can also have ids that still reference a valid texture or valid renderbuffer that will return false from those tests and there are ways to reproduce those issues in WebGL so they are not the correct solution.

https://jsfiddle.net/greggman/1ftkw8s5/

On top of that most queries, including those, are very expensive. They require stalling the GPU pipeline to make sure that all commands before the query have been executed.

Having WebGLRenderBuffer and WebGLTexture are required by the WebGL spec so maybe look into getting the node implementation to add those would be one solution

cwill commented 4 years ago

Ah, shame, figured there must be some reason those weren't already in use, that's tricky. Thanks for looking into it.

I'll see if there's a convenient way to expose the needed symbols, in the worst case it should be feasible to shim via a transpiler step or similar.

greggman commented 4 years ago

which node webgl library are you using and what are you using it for? For example you could use puppeteer to use webgl from node. Kind of giant but just saying :P

cwill commented 4 years ago

We're using wpe-webgl to run a webgl context on a raspberry pi to render content for an unusual display (the rendered frames are ultimately getting sent over the network to an LED controller). It's possible this is only an issue with that particular library, since it exports the necessary classes rather than defining them as globals as would be expected with the typical DOM-like environment.

Totally understand if this doesn't seem like a feasible use case to support (feel free to close if so), though I might send a PR if I manage to get something working and it's not easily fixable with a shim or build system hackery on my side.

greggman commented 4 years ago

Doesn't something like

const webgl = require('wpe-webgl');
global.WebGLTexture = webgl.WebGLTexture;
...

work? Or however you get the types out

I didn't test wpe-webgl because it required installing a bunch of system level libs and tools but testing the idea out works

https://glitch.com/edit/#!/bubbly-supernova?path=test.js:6:0

cwill commented 4 years ago

That seems to do the trick! Thanks, knew I was overcomplicating things somewhere.

cwill commented 4 years ago

(If you're curious, the display in question is this: https://drive.google.com/file/d/1Yw_jIw4Mlb2aVq-CIKyt0Nx-PSDKWcpw/view?usp=sharing )