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

Avoid allocations for loader cache lookup #5584

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

mineichen
Copy link
Contributor

[ x ] I have followed read the instructions in the PR template

Unfortunately i had several issues:

  • Some snapshot-tests didn't run successfully on osx. diff shows errors around fonts or missing menu items)
  • cargo clippy doesn't run successfully (egui_kittest cannot find wgpu and image)
  • ./scripts/check.sh had other issues on my system (env: python: No such file or directory), even if python3 can be called via python in my shell

Is there a system independent, standard way to run these tools (e.g. via Docker?)
I submit the pr anyway, because there changes are very simple and shouldn't cause issues.

@YgorSouza
Copy link
Contributor

Is there a system independent, standard way to run these tools (e.g. via Docker?)

I made #4293 long ago to start converting the scripts to Rust to avoid this kind of problem, but I never got around to converting the rest of them. I'll see if I can get back to it. But the clippy issue seems to be a bug in egui_kittest. It doesn't compile with default features, and that is not caught by the CI which only runs clippy with --all-features. There is also the problem that what the CI currently does is different from what contributors are asked to do locally, so the checks can fail locally and pass on the CI and vice versa. Plenty of room for improvement there, but it will take some developer time.

Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Nice

@emilk emilk added performance Lower CPU/GPU usage (optimize) egui egui_extras labels Jan 6, 2025
Copy link

github-actions bot commented Jan 6, 2025

Preview available at https://egui-pr-preview.github.io/pr/5584-avoidloaderallocations
Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.

@emilk emilk merged commit 0fac8ea into emilk:master Jan 6, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
egui_extras egui performance Lower CPU/GPU usage (optimize)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants