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

bug(perf): auto-theme performance regression #650

Closed
3 tasks done
Rudxain opened this issue Sep 24, 2024 · 10 comments · Fixed by #702
Closed
3 tasks done

bug(perf): auto-theme performance regression #650

Rudxain opened this issue Sep 24, 2024 · 10 comments · Fixed by #702
Labels
bug Something isn't working
Milestone

Comments

@Rudxain
Copy link
Member

Rudxain commented Sep 24, 2024

Describe the bug

#531, more specifically this commit 76aba1e (according to git bisect), causes the entire UI to become sluggish on a Dell Inspiron 15R.

IDK exact perf, but I guess previous commit (5f43725) is ~60FPS and the one I referenced is ~1FPS (no device connected) or ~0.5FPS (when the list of installed packs has been fetched)

Expected behavior

UAD-NG must have smooth performance while idle

You have a solution?

I suspect Iced is somehow calling dark_light::detect every frame 💀 (possibly our fault).

If we upgrade to the new Iced (and ditch our implementation), then the dark_light integration should work properly as-is

Operating System

Linux

Distro version

Debian 13

Window System + Manager

Wayland + Mutter

Desktop Environment

GNOME 46 (some apps are v47)

Provide logs

This may be irrelevant:

2024-09-24 13:34:41 WARN  [/home/rudxain/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-hal-0.19.4/src/vulkan/instance.rs:696] InstanceFlags::VALIDATION requested, but unable to find layer: VK_LAYER_KHRONOS_validation
MESA-INTEL: warning: Ivy Bridge Vulkan support is incomplete
2024-09-24 13:34:41 WARN  [/home/rudxain/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-hal-0.19.4/src/vulkan/adapter.rs:1143] Adapter is not Vulkan compliant, hiding adapter: Intel(R) HD Graphics 4000 (IVB GT2)

Acknowledgements

  • This issue is not a duplicate of an existing bug report.
  • I have chosen an appropriate title.
  • All requested information has been provided properly.
@Rudxain Rudxain added the bug Something isn't working label Sep 24, 2024
@AnonymousWP AnonymousWP added this to the v1.1.1 milestone Sep 24, 2024
@neingeist
Copy link

This makes the UI very slow, btw!

@Rudxain
Copy link
Member Author

Rudxain commented Sep 25, 2024

I've confirmed that if the user selects any theme other than "Auto", the UI performance is fast.

Being optimistic, this means there's a workaround for users who don't want to await the next release (minor or hotfix patch). The feature is broken anyways (#627), so I doubt most people would use it as-is 🥲

@neingeist
Copy link

I've confirmed that if the user selects any theme other than "Auto", the UI performance is fast.

This workaround also works for me ⚡

@Rudxain
Copy link
Member Author

Rudxain commented Oct 26, 2024

I doubt most people would use it

I was wrong. It's the default setting:

#[default]
/// `Dark` or `Light`, according to `dark_light`
Auto,

We should fix this ASAP

@AnonymousWP
Copy link
Member

I doubt most people would use it

I was wrong. It's the default setting:

#[default]
/// `Dark` or `Light`, according to `dark_light`
Auto,

We should fix this ASAP

Is it an easy fix?

@ZMJGMADHPXWT
Copy link

Workaround works well, thank you for help!

@Rudxain
Copy link
Member Author

Rudxain commented Oct 27, 2024

Is it an easy fix?

Sure! we can simply move the attribute to Dark (as per #540)

But the config file on some users' devices will still be set to "Auto". So UADNG should deem Auto as invalid (as if the theme didn't exist) and fall-back to Dark

@Rudxain
Copy link
Member Author

Rudxain commented Oct 28, 2024

Iced is somehow calling dark_light::detect

Found the culprit (search palette() to see the horror 💀)

Who assigned to p and then forgot to use it literally 4 lines later? 😭

let p = self.palette();
let appearance = button::Appearance {
border: Border {
color: self.palette().normal.primary,

@Rudxain
Copy link
Member Author

Rudxain commented Oct 28, 2024

I'm already working on using static_init to cache dark_light on startup. This should fix the perf problem and patch (not a real fix) #550 !

@AnonymousWP
Copy link
Member

Great work!

@Rudxain Rudxain changed the title bug(perf): auto-theme introduces performance regression bug(perf): auto-theme performance regression Nov 1, 2024
@Rudxain Rudxain pinned this issue Nov 6, 2024
@Rudxain Rudxain unpinned this issue Nov 6, 2024
@Rudxain Rudxain pinned this issue Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants