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

Add worldspace egui #214

Closed
wants to merge 1 commit into from
Closed

Conversation

TheButlah
Copy link
Contributor

@TheButlah TheButlah commented Oct 2, 2023

Rebased on #210 and #211.

Adds worldspace UI to bevy_egui via the EguiRenderToTexture component. Handling input is not implemented in this PR.

This PR is marked as a draft, because I thought that I could have multiple StandardMaterials on the same mesh, but that is not the case right now in bevy. This means that a user cannot easily have a base texture and then apply the egui overlaid transparently on top.

I think the right solution is likely to include an EguiMaterial as part of this plugin for users' convenience, and keep the EguiRenderToTexture as an option for people to use if they want to create their own custom materials. I don't have any experience with bevy materials yet, so I will revisit that later.

I'm also not particularly happy with my refactor - I feel like the systems could be unified more - there is still some branching logic to handle windows vs textures.

image

@TheButlah
Copy link
Contributor Author

TheButlah commented Dec 20, 2023

Hi, @mvlabat what do you think I should take as the next steps for getting this mergeable?

pub struct ExtractedEguiSettings(pub EguiSettings);
/// The extracted version of [`EguiManagedTextures`].
#[derive(Debug, Resource)]
pub struct ExtractedEguiManagedTextures(pub HashMap<(Entity, u64), Handle<Image>>);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's turn this (Entity, u64) tuple into a struct, named fields will help to understand what Entity and u64 are related to.

@vladbat00
Copy link
Owner

vladbat00 commented Jan 7, 2024

@TheButlah apologies for the delayed response. I'm yet to take a closer look to better answer the question. And I'll need to re-read our Discord DM to remember all the context :)
Anyway, as for the first step, let's rebase this against the latest bevy_egui version

@TheButlah TheButlah force-pushed the add-worldspace-egui branch 2 times, most recently from 979ed1a to 278bae4 Compare January 9, 2024 06:40
@TheButlah
Copy link
Contributor Author

TheButlah commented Jan 9, 2024

Anyway, as for the first step, let's rebase this against the latest bevy_egui version

Just completed this :) However, the cube now looks like this:
Screenshot 2024-01-09 at 1 40 53 AM

It turns out, this is identical to what I get when I cargo run --example worldspace in my original commit. IDK then if the above screenshot came from some uncomitted tweak I did or what, but it never actually did the transparency correctly, I guess.

When I was doing my experiments with wgpu directly, this would happen when the BlendState was not quire configured correctly, causing it to repeatedly add until the values were opaque instead of partially transparent. I'm not sure exactly if that is what is happening here.

I'll need to revise the PR to fix it, but feel free for someone to suggest a fix if they know what the issue is!

@TheButlah TheButlah force-pushed the add-worldspace-egui branch from 278bae4 to 7124092 Compare January 9, 2024 06:53
@TheButlah TheButlah force-pushed the add-worldspace-egui branch from 7124092 to 9b2fcb0 Compare January 9, 2024 06:58
@v-kat
Copy link
Contributor

v-kat commented May 17, 2024

Are you going to convert this to not a draft? It looks very useful....

@vladbat00
Copy link
Owner

Merged as part of #304. Thank you so much for your initial work on this!

@vladbat00 vladbat00 closed this Aug 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants