go-gl / glow

Go binding generator for OpenGL
MIT License
250 stars 25 forks source link

Lack of extensions in the core profile #77

Closed davidmanzanares closed 7 years ago

davidmanzanares commented 7 years ago

Hi,

I was using https://github.com/go-gl/gl bindings and I saw that EXT_texture_filter_anisotropic is not exposed when using the core profile. I report this there, but it seems that this is glow related. I guess that this is expected behavior.

However, I don't understand a few things. Firstly, why did you remove extensions from the core profile? After reading https://www.opengl.org/wiki/Core_And_Compatibility_in_Contexts I think that extensions are not marked as deprecated, some old "core" functionality is.

So, there are 3 options: use the compability profile with all extensions included, use the compability profile with some extensions excluded as the user excludes them, and use the core profile without any extensions (I don't think that extensions included in core OpenGL should count as an extension). I don't understand why a fourth option (core+extensions) can be used.

Moreover, I think that this fourth option is the better way to go. Removing deprecated functionality (like fixed pipeline) is normally the best option, and I guess that most people will agree with me in that. However, removing all extensions from the core profile is not good at all. There are extensions that provides really useful functionality and that are implemented everywhere, see https://www.opengl.org/wiki/Ubiquitous_Extension . Using OpenGL without anisotropic filtering in 2016 seems plainly wrong to me, this extension was made in the 1999, all AAA games provides it, it is very easy to use and it improves image quality. Moreover, there is no possible good workaround to this since using texelFetch will mean an important performance loss (and implementing anisotropic filtering in a shader won't be an easy task either).

Maybe you don't want to include all extensions on the core profile by default, but at least I think that an option to include extensions should exists.

Finally, I think that this should be documented in the Readme since "remext: A regular expression describing which extensions to exclude. Empty by default, excluding nothing. Takes precedence over explicitly added regular expressions." gives the impression that all extensions are included by default on both profiles.

pwaller commented 7 years ago

I'm afraid I can't help that much as I'm not a heavy user of extensions, so I don't know the subtlties of their use.

Firstly, why did you remove extensions from the core profile?

Are you referring to a specific action of a specific person? If so, please provide a reference since I can't easily tell.

I don't see anything wrong with exposing extensions. Are there any downsides to doing so? I suppose it runs the risk someone makes use of an extension without being aware that it is an extension - but this could be mitigated by putting extensions in a different namespace or giving them a prefix.

Would it be possible/reasonable to expose extensions in a separate package? Something like v4.5-core/glext?

davidmanzanares commented 7 years ago

Are you referring to a specific action of a specific person? If so, please provide a reference since I can't easily tell.

I don't know if this behavior has been there from the beginning.

I suppose it runs the risk someone makes use of an extension without being aware that it is an extension

That's exactly the problem. I think that the best solution is to let the final user pick the extensions he needs, disabling the rest by default. Therefore, he won't be able to use an extension without adding it explicitly.

For example if I need OpenGL 4.0 and EXT_texture_compression_s3tc, it would be good to just tell glow that: request an OpenGL4.0 Core with EXT_texture_compression_s3tc.

Thanks for your time!

errcw commented 7 years ago

For example if I need OpenGL 4.0 and EXT_texture_compression_s3tc, it would be good to just tell glow that: request an OpenGL4.0 Core with EXT_texture_compression_s3tc.

To distill the feature request here, we want to allow glow to respect the -addext parameter for extensions even when the extension would otherwise be "unsupported" per the specification XML. Right now SpecificationExtension.shouldInclude will always return false for EXT_texture_compression_s3tc in the core profile.

Perhaps the right way to approach this is to rethink the semantics of the -addext and -remext parameters. Currently we include all supported extensions by default by setting addext=.* and remext=^$. If we simply unset those parameters by default and allowed them to act as overrides, we'd get the desired behavior. We'd lose the ability to exclude supported extensions, but we could always introduce another ext parameter to control whether we include extensions by default.

@dv343 would this approach resolve your concerns? go-gl folks, your thoughts?

pwaller commented 7 years ago

Sounds fine to me. I did not understand though if you addressed whether they would appear by default in the go-gl repositories?

errcw commented 7 years ago

The go-gl repositories would remain as-is. Implementing this feature would allow users to generate bindings that include the extensions of their choice. I'd suggest that we should not be in the business of deciding which extensions to support.

davidmanzanares commented 7 years ago

I don't understand what do you mean about

"unsupported" per the specification XML exclude supported extensions

(I don't know much about the XML spec nor glow)

However, I guess that it will fix my problem.

I agree with errcw, I don't expect go-gl to cherry-pick their own selection of extensions.

errcw commented 7 years ago

OpenGL maintains a set of XML files that describe the OpenGL API (core and extensions). Glow uses these specifications to generate bindings. Extensions contain a supported tag that indicate the profile where the extension is supported. Glow currently respects this support bit even when the extension is listed in -addext. The proposal is to allow the parameter to take precedence over the spec.

davidmanzanares commented 7 years ago

I follow your information and read https://github.com/KhronosGroup/OpenGL-Registry/blob/master/xml/readme.pdf and https://cvs.khronos.org/svn/repos/ogl/trunk/doc/registry/public/api/gl.xml

So... Basically, what I was requesting is unsafe/unsupported by OpenGL itself, right?

I read the .xml file and found that the extensions I'm interested only support "gl" and not "glcore". Could someone explain to me the reason behind this?

I still think that my idea of using Core+extensions is the right one, but it seems that this is not a problem with glow.

errcw commented 7 years ago

Leaving aside the question of OpenGL support, I still believe the change to the semantics of -addext are correct because it gives glow users more control.

As for whether OpenGL "supports" certain combinations of profiles and extensions, it's largely up to the driver vendor for what is supported in practice. I'd be hesitant to generalize.

errcw commented 7 years ago

Glow now has improved support for user selection of extensions. Please let me know if this feature doesn't work for you.