Skip to content

Introduce egui::FullOutput, returned from Context::run - #1292

Merged
emilk merged 2 commits into
masterfrom
refactor-output
Feb 22, 2022
Merged

emilk merged 2 commits into
masterfrom
refactor-output

Conversation

@emilk

@emilk emilk commented Feb 22, 2022

Copy link
Copy Markdown
Owner

This adds a new struct:

pub struct FullOutput {
    pub output: Output,
    pub needs_repaint: bool,
    pub textures_delta: epaint::textures::TexturesDelta,
    pub shapes: Vec<epaint::ClippedShape>,
}

The name is not great, nor the name of the nested Output, but structurally it works rather well.

Closes #1281

@derivator

Copy link
Copy Markdown

I think I prefer the names I suggested: since this is a breaking change anyway, you might as well rename Output... FullOutput is a bit awkward.
It's fine though, at least handle_egui_output does not return TexturesDelta anymore, I think that was the most confusing thing. 👍

@emilk

emilk commented Feb 22, 2022

Copy link
Copy Markdown
Owner Author

If we rename Output we should probably also rename ctx.output() which makes it even more of a breaking change :/

@derivator

derivator commented Feb 22, 2022

Copy link
Copy Markdown

Ah, I see. I guess the use case there is e.g. for custom widgets to be able to open a URL or set the cursor? Seems like it's not something that most users would encounter often. egui itself only calls output() 15 times. Maybe deprecate it with a note to use the new name? That way calls of the form ui.ctx().output() would still work even though the name of the return type changed...

This makes me notice another issue though: custom widgets probably want to be able to set needs_repaint, right? Possibly even insert textures or shapes into the output? That seems like it has huge footgun potential though...

Ultimately, this is getting into egui internals that I'm not that familiar with, so I'm happy to just trust your judgement here :)

@emilk

emilk commented Feb 22, 2022

Copy link
Copy Markdown
Owner Author

This makes me notice another issue though: custom widgets probably want to be able to set needs_repaint, right? Possibly even insert textures or shapes into the output? That seems like it has huge footgun potential though...

No - users should call ctx.request_repaint() and use the TextureManager and Painter API:s instead of touching the output directly.

@emilk

emilk commented Feb 22, 2022

Copy link
Copy Markdown
Owner Author

I renamed it PlatformOutput but kept the name ctx.output() for now. I don't want to spend too much time on this (I want to release egui 0.17.0!)

@derivator

Copy link
Copy Markdown

Looks good to me 👍 Thanks for all the work on egui, it really is a fantastic library :)

@emilk
emilk merged commit 31d3249 into master Feb 22, 2022
@emilk
emilk deleted the refactor-output branch February 22, 2022 16:13
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.

Awkward API for TexturesDelta in integrations

2 participants