Update glutin to 0.30 #2036

Melchizedek6809 · 2023-01-02T16:04:21Z

Hey there,

I've been working on updating glium to work with glutin 0.30 and things are looking quite well, I've added a new example triangle-new because glutin needs to be initialized quite differently. I also restructured the overall example so that everything gets setup on receiving a Resume Event and dropped when a Suspend Event arrives, this apparently should only happen on Android, but I thought that examples should probably promote the most reliable/safe way of handling things.

Apart from that I tried to keep changes to the API as little as possible, which is why I've added the ContextSurfacePair type. I've also experimented on another branch with just keeping the Context around, but that meant breaking the API since we can't make a Context current without a Surface, and we can't determine the size of the Framebuffer without a Surface.

Two other things where someone with more Rust experience may know of a better Solution:

Is there a better/more efficient way of turning a &str into a &CStr? I'm currently doing the following:

#[inline]
unsafe fn get_proc_address(&self, symbol: &str) -> *const c_void {
    let symbol = CString::new(symbol).unwrap();
    self.borrow().display().get_proc_address(&symbol) as *const _
}

I'm not sure about the 'static lifetime for the Display struct in src/backend/glutin/mod.rs:82, I have a gut feeling that this is wrong, but I'm too new to Rust to know for certain.

Oh and it currently only works on *nix Platforms, due to glutin not implementing Surface::{width,height} for CGL/WGL contexts which this implementation depends on, I'm looking into implementing that for glutin but wanted to create a draft request already since this might take some time.

est31 · 2023-01-02T22:49:03Z

Is there a better/more efficient way of turning a &str into a &CStr?

Sadly no. You have to allocate by creating a CString as the &str has no zero byte at the end, while CStr guarantees trailing zeroes.

I'm not sure about the 'static lifetime for the Display struct

Not 100% sure, for that I have to see the error that happens if you remove it...

Oh and it currently only works on *nix Platforms, due to glutin not implementing Surface::{width,height} for CGL/WGL contexts which this implementation depends on

Yeah if possible, getting the width/height on the surface should be cross platform.

Melchizedek6809 · 2023-01-02T22:55:42Z

Sadly no. You have to allocate by creating a CString as the &str has no zero byte at the end, while CStr guarantees trailing zeroes.

Ah alright, too bad

Not 100% sure, for that I have to see the error that happens if you remove it...

I get the following error without the static lifetime:

error[E0310]: the parameter type `T` may not live long enough
   --> src/backend/glutin/mod.rs:172:32
    |
172 |         let context = unsafe { context::Context::new(glutin_backend, checked, debug) }?;
    |                                ^^^^^^^^^^^^^^^^^^^^^ ...so that the type `T` will meet its required lifetime bounds
    |
help: consider adding an explicit lifetime bound...
    |
111 | impl<T: SurfaceTypeTrait + ResizeableSurface + 'static> Display<T> {
    |                                              +++++++++

Yeah if possible, getting the width/height on the surface should be cross platform.

I've already opened a PR which implements the missing functionality on Windows/Mac: rust-windowing/glutin#1551 So hopefully this won't be an issue for long.

est31 · 2023-01-03T00:30:19Z

I get the following error without the static lifetime:

Yeah that's because Display::new requires 'static for its backend argument's type. That's present because in that call we put the backend into a boxed trait object.

I've already opened a PR which implements the missing functionality on Windows/Mac

Very cool :).

kchibisov · 2023-02-07T16:37:32Z

I've released glutin 0.30.4 which has width/height implemented for every graphics backend, it still returns an Option, but it means that the call failed and not that you can do much(though, I'm not sure they could fail...). I'd assume you now don't really on winit anymore, given that you wanted stuff like that implicitly.

Be aware that on macOS context/width/height/resize calls will block off the main thread if the main thread is blocked, though I doubt many folks use thread per context anyway, given that before macOS was SIGILL-ing.

Melchizedek6809 · 2023-03-09T18:59:39Z

@est31
Alright, now this branch depends on the version from crates.io and the new example seems to work on my devices at least.
Is there anything you'd like me to change?

est31 · 2023-03-09T19:12:10Z

@Melchizedek6809 I need to give this a closer look on the weekend. Do you have ideas what to do about the examples? I don't know yet which direction they should go so for now I'm only looking for ideas, not neccessarily having the migration implemented.

Melchizedek6809 · 2023-03-09T21:11:52Z

@est31
Maybe we could put the whole context creation/suspension/resumption handling into the example support module, with each example then only implementing a Trait with methods for initialization/drawing?

Since from what I can gather on first glance, most examples don't seem to differ in the way they create the Context or deal with the event loop, which might also lead to the specific examples not directly depending on glutin/winit anymore.

It might however make some of the smaller examples more verbose due to the state struct.

est31 · 2023-03-12T20:59:10Z

Sorry for coming back only now, my weekend was very busy. In general examples should be self-contained and small so that you know how to use the API from looking only at few lines of code. On the other hand, being able to focus only on the parts that matter is also very useful. I think for the very introductory examples it would be best to not use util modules but one could use it for more involved examples. It's a bit of an effort to port all of them.

Melchizedek6809 · 2023-03-12T21:39:35Z

@est31
No problem, thank you for taking the time anyways.

Yeah, I though it might make some examples clearer if the whole context creation boilerplate gets moved somewhere else, since it barely changes, though as you metion it's probably bad for the tutorials and associated examples.

What do you think about using the same/similar structure but including it directly in the examples / tutorials so that it's not hidden away?

Yeah it's probably quite a bit of work to update all the examples and especially the tutorials, if you're satisfied with the overall structure/architecture of the update I could look into updating some of them, then we might get a better picture at how much work it actually ends up being.

est31 · 2023-03-13T12:17:43Z

It might be the best to work on the glutin upgrade on a separate branch, as it might be quite involved to port all the examples and the book. I have created a new glutin-0.30 branch. Feel free to close this PR and file one against that one. Then we can merge it to master when the time is ready. I think if the utils module has less than 300-ish lines and is one file only then it's okay to include it as a separate file, but at least one or two examples should remain self contained.

Melchizedek6809 · 2023-03-13T15:26:32Z

Alright, I changed the base branch of this PR.
I'll look into adding an abstraction to the support crate and port an example to see if it fits in 300-ish lines.

What do you think about leaving the tutorial examples self contained?
Since I suppose most readers/users will end up building their own abstractions anyways.

…uniforms-program-change Fixed subroutine uniforms not being updated on program change

est31 · 2023-03-13T18:25:42Z

What do you think about leaving the tutorial examples self contained?

I'm okay with that but if it becomes too much copy-paste then it would be better to make them use a support module. This is good both from a reading point of view, you know you can skip those parts because they are the same in all examples and there are no slight example specific modifications, and also good from a maintenance point of view. The readers have top priority here.

Melchizedek6809 · 2023-03-14T20:44:23Z

I'm okay with that but if it becomes too much copy-paste then it would be better to make them use a support module.

Alright I'll keep that in mind

The readers have top priority here.

I respect that very much, I'll try my best.

Melchizedek6809 · 2023-03-26T01:44:09Z

@est31 Finally got around to looking at the examples, added a generic container that handles the event_loop and a trait to be implemented by the examples, the support module is currently at 173 lines, I hope that is alright.

Regarding the self-contained examples: there are a couple of examples that don't fit the usual schema (for example: gpgpu, info, screenshot, manual-creation, multiple_windows), do you think it's sufficient to have these be the self-contained ones?

examples/support/mod.rs

est31 · 2023-03-28T02:12:58Z

Regarding the self-contained examples: there are a couple of examples that don't fit the usual schema (for example: gpgpu, info, screenshot, manual-creation, multiple_windows), do you think it's sufficient to have these be the self-contained ones?

Ideally, the triangle example would also be self-contained so that users can familiarize themselves with the entire structure. But maybe one should only do this for the book, so I guess it's fine to keep it here in the normal examples.

Melchizedek6809 · 2023-04-01T00:14:06Z

@est31 Alright, I've finally managed to port all the examples, most use the support crate except for tutorial1/2 as well as the SPIR-V example (it requires a higher OpenGL version than the rest). Those 3 also take some shortcuts to make things slightly less verbose.

I still have to check the book, I'm thinking about inserting a chapter in between 2 and 3 just dealing with context creation, since this probably requires an entire chapter to explain properly.

Apart from that I've fixed the tests so that they compile, they still fail as they do on the master branch, I suppose fixing them should probably go in another PR.

Also, I think we can remove the headless feature, since users can just initialize glutin without a window on Mac/Linux, so it doesn't seem to be required anymore, although I might be missing something, what do you think?

I'll also do some more testing on various machines to make sure that things work reliably since I've only really tested on Linux so far where things seem quite stable.

This would lead to other errors down the line, also fixed the fxaa example, since when minimizing/resizing on Windows the framebuffer could become 0 on one dimension.

…_zero_size Validate that a framebuffer isn't 0 on any dimension

