Skip to content

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

blackberryfloat
Copy link

@blackberryfloat blackberryfloat commented Jun 3, 2025

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)

Copy link
Collaborator

@lucasmerlin lucasmerlin left a 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

@lucasmerlin lucasmerlin added this to the 0.32.0 milestone Jun 16, 2025
@blackberryfloat blackberryfloat force-pushed the wm/allow-memory-removal branch from c3d4446 to 34ab1f7 Compare June 16, 2025 17:04
@blackberryfloat blackberryfloat requested a review from Wumpf as a code owner June 16, 2025 17:04
@blackberryfloat blackberryfloat changed the base branch from patch-0.31.1 to main June 16, 2025 17:17
@blackberryfloat
Copy link
Author

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

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.

Comment on lines +373 to +375
/// Default: `false`.
#[inline]
pub fn sizing_pass(mut self, resize: bool) -> Self {
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 explain that the user should only call this ONCE, i.e. not have a hard-coded .sizing_pass(true) in their code.

Copy link
Author

@blackberryfloat blackberryfloat Jun 24, 2025

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?

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.

4 participants