-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Update area struct to allow force resizing #7114
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
base: main
Are you sure you want to change the base?
Update area struct to allow force resizing #7114
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I'm surprised this wasn't possible before. This probably won't be necessary after #5138 but since that might not be fixed by next egui release we should still merge this. But since we'll have a release soon please change the PR to merge to the main branch
c3d4446
to
34ab1f7
Compare
I was too. I assumed I might be missing a known workflow or something. I figured posting a pull request was the best way to find out and I might provide something useful otherwise. |
/// Default: `false`. | ||
#[inline] | ||
pub fn sizing_pass(mut self, resize: bool) -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's explain that the user should only call this ONCE, i.e. not have a hard-coded .sizing_pass(true)
in their code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get the intent for helping make sure people don't misuse this but this function is meant to be used multiple times. I feel like the description effectively conveys that if it is true what the implications are and that false keeps the default behavior. I am open to wording ideas but my expectation was that people will connect that while the variable it true the content will always be invisible.
If we only needed this once, this function would be pointless since the creation of the Area would solve that singular use. My expectation and how I have used it is to call this function every frame and the flag being passed to it only gets set to true when the content within the area changes (ie len of an alerts vector changes).
I can add a note about my usage if that makes sense?
This is a really small PR so I am skipping the issue (based on contributing.md). This change adds an optional field and thus non breaking for the API.
I ran into an issue during my development of an alerts manager widget (see PR) where I needed a scrollable overlay that did not block clicking areas of a parent widget when my alerts did not take up the entire parent. To achieve this I detect the sizing pass via the invisible flag and only render the alerts content and then on the next pass I add the scroll bar in around the alert content. Whenever the alert content changed though I would need to create a new Area with a new id to get proper sizing. That is a memory leak so I wanted to reset the size state to trigger a sizing pass. Memory is rightfully protected enough that the path to remove memory was dropped and I just added a hook to set a resize flag.
I am sure there are better ways but this is what made sense to me. Looking forward to thoughts.
Logistics wise, I have proposed it as a patch because I was based off 0.31.1 for testing. I was also thinking it could be released quickly. I am happy to cherry pick onto main after. If that is not allowed I can rebase to main and pull against that.(rebased on main)