Moved glutin-winit,winit and raw-window-handle to dev-dependencies since they are only necessary for the examples/tests. Also set default-features = false for glutin so that users can specify the feature set that they want.

Mainly a new resize method has been added to the Backend trait, this way we can direcly resize the Display, instead of having to borrow the underlying context_surface_pair first.

The headless backend isn't necessary anymore since glutin can now be initialized without a window on some platforms. Since the test_headless feature only switched the tests to use a headless backend I've removed it as well. Should also make things cleaner since it's not something users are likely to change.

Mainly making sure that it works with glutin 0.30, but I've also fixed a couple of typos, rewrote sentences so they are easier to understand and changed chapter 5 to have the color be specified in as an additional vertex attribute, and changed chapter 6 to draw a textured square instead of a textured triangle, since the OpenGL logo looks nicer that way.

est31 · 2023-05-05T00:55:03Z

would be nice if you could do a final review of the changes.

Okay thanks I somehow missed that this was ready for review. I am a bit busy with work and other projects atm but I will give it a review in the coming days, hopefully.

est31

Sorry for the late review, it looks really good already! Thanks for putting in all the work.

src/backend/glutin/mod.rs

book/tuto-01-getting-started.md

book/tuto-02-triangle.md

src/backend/glutin/mod.rs

est31 · 2023-05-20T09:36:30Z

src/lib.rs

    // 2. Parameters for building the Window.
-    let wb = glium::glutin::window::WindowBuilder::new()
-        .with_inner_size(glium::glutin::dpi::LogicalSize::new(1024.0, 768.0))


Is this example still accurate?

Not really, I've rewritten it using simple_winit_window, I hope that's alright, since otherwise it'd get pretty long.

Can you add back the function calls here on the SimpleWindowBuilder?

src/vertex_array_object.rs

src/backend/glutin/mod.rs

dcvz · 2023-07-03T11:27:55Z

@Melchizedek6809 sorry for the ping. Are you still interested in taking this past the finish line? Looks like there's a few small comments from the last review. If you need some assistance I can help out fixing these up.

Since it's not necessary anymore I've removed the old simple_winit_window function, since it's not really simpler than using the builder.

Melchizedek6809 · 2023-07-03T22:52:53Z

@dcvz No problem and thanks for the offer and reminding me, I might take you up on that if there's much to do in the next round.

est31

Thanks a lot for this monumental effort. This has been one of the biggest changes in glium in the last 5 years I think.

est31 · 2023-07-10T02:10:42Z

src/lib.rs

    // 2. Parameters for building the Window.
-    let wb = glium::glutin::window::WindowBuilder::new()
-        .with_inner_size(glium::glutin::dpi::LogicalSize::new(1024.0, 768.0))


Can you add back the function calls here on the SimpleWindowBuilder?

est31 mentioned this pull request Jan 2, 2023

Update glutin to 0.30 #2025

Closed

est31 mentioned this pull request Mar 10, 2023

Option to disable default features for glutin #2045

Closed

Fixed subroutine uniforms not being updated on program change

843a0e5

Melchizedek6809 changed the base branch from master to glutin-0.30 March 13, 2023 15:17

Merge pull request glium#2047 from Melchizedek6809/bugfix-subroutine-…

5026054

…uniforms-program-change Fixed subroutine uniforms not being updated on program change

parasyte mentioned this pull request Mar 14, 2023

Update winit to 0.28 imgui-rs/imgui-rs#716

Closed

est31 reviewed Mar 28, 2023

View reviewed changes

examples/support/mod.rs Outdated Show resolved Hide resolved

Melchizedek6809 force-pushed the update_glutin branch from df05af9 to 23274f7 Compare March 28, 2023 01:52

Melchizedek6809 and others added 3 commits April 1, 2023 23:15

Validate that a framebuffer isn't 0 on any dimension

ca8f54b

This would lead to other errors down the line, also fixed the fxaa example, since when minimizing/resizing on Windows the framebuffer could become 0 on one dimension.

Fixed picking example crash when minimizing on Windows

472401f

Merge pull request glium#2051 from Melchizedek6809/bugfix_framebuffer…

3fb10d2

…_zero_size Validate that a framebuffer isn't 0 on any dimension

Melchizedek6809 force-pushed the update_glutin branch 2 times, most recently from 6f98028 to 9590fe6 Compare April 2, 2023 15:55

Melchizedek6809 linked an issue Apr 8, 2023 that may be closed by this pull request

There's a mistake in the event loop code in the mdbook #1979

Open

Melchizedek6809 and others added 18 commits May 5, 2023 00:19

Ported fxaa example

a58d25b

Ported manual-creation example

8e4d559

Ported screenshot-asynchronous example

4c1e691

Ported the tutorial examples

66a3545

Ported spirv example

836f624

Fix examples crashing when minimizing on Windows

996df3c

Updated dependencies

e6ad363

Cleaned up dependencies

d9ba607

Moved glutin-winit,winit and raw-window-handle to dev-dependencies since they are only necessary for the examples/tests. Also set default-features = false for glutin so that users can specify the feature set that they want.

Cleaned up the glutin backend and Backend trait

2c42cf2

Mainly a new resize method has been added to the Backend trait, this way we can direcly resize the Display, instead of having to borrow the underlying context_surface_pair first.

Updated first 2 chapters and minor example cleanup

c0753ae

WIP

5ca9edc

Added simple_winit_window and updated tutorial examples

8b7e70c

Updated first 4 chapters

ed1f468

Fix cargo doc --all --all-features

bc787de

Fix failing tests on Windows

4add826

Revert change to test

4832c56

Melchizedek6809 force-pushed the update_glutin branch from 0fa6243 to 4832c56 Compare May 4, 2023 22:19

est31 requested changes May 20, 2023

View reviewed changes

est31 reviewed May 20, 2023

View reviewed changes

src/backend/glutin/mod.rs Outdated Show resolved Hide resolved

Melchizedek6809 added 3 commits July 3, 2023 23:45

Fixed some typos, and example code

f413a2b

Added SimpleWindowBuilder and changed examples

9a88b5f

Since it's not necessary anymore I've removed the old simple_winit_window function, since it's not really simpler than using the builder.

Improved SimpleWindowBuilder

d19d772

est31 approved these changes Jul 10, 2023

View reviewed changes

est31 merged commit 3335248 into glium:glutin-0.30 Jul 10, 2023
7 checks passed

est31 mentioned this pull request Dec 16, 2023

Headless rendering example for the new version #2091

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update glutin to 0.30 #2036

Update glutin to 0.30 #2036

Melchizedek6809 commented Jan 2, 2023 •

edited

est31 commented Jan 2, 2023

Melchizedek6809 commented Jan 2, 2023

est31 commented Jan 3, 2023

kchibisov commented Feb 7, 2023

Melchizedek6809 commented Mar 9, 2023

est31 commented Mar 9, 2023

Melchizedek6809 commented Mar 9, 2023

est31 commented Mar 12, 2023

Melchizedek6809 commented Mar 12, 2023

est31 commented Mar 13, 2023

Melchizedek6809 commented Mar 13, 2023

est31 commented Mar 13, 2023

Melchizedek6809 commented Mar 14, 2023

Melchizedek6809 commented Mar 26, 2023

est31 commented Mar 28, 2023

Melchizedek6809 commented Apr 1, 2023

est31 commented May 5, 2023

est31 left a comment

est31 May 20, 2023

Melchizedek6809 Jul 3, 2023

est31 Jul 10, 2023

dcvz commented Jul 3, 2023

Melchizedek6809 commented Jul 3, 2023

est31 left a comment

est31 Jul 10, 2023

Update glutin to 0.30 #2036

Update glutin to 0.30 #2036

Conversation

Melchizedek6809 commented Jan 2, 2023 • edited

est31 commented Jan 2, 2023

Melchizedek6809 commented Jan 2, 2023

est31 commented Jan 3, 2023

kchibisov commented Feb 7, 2023

Melchizedek6809 commented Mar 9, 2023

est31 commented Mar 9, 2023

Melchizedek6809 commented Mar 9, 2023

est31 commented Mar 12, 2023

Melchizedek6809 commented Mar 12, 2023

est31 commented Mar 13, 2023

Melchizedek6809 commented Mar 13, 2023

est31 commented Mar 13, 2023

Melchizedek6809 commented Mar 14, 2023

Melchizedek6809 commented Mar 26, 2023

est31 commented Mar 28, 2023

Melchizedek6809 commented Apr 1, 2023

est31 commented May 5, 2023

est31 left a comment

Choose a reason for hiding this comment

est31 May 20, 2023

Choose a reason for hiding this comment

Melchizedek6809 Jul 3, 2023

Choose a reason for hiding this comment

est31 Jul 10, 2023

Choose a reason for hiding this comment

dcvz commented Jul 3, 2023

Melchizedek6809 commented Jul 3, 2023

est31 left a comment

Choose a reason for hiding this comment

est31 Jul 10, 2023

Choose a reason for hiding this comment

Melchizedek6809 commented Jan 2, 2023 •

edited