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

Use bitfield instead of bools in Response #5556

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

polwel
Copy link

@polwel polwel commented Jan 1, 2025

Closes #3862.

Factoring the bool members of Response into a bitfield, the size of Response is now 96 bytes (down from 104).

I gave Sense the same treatment, however this has no effects on Response due to padding. I've decided not to pursue PointerState, as it is quite large (many members that are sized and aligned to multiples of 8 bytes), so I don't expect any noticeable benefit from making handful of bools slightly leaner.

In any case, the changes to Sense are already quite a bit more intrusive than those to Response.
The previous implementation overloaded the names of the attributes click and drag with similarly named methods that construct Sense with the corresponding flag set. Now, that the attributes can no longer be accessed directly, I had to introduce methods with new names (senses_click(), senses_drag() and is_focusable()). I don't think this is the cleanest solution: the old methods are essentially redundant now that the named constants like Sense::CLICK exist. I did however not want to needlessly break backwards compatibility.
I am happy to revert it (or go further 🙂) if there are concerns.

@@ -133,6 +67,71 @@ pub struct Response {
/// for improved layouting.
/// See for instance [`egui_flex`](https://github.com/lucasmerlin/hello_egui/tree/main/crates/egui_flex).
pub intrinsic_size: Option<Vec2>,

#[doc(hidden)]
pub(crate) flags: Flags,
Copy link
Author

Choose a reason for hiding this comment

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

I am slightly confused by the // IN and // OUT comments (still unsure what they mean), and also that all the fields are pub. I made this one pub(crate) as I really don't see why users would need to muck with it. I would even have made it so that even Context can't access the field directly, and has to go through accessor methods instead of messing with flags.contains(), flags.set(), etc. But I did not want to go overboard with my 1st attempt of hacking on egui 🙂

@polwel polwel changed the title Use bitfield instead of bools in Response (#3862) Use bitfield instead of bools in Response Jan 4, 2025
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.

Performance: replace bools in Response with bit-sets
1 participant