gowebapi / webapi

Go Lang Web Assembly bindings for DOM, HTML etc
BSD 3-Clause "New" or "Revised" License
174 stars 12 forks source link

Problematic to bind NULL values in webgl #20

Open ptxmac opened 1 year ago

ptxmac commented 1 year ago

A typical pattern in WebGL is to bind a "null" value.

E.g.

gl.bindFramebuffer(gl.FRAMEBUFFER, null);

According to the documentation, this is how to reset the current FBO back to the canvas:

framebuffer A WebGLFramebuffer object to bind, or null for binding the Canvas or OffscreenCanvas object associated with the rendering context.

Because of how the types are implemented in WebGL, this is not elegant to do

Preferred pattern:

gl.BindFramebuffer(webgl.FRAMEBUFFER, webgl.FramebufferFromJS(js.Null()))

Unfortunately this will panic

Problem

From webgl_js.go:

// FramebufferFromJS is casting a js.Value into Framebuffer.
func FramebufferFromJS(value js.Value) *Framebuffer {
    if typ := value.Type(); typ == js.TypeNull || typ == js.TypeUndefined {
        return nil
    }
    ret := &Framebuffer{}
    ret.Value_JS = value
    return ret
}

In the case of js.Null() this returns a nil pointer, which we try to pass to BindFramebuffer

func (_this *RenderingContext) BindFramebuffer(target uint, framebuffer *Framebuffer) {
    var (
        _args [2]interface{}
        _end  int
    )
    _p0 := target
    _args[0] = _p0
    _end++
    _p1 := framebuffer.JSValue()
    _args[1] = _p1
    _end++
    _this.Value_JS.Call("bindFramebuffer", _args[0:_end]...)
    return
}

Unfortunately, this code expected a non-nil Framebuffer, and will panic

Workaround

It's possible to work around it by manually creating the Framebuffer object, but it's not elegant:

nullFBO := &webgl.Framebuffer{Object: webgl.Object{
    Value_JS: js.Null(),
}}
gl.BindFramebuffer(webgl.FRAMEBUFFER, nullFBO)

Solutions

I can think of a couple of solutions:

  1. Simple remove the IsNull check from the *FromJS methods and allow the creation of null objects. gl.BindFramebuffer(webgl.FRAMEBUFFER, webgl.FramebufferFromJS(js.Null()))
  2. Create global Null versions of objects. gl.BindFramebuffer(webgl.FRAMEBUFFER, webgl.NullFramebuffer)
  3. Change the bind methods and convert nil to js.Null gl.BindFramebuffer(webgl.FRAMEBUFFER, nil)

Notes

From my OpenGL understanding this pattern is used for all objects. I.e. all Bind* and Use* methods (gl.bindTexture/gl.useProgram/gl.bindBuffer etc)

The mozdev webgl documentation is unfortunately a little lacking, and only framebuffers are explicitly mentioned to be null - but I assume it also holds true for any other objects

janpfeifer commented 1 year ago

Thanks for the detailed report Peter.

I haven't used it in a while (but I'm planning to next year), but another solution that comes to mind, to preserve the interchangeability between Go's nil and JS's Null, would be changing the .JSValue() method to convert nil to the a js.Null. So the line below would work:

func (_this *RenderingContext) BindFramebuffer(target uint, framebuffer *Framebuffer) {
    ...
    _p1 := framebuffer.JSValue()
    ...

What do you think ? Would that work ?

ptxmac commented 1 year ago

I think that's more or less what I meant with my 3rd suggestion. I.e. from the users perspective, it would look like the following?

gl.BindFramebuffer(webgl.FRAMEBUFFER, nil)

(edited)

So you're suggesting making a change like this?

func (_this *Object) JSValue() js.Value {
+   if this == nil {
+      return js.Null()
+   }
    return _this.Value_JS
}

Looks good to me, I can't think of any time where a panic is preferred to js.Null 😄

The only thing to be mindful of is null vs undefined - I don't think there's anywhere in WebGL where you would use undefined, but that's a little outside my experience

janpfeifer commented 1 year ago

Yes, that's what I had in mind.

I'm also no JavaScript expert, so I don't have an opinion on the null vs undefined choice. But as you said, better than panic.

Do you want to write the PR ? I'll approve it. I would just ask to add a line of comment on the JSValue() method about nil being converted to null in JavaScript.

ptxmac commented 1 year ago

Do you want to write the PR ? I'll approve it. I would just ask to add a line of comment on the JSValue() method about nil being converted to null in JavaScript.

Yup, I'll take a stab at it

ptxmac commented 1 year ago

Created the first attempt in https://github.com/gowebapi/webidl-bind/pull/11

ptxmac commented 1 year ago

This can now be resolved if the files are re-generated

ptxmac commented 1 year ago

@janpfeifer could to re-generate the files?

(Or should I create a PR with it?)

janpfeifer commented 1 year ago

hi @ptxmac, sorry I missed it. I was hoping you would re-generate the files as well. I only did it once, > 1 year ago, I would have to investigate how to do it.

I'm currently finishing a different project that doesn't use gowebapi. I'm planning to get back to it next year, in a couple of months.