go-gl / gl

Go bindings for OpenGL (generated via glow)
MIT License
1.07k stars 74 forks source link

all: Regenerate all bindings with new XML data. #93

Closed dmitshur closed 6 years ago

dmitshur commented 6 years ago

Despite being entirely generated, we should review this PR with higher scrutiny. This uses the new source of XML data, and there are some changes and additions to the existing packages. We should make sure everything looks good before committing to this (otherwise, we might need to change glow first).

This is based on top of go-gl/glow#95.

Done with latest version of glow and newly downloaded XML data:

go generate -tags=gen github.com/go-gl/gl
dmitshur commented 6 years ago

I have the following question. Why are there so many additions to v2.1/gl/package.go? Are the things that are added new extensions, and it's normal/expected to see so many new symbols in an old OpenGL version?

errcw commented 6 years ago

I'm not entirely sure what happened to v2.1, to the best of my knowledge the additions are all extensions. I think we've been further out of date than we thought by continuing to use the old SVN repository.

I'll leave it to you to merge this PR, just so you can double-check the changes.

dmitshur commented 6 years ago

Thanks.

I'll test this locally a bit, and merge soon thereafter.

dmitshur commented 6 years ago

I've tested with some of my projects and did not find any issues.

It's still concerning to me that this change adds 69k lines while removing only 9k (so net +60k increase). From looking into where the additions come from, it is all extensions. v2.1 package ends up being one of the largest (even bigger than all-core). The v3.x and v4.x packages actually remove a lot of stuff while adding very little core stuff.

I've downloaded XML files again via glow download to see what would change since last time (just 15 days ago, in go-gl/glow#95). It adds an extension called GL_EXT_EGL_image_storage, which comes with glEGLImageTargetTexStorageEXT, and that func is added as a result to all generated gl packages.

I guess this is fine, as long as we decide to include all extensions, that is the expected outcome. Unfortunately, OpenGL together with extensions is a very sizeable API.

I will merge this because it's functional and there are no unexpected issues. But we should keep an eye on the binding sizes. If they get even larger, or the current large size starts causing issues, we might want to rethink how many extensions we include.