Wasm support and some cleanup. #18
Conversation
| canvas := document.Call("createElement", "canvas") | ||
| if canvas == js.Null() { | ||
| fmt.Println("CANVAS IS NULL") |
There was a problem hiding this comment.
Early return would be nice here.
There was a problem hiding this comment.
OK. I was just using the existing example but yea sounds like an improvement. Will add that.
webgl.go
Outdated
| gl := canvas.Call("getContext", "webgl", attrs) | ||
| if gl == nil { | ||
| if gl == js.Null() { |
There was a problem hiding this comment.
Can this be js.Undefined()? I'm not confident on this.
webgl.go
Outdated
| gl = canvas.Call("getContext", "experimental-webgl", attrs) | ||
| if gl == nil { | ||
| if gl == js.Null() { |
| } | ||
| ``` | ||
|
|
||
| webgl_example.html: | ||
| js_example.html: |
There was a problem hiding this comment.
I renamed it to make a clearer distinction between the JS and the WASM example. Webgl seemed ambiguous.
webgl.go
Outdated
| for i := 0; i < num; i++ { | ||
| field := fields.Field(i) | ||
| // Ignore the embedded js.Value. All other fields are gl constants. | ||
| if field.Name != "Value" { |
There was a problem hiding this comment.
Early return would make this more readable:
if field.Name == "Value" {
continue
}
// ...Wdyt?
There was a problem hiding this comment.
Sure for me both are fine. I can change it.
webgl.go
Outdated
| if field.Name != "Value" { | ||
| // Retrieve value from gl context. | ||
| jsval := ctx.Get(field.Name) | ||
| if jsval.Type() == js.TypeNumber { |
There was a problem hiding this comment.
When is the type not number? If we cannot expect, should it return error or just panic?
There was a problem hiding this comment.
E.g. the type is not a number if the GL constant does not exist in webgl. I discovered a few of them through this and removed them from the constants above.
I checked for "not number" so we can just ignore all missing values. In fact there should be no missing values now. Maybe we should just panic here if something is missing?
There was a problem hiding this comment.
Sure, I prefer panic since the constants in the spec must be obtained.
It looks like the removed constants might be defined in extensions?
There was a problem hiding this comment.
The constants I removed are the following:
INFO_LOG_LENGTH
not needed in webgl.
NUM_COMPRESSED_TEXTURE_FORMATS
maybe related to https://developer.mozilla.org/en-US/docs/Web/API/WebGL_API/Compressed_texture_formats
SHADER_COMPILER
no idea what this is
SHADER_SOURCE_LENGTH
not needed in webgl
STENCIL_INDEX
not in spec. see KhronosGroup/WebGL#2370 (comment)
|
@hajimehoshi what do you think about the current state? |
|
Sorry about the slow response. My concern here is that this project doesn't have an active owner/maintainer. Adding more functionality here makes it even harder to maintain. This was the very first If the Wasm port lived in another repository where it's more clear who develops/maintains it, that might be a more manageable solution. It would be less constrained by unnecessary backwards compatibility. Alternatively, if we can add a maintainer to this repo. But I don't know what it means to be breaking API compatibility here, especially after this package has inactive for so long. |
Hi,
first of all I don't expect this to get merged already!
This PR adds wasm support via
github.com/gopherjs/gopherwasm/jsto this package but js via gopherjs should also still work. I also removed a few gl constants that are undefined in webgl. And last I changedTexImage2Dto accept a[]byteslice as wished in #11 .Note that the new function
(c *Context) initGlConstants()is needed because wasm does not support struct tags. It uses reflection because else we would need to list all gl constants again. If you think this would be better that's no problem to change it. I already have the equivalent function generated and ready.The struct tags are removed because Gopherjs makes trouble if you try to set a value for a field with struct tag (probably a bug). But we don't need them anyways.
Let me know what you think!