Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

[WIP] Initial update to egui 0.19 based on glium, glow and miniquad implementations #1

Merged
merged 4 commits into from
Oct 21, 2022

Conversation

EriKWDev
Copy link
Contributor

This is an initial update of egui to latest.
Quite a bit has changed but thanks to https://github.com/not-fl3/egui-miniquad/blob/master/src/painter.rs, https://github.com/emilk/egui/blob/master/crates/egui_glow/src/painter.rs, and https://github.com/emilk/egui/blob/master/crates/egui_glium/src/painter.rs I was able to come quite a bit.

Sadly, the egui texture seems to be black. Input seems to be working and the shadow behind the window display correctly.

Could you perhaps have a look at what could be wrong?

image

@cohaereo
Copy link
Owner

Quickly looking at the issue on my way home, it seems that the font texture does get uploaded, but is deleted right after the first drawn frame, though DeleteTextures is never called... I'll be taking a more extensive look when I get home

@EriKWDev
Copy link
Contributor Author

EriKWDev commented Aug 24, 2022

Some things to note that might be wrong:

  • I only used gl::RGBA for textures, but there is a lot of code relating to sRGBA so maybe other texture formats should be used
  • The egui::TextureId::User(id) assigned to a UserTexture should probably be the same as it's OpenGL textureid, currently just setting it to self.textures.len() which is most likely wrong. This could be fixed if we stop lazily creating the textures and instead do it immediately like glow-implementation does it: https://github.com/emilk/egui/blob/d5933daee5328d3ca37741c0642d09a1e71232fb/crates/egui_glow/src/painter.rs#L523
  • I used the vertex and fragment shaders from miniquad since they supported the oldest version of glsl (100)

@EriKWDev
Copy link
Contributor Author

Did you you find anything? :)

@cohaereo
Copy link
Owner

cohaereo commented Sep 2, 2022

Sorry for the delay, I've been a bit busy.
I managed to find the issue.

At this line, you're binding egui textures as opengl textures, which use their own internal ID (starting at zero, so binding the first egui texture as opengl texture effectively unbinds the texture)

match mesh.texture_id {
    egui::TextureId::Managed(id) | egui::TextureId::User(id) => unsafe {
        gl::BindTexture(gl::TEXTURE_2D, id as _)
    },
}

Mapping the IDs through a hashmap should be enough to resolve the issue.
(i just quickly binded the egui texture here, that's why it looks weird)
image

@EriKWDev
Copy link
Contributor Author

EriKWDev commented Sep 4, 2022

That was it! I had already created a hashmap with the correct ids.. Just didn't use it there correctly.

Changed that line to:

let it = self
    .textures
    .get(&mesh.texture_id)
    .expect("Textures should have been added to hash map by now");

unsafe {
    gl::BindTexture(
        gl::TEXTURE_2D,
        it.texture
            .expect("Texture should have a valid OpenGL id now"),
    );
}

The assumptions in the .expect() might be incorrect though.

image

The colors look off though

@EriKWDev
Copy link
Contributor Author

EriKWDev commented Sep 4, 2022

There we go

image

@EriKWDev
Copy link
Contributor Author

EriKWDev commented Sep 4, 2022

Getting segfaults in some of my more complex UI apps.. Will try to isolate

@cohaereo
Copy link
Owner

Is this ready now?

@EriKWDev
Copy link
Contributor Author

EriKWDev commented Sep 22, 2022

I haven't tested very extensibly, but with the new safety checks I haven't gotten a segfault yet in my applications.

I feel like the API from the User's point of view could probably be made simpler to use, but that should probably be a separate PR

@rdeepak2002
Copy link

Thanks for making this! It seems to work well on my end.

@cohaereo cohaereo merged commit 61d4564 into cohaereo:master Oct 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants