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

glow design rationale #76

Open
notdanilo opened this issue Jan 20, 2020 · 6 comments
Open

glow design rationale #76

notdanilo opened this issue Jan 20, 2020 · 6 comments
Labels
question Further information is requested

Comments

@notdanilo
Copy link
Contributor

notdanilo commented Jan 20, 2020

glow is supposed to provide GL on Whatever. But what is the strategy to achieve it? Being clear about the design rationale would increase trust in the project.

As suggested in the comment bellow, glow only exposes a subset of the functions which are present in OpenGL, OpenGLES3 and WebGL2.

[...] generally if there's a function which exists in all of {OpenGL, OpenGL ES 3, WebGL2} and it's not exposed in glow then it's just unimplemented at the moment.

#75 (comment)

It makes sense for the project purpose, but what if we want to use platform-specific functionalities?

For instance: It's totally possible to keep track of the platform specific context, but it doesn't seem to be possible to use an OpenGL object created with glow in the original context because the object ID isn't exposed.

@grovesNL
Copy link
Owner

But what is the strategy to achieve it?

Basically the goal is to make it easy as possible to use the portable subset between OpenGL, OpenGL ES, and WebGL. We want to avoid users having to write #[cfg(target_arch = "wasm32")] in order to switch between these APIs.

Some of the original rationale was described in gfx-rs/gfx#2554 but the glow README definitely needs some more detail about this :)

It makes sense for the project purpose, but what if we want to use platform-specific functionalities?

I think it's reasonable to expose parts of the API outside that subset as platform-specific extensions as long as the user is aware that it's not portable. For example, we already started exposing some WebGL-specific functionality that doesn't exist on OpenGL or OpenGL ES.

winit also has a nice approach to handle platform-specific functionality -- we might be able to apply a similar approach to glow.

@grovesNL grovesNL added the question Further information is requested label Jan 20, 2020
@TannerRogalsky
Copy link
Contributor

As it pertains to the design of this lib, I'm curious about ed1ca43. Was the driving force behind removing the more complex type system just to keep this crate as simple as possible? Creating a more 'Rusty' type representation fell outside of it's scope?

@grovesNL
Copy link
Owner

grovesNL commented Feb 5, 2020

@TannerRogalsky Great question :) It's difficult to say whether removing typed enums/newtypes/bitflags is the best move, but the biggest issues I noticed were:

  • We basically had to invent names for these new groups of values, and it wasn't clear which values should be grouped together (we'd have to create enums specific to functions if we want to provide meaningful restrictions). For example, which would we prefer:
  • For enums/bitflags, it required more user effort to port existing GL code. For example, following along with an OpenGL tutorial or switching a crate from gl_generator to glow required extra work (especially if the enum name isn't obvious which could be tricky due to the first point). It's also common to have GL code like GL_COLOR_ATTACHMENT0 + i to select an attachment which is more difficult when these are enum variants.

There are probably some places where enums/bitflags are obvious improvements, but at least for now I thought it would be easier if we just kept using constants.

@rspencer01
Copy link
Contributor

I'd say the biggest loss was actually the newtypes. I recently ran into a (stupid) error which could have been caught by the compiler telling me I was trying to use a BufferID where a BufferTarget was needed. I think these newtypes don't remove much porting ease, since one hardly ever (never?) manipulates some of these identifiers.

@grovesNL
Copy link
Owner

grovesNL commented Jul 2, 2021

@rspencer01 interesting, maybe we could improve that by using newtypes inside of the associated types at

glow/src/native.rs

Lines 84 to 95 in c5a7ec7

type Shader = native_gl::GLuint;
type Program = native_gl::GLuint;
type Buffer = native_gl::GLuint;
type VertexArray = native_gl::GLuint;
type Texture = native_gl::GLuint;
type Sampler = native_gl::GLuint;
type Fence = native_gl::GLsync;
type Framebuffer = native_gl::GLuint;
type Renderbuffer = native_gl::GLuint;
type Query = native_gl::GLuint;
type UniformLocation = native_gl::GLuint;
type TransformFeedback = native_gl::GLuint;

e.g. type Shader = NativeShader; with NativeShader defined as struct NativeShader(native_gl::GLuint)

The web backend already does something similar.

@rspencer01
Copy link
Contributor

It introduces a bit of boilerplate and destructuring code throughout, but I would personally think it is worth it (and, as you say, no worse than the web code).

bors bot added a commit that referenced this issue Jul 3, 2021
180: Use newtypes for native GL handles r=kvark,rspencer01 a=grovesNL

Fixes #178 and should help #76 (comment)

Co-authored-by: Joshua Groves <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants