Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expose the raw field of context #182

Open
coderedart opened this issue Jul 23, 2021 · 21 comments
Open

expose the raw field of context #182

coderedart opened this issue Jul 23, 2021 · 21 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@coderedart
Copy link
Contributor

i'm learning OpenGL and there's a lot of modern opengl functions/features that i would like to try like DSA or MultiDraw or Indirect rendering. but glow doesn't have most of these functions yet.

but if the core context was exposed either through an unsafe function or by just making the raw field of the Context Struct public, i can just use that to try out all the new latest stuff.

@grovesNL
Copy link
Owner

I'd prefer to add these functions to glow instead if possible.

One problem with exposing raw is that the current bindings/binding generation is meant to be an internal implementation detail. e.g. if we change how bindings are generated somehow (function names, types, fallbacks between versions/extensions, which parts of the GL API are included, etc.) then we might break any crates relying on raw

@grovesNL grovesNL added enhancement New feature or request help wanted Extra attention is needed labels Jul 30, 2021
@coderedart
Copy link
Contributor Author

Can we have a sort of contributing.md file ? would help some new people to know how to start contributing as this seems like one of the easiest rust projects to contribute to, especially when they are already familiar with opengl.

@grovesNL
Copy link
Owner

Yeah absolutely 👍 I'll try to create one sometime soon

@coderedart
Copy link
Contributor Author

oof, 3 months is a long time. but just wanted to see if there is some kind of progress related to separating platform specific functionality #76 (comment) . pretty much none of the modern opengl functions exist in webgl(2) and they will all be unimplemented!().

if there's a sort of winit (platformEXT trait) functionality, lets say DesktopEXT, that will allow people to contribute without having to worry about webgl or openglES. the core HasContext can have upto ES or webgl level of core functionality, and the DesktopEXT can have everything up to the latest 4.6 functions. people who use functions from this trait can check whether their minimum opengl core vesion actually has those functions.

Although, I don't know how something like gl functions that are in ARB extensions are dealt with. maybe another trait? where everything is allowed. a good example is bindless textures. to quote khronos wiki page
not all 4.3 hardware can handle bindless textures. The OpenGL ARB decided to make the functional optional by having it be an extension. It is implemented across much of the OpenGL 4.x hardware spectrum. Intel is mostly absent, but both AMD and NVIDIA have a lot of hardware that can handle it. .

I only added some dsa functions in #191 , and before i start playing with more stuff like multidraw or indirectdraws or bindless textures, I wanted to see if i could avoid polluting webgl context code which will be a lot of unimplemented!() and unnecessary autocompletions.

@TannerRogalsky
Copy link
Contributor

I would suggest that a trait like that might not belong in this crate which, as you've pointed out, is more about bridging the gap between OpenGL and WebGL than creating a Rusty interface for all of OpenGL. Personally, I like that, generally, the API surface defined in this crate approximates the features that are usable on both platforms. I think I'd rather see a, for example, glow-gl4-ext crate that implements the additional functionality (although at that point I'm not sure why you wouldn't just use the generated GL bindings directly).

@coderedart
Copy link
Contributor Author

ah. my bad. my use case of glow is to just have a clean wrapper instead of raw gl bindings with raw pointers and casts flying around everywhere. i asked in the rust gamedev discord and was told that glow allows stuff that is technically not a strict subset of the es/web/desktop gl versions. like c0d566e . either the trait way or a separate crate glow-gl4-ext way is fine for me.

@TannerRogalsky
Copy link
Contributor

TannerRogalsky commented Nov 5, 2021

Yeah, you're 100% right that there are some places where the practical desire for this to be generally useful has overridden the aspirational desire to represent the API available on both platforms. My personal feelings are that a push to implement all of gl46 in this pseudo-Rusty style that glow does might be too big a departure from the generally stated goal of this crate.

But, hell, I'm not the maintainer. :P

@PolyMeilex
Copy link

ah. my bad. my use case of glow is to just have a clean wrapper instead of raw gl bindings with raw pointers and casts flying around everywhere. i asked in the rust gamedev discord and was told that glow allows stuff that is technically not a strict subset of the es/web/desktop gl versions. like c0d566e . either the trait way or a separate crate glow-gl4-ext way is fine for me.

It's same for me, I don't use the webgl side of glow at all 😅, and I'm well aware that it's the whole point of this crate

@grovesNL
Copy link
Owner

grovesNL commented Nov 7, 2021

@coderedart for now I think it's fine to add them to the regular HasContext trait, and panic in the web backend implementations for those functions.

We could always consider restructuring it later if this eventually causes problems (e.g. bloating the web backend too much, too confusing when referencing documentation, etc.). In that case we could probably move them into a separate GL-only trait (possibly also behind a feature that's off by default or separate crate).

I'd also like to add doc comments to each function to clarify which versions/backends/extensions support it, which would be a simple way to improve ergonomics in general.

@coderedart
Copy link
Contributor Author

Has anything changed regarding this issue in the last two years? like exposing the raw fn pointers behind a feature like unstable-raw-expose or something similar.

If it feels like we should not expose it, then its probably time to close this issue.

@polyzium
Copy link

Also asking for this half a year later. I am migrating my codebase from gl to glow because the former seems to be abandoned, as well as I hit a roadblock adding imgui to my SDL project, because imgui SDL support uses glow and I use gl here.

@grovesNL
Copy link
Owner

Hi @polyzium 👋 Could you provide an example of the problem you're running into, and how the raw field would help? Is there a crate you're using for imgui SDL support?

@polyzium
Copy link

@grovesNL gl is a crate containing the raw OpenGL API without any abstractions, it's just a 1-to-1 binding to the C API. And during that time I was following learnopengl.com and translating calls to Rust on the fly. However because glow instead provides abstractions to these APIs, I was confused at first to see no equivalent, that is until I looked at the source code later where e.g. glGenBuffers was hidden under Context::create_buffer. It also makes it very hard if not impossible to refer to resources like the OpenGL spec, docs.gl, or the OpenGL wiki by the Khronos Group themselves. And as many people pointed out in the issues, some functions from the original OpenGL C API are missing/have no Rusty abstractions implemented (yet), and there's also a whole different can of worms in regards to extensions.

As for imgui, I am using imgui-sdl2-support which in turn uses imgui-glow-renderer. I was not able to find a renderer that uses the gl crate, and making my own renderer would take a significant chunk of my time and severely hinder my productivity (same thing goes for egui). In the end I stuck with gl, scrapped the idea and wanted to make my own UI instead.

I know what people might ask me, why didn't I use wgpu-rs as it is a more modern way or something like that. The reality of it is that I don't want to use any graphics API abstractions that might introduce their own behaviours, and I want to have full control over the software I write, like the rest of the gaming industry making their own game engines. In fact this very problem is why I started doing software rendering literally around one and a half weeks ago. Yes it's harder (manual perspective correction and zbuffering), yes it's slower (singlethreaded on the CPU), but it taught me a lot about what 3D graphics consist of, free from the wrath of OpenGL/Vulkan/etc.

Excuse me for the rant, but as you can see I am quite lost.

@grovesNL
Copy link
Owner

@polyzium I was wondering how exposing the native context itself would help. Is it because the names are slightly different and you want to use the C names?

glow doesn't try to abstract very much, it's just trying to make it a slightly more convenient where it makes sense and find some common naming between the different GL specifications.

glow doesn't use the gl crate internally, so if you're using gl then there's not much glow can help with.

@coderedart
Copy link
Contributor Author

Could you provide an example of the problem you're running into, and how the raw field would help?

One very obvious issue that I faced in the past is following a gl tutorial and writing a basic game/app.

Now, if I wanted to use a library which uses glow, like the above mentioned imgui-glow-renderer, then I have to port my entire app to using the glow api or maintain two different contexts (a raw gl context for my app and a glow context for the libraries which use glow). My functions will need to pass around these two different contexts to the right places, which.. just feels like unneeded complexity.

If glow exposed the raw context, then I could just do

pub struct MyGlContext(glow::Context);
impl AsRef<glow::Context> for MyGlContext {
	fn as_ref(&self) -> &glow::Context {
		&self.0
	}
}
impl AsRef<RawDesktopGlContext> for MyGlContext {
	fn as_ref(&self) -> &RawDesktopGlContext {
		&self.0.raw // something like this
	}
}

Now, glow based libraries which take &glow::Context and my API which takes &RawDesktopGlContext would both use it. renaming parameter/type (if necessary) across the app is just one find_and_replace away. This allows me to just follow most opengl guides (3 years ago, I was learning about DSA stuff when I raised this issue) without any translations (or worrying about missing functions). opengl has confusing names and glow adds more to this confusion by mixing webgl and desktop gl.

eg: glGenBuffers is the usual thing people learn first in opengl. glow doesn't have that. glow provides create_buffer (singular) instead. If i'm learning modern opengl, I will learn about createBuffers (plural). It very much looks like glow's create_buffer, but its not. Instead it is implemented with glGenBuffers behind the scenes.

At this point, I am comfortable enough with gl/glow that I can more or less figure out the glow's API, but back then, it was one more challenge when learning openGl.

@polyzium
Copy link

Is it because the names are slightly different and you want to use the C names?

Yeah mostly it is exactly that. I am pretty new to OpenGL but I have a basic understanding of graphics, having worked with programs like Blender before, so it would be helpful if I could use C names, or at least snake_case versions of those.

also as @coderedart pointed out this is the exact issue I have faced. Either I port my app to glow or maintain a bridge between gl and glow.

@TannerRogalsky
Copy link
Contributor

I'm wondering why you're both wanting to use the imgui-glow-renderer if neither of you want or need to use glow? I would expect you to use the imgui renderer backend associated with the backend you're already using. Maybe this one? https://github.com/michaelfairley/rust-imgui-opengl-renderer

@polyzium
Copy link

I initially thought of using this one, but it seems like it works only with older imgui-rs and I didn't want to downgrade it just to get it to work.

@TannerRogalsky
Copy link
Contributor

I just cloned it and bumped the allowed version of imgui. It seems to work just fine. I'm not trying to dissuade you, necessarily, from any pedagogical or functional gripes you may have with glow but I feel like your individual easiest path toward what you seem to want is likely to clone that repo, use your fork as a dependency, bump the version of imgui that it allows, and not worry about glow.

@grovesNL
Copy link
Owner

Yeah mostly it is exactly that. I am pretty new to OpenGL but I have a basic understanding of graphics, having worked with programs like Blender before, so it would be helpful if I could use C names, or at least snake_case versions of those.

That makes sense, although there are only a few functions like GenBuffers where the snake case versions don't match.

I'd really recommend trying out a switch to glow and glancing at the source if you can't find something that matches the GL function that you're looking for. I'd like to add more docs to show how each glow function ties to the corresponding functions in the GL specifications but most names are already 1:1 anyway.

@coderedart
Copy link
Contributor Author

imgui is just an example. The ecosystem has settled on glow, and going against that will lead to a lot of friction.
Think of eframe (egui). Its official backend is glow. https://github.com/emilk/egui/tree/master/examples/custom_3d_glow shows how to draw something using glow inside a widget. If you wanted to draw using desktop gl functions, there's not really much of choice.

I think focusing on any particular example would be going too offtopic, as there will always be workarounds. This issue was made when I was just learning openGL (right after learning rust) and back then, it would be really hard to think about forking and patching rust code . Even now, This feature is still useful to me, as I am more likely to use a cool modern gl function if its readily available. But its not absolutely necessary. We can simply workaround by having a fork of glow that exposes the gl fns publicly and Override Crates.io Dependencies to point glow to my fork.

This issue is about whether glow would like to expose its raw fns behind an unstable-raw-gl feature. Its hacky/dirty, but will allow easy access to everything behind an unstable feature flag. If not, this issue should probably be closed.

For more general discussion of how to expose platform specific gl functionality, see #76 (comment) .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants