feat: fetch system information #1314

derezzedex · 2022-04-21T13:55:32Z

The goal of this PR is to improve ways to debug Iced's built-in backends creation/boot.

For that, it currently:

Adds a new Error::ContextCreationFailed in iced_graphics.
Adds some logging around context creation in both renderers (iced_glow and iced_wgpu).
Adds a new System variant to Action.
Adds a new System::QueryInformation action to fetch system and hardware information (gated by the sysinfo feature flag).
Exposes a generic system::Information struct containing acquired hardware information.

If there are any other points that could be worth logging or any new fields that we could add to system::Information, please let me know!

derezzedex · 2022-04-21T13:57:21Z

winit/src/application.rs

@@ -536,8 +543,10 @@ pub fn run_command<Message: 'static + std::fmt::Debug + Send, E: Executor>(
    clipboard: &mut Clipboard,
    proxy: &mut winit::event_loop::EventLoopProxy<Message>,
    window: &winit::window::Window,
+    graphics_info: &window::GraphicsInformation,


I couldn't think of other ways to do this, I'd be happy to receive feedback on this.

Maybe we could pass a closure impl FnOnce() -> window::GraphicsInformation (or the Compositor directly), so we can lazily request the information only for the relevant Command?

Done in 6e16767.

derezzedex · 2022-04-21T14:02:00Z

winit/src/system.rs

+    graphics_info: &window::GraphicsInformation,
+) -> Option<Information> {
+    use sysinfo::{ProcessExt, ProcessorExt, System, SystemExt};
+    let mut system = System::new_all();


The create says it would better to keep System instead of creating it everytime, but then it would need to be passed in a few places, so I decided to do it like this. Any ideas or feedback?

It depends on what "better" means! If it's just a performance optimization, then I believe it should be fine.

Because of 5eefa5d, I've decided to maintain this here (this should isolate the crate to the Action itself).
And it seems that refreshing and creating are almost the same.

derezzedex · 2022-04-21T14:18:12Z

winit/src/system.rs

+pub(crate) fn get_information(
+    _graphics_info: &window::GraphicsInformation,
+) -> Option<Information> {
+    None


I wasn't sure about removing the system::Information struct or/and removing the System::QueryInformation enum variant if sysinfo wasn't enabled, so I decided to turn this into an Option.

Any improvements on this would be nice!

I think we should simply conditionally compile the information function here (i.e. making it available only if sysinfo is enabled).

That way, besides removing the unnecessary Option, if a user tries to request system information in their code without the feature enabled, they will get a compiler error.

Simply conditionally compiling the system::information helper wouldn't work
because the run_command helper matches on every variant of Action and so
would throw the error for anyone not using sysinfo feature.

So, I've gone ahead and completely remove the sysinfo flag in 5eefa5d.
By not storing and delaying the fetching of information until requested (6e16767),
we remove the need of feature flagging.

Reverting this should be as simples as yanking the last commit.

Gave it a shot in 93bfe2c.

Interesting, would it be possible to also gate the iced_native side? That would make it impossible for someone to misuse this.

I've also commited a simple fix to another issue in a447772 (which I was planning on squashing, but didn't for now).

I don't think it would be possible since we cannot guarantee the implementation of every shell will support all of the Action variants.

In any case, I have plans to refactor the way Action and Command work to allow defining them outside of iced_native and introduce additional functionality to a shell.

hecrj

This is awesome. Great work! 👍

I left some suggestions here and there. Let me know what you think!

hecrj · 2022-04-25T07:50:52Z

graphics/src/window/compositor.rs

+
+/// Contains informations about the graphics (e.g. graphics adapter, graphics backend).
+#[derive(Debug)]
+pub struct GraphicsInformation {


Maybe just Information or Description? They both seem descriptive enough when prefixing them with compositor.

Renamed in 5be1ac1.

Additionally refactored access to compositor in 984d1f3, thus changing it to compositor::Information instead of window::Information.

hecrj · 2022-04-25T07:51:45Z

graphics/src/window/compositor.rs

@@ -38,6 +38,9 @@ pub trait Compositor: Sized {
        height: u32,
    );

+    /// Returns [`GraphicsInformation`] used by this [`Compositor`].
+    fn get_information(&self) -> GraphicsInformation;


I tend to avoid using getters in Rust. Does just information work?

If we are trying to convey the idea that this may be slow, then maybe we can use a different verb (e.g. request, fetch...).

Renamed in 005e516.

Since retrieving this information is done through a dependency (that uses system calls), I decided to stick with fetch_, although this is done synchronously.

hecrj · 2022-04-25T07:56:10Z

src/error.rs

-    GraphicsAdapterNotFound,
+    /// The application context could not be created.
+    #[error("the application context could not be created")]
+    ContextCreationFailed(iced_graphics::Error),


Context is a very ambiguous term and extremely overused. Maybe we can say GraphicsCreationFailed?

Done in 18ecec2.

hecrj · 2022-04-25T07:58:03Z

wgpu/src/window/compositor.rs

+            let available_adapters: Vec<_> = instance
+                .enumerate_adapters(settings.internal_backend)
+                .map(|adapter| adapter.get_info())
+                .collect();
+            log::info!("Available adapters: {:#?}", available_adapters);


Since we are only collecting these for logging, maybe we can do it conditionally based on log::max_level?

Done in 7a13928.

hecrj · 2022-04-25T08:01:35Z

winit/src/application.rs

@@ -536,8 +543,10 @@ pub fn run_command<Message: 'static + std::fmt::Debug + Send, E: Executor>(
    clipboard: &mut Clipboard,
    proxy: &mut winit::event_loop::EventLoopProxy<Message>,
    window: &winit::window::Window,
+    graphics_info: &window::GraphicsInformation,


Maybe we could pass a closure impl FnOnce() -> window::GraphicsInformation (or the Compositor directly), so we can lazily request the information only for the relevant Command?

hecrj · 2022-04-25T08:04:26Z

winit/src/system.rs

+/// Query for available system information.
+///
+/// Returns `None` if not using the `sysinfo` feature flag.
+pub fn information<Message>(


Should we rename it to request_information?

Done in 8643fbe.

hecrj · 2022-04-25T08:05:56Z

winit/src/system.rs

+    graphics_info: &window::GraphicsInformation,
+) -> Option<Information> {
+    use sysinfo::{ProcessExt, ProcessorExt, System, SystemExt};
+    let mut system = System::new_all();


It depends on what "better" means! If it's just a performance optimization, then I believe it should be fine.

hecrj · 2022-04-25T08:08:56Z

winit/src/system.rs

+pub(crate) fn get_information(
+    _graphics_info: &window::GraphicsInformation,
+) -> Option<Information> {
+    None


I think we should simply conditionally compile the information function here (i.e. making it available only if sysinfo is enabled).

That way, besides removing the unnecessary Option, if a user tries to request system information in their code without the feature enabled, they will get a compiler error.

hecrj

Looks good!

I noticed that the action blocks the main thread for a second or two. It's not a big deal since you shouldn't be calling this often, but it's still a bit jarring to freeze the GUI to users. So I have moved the slow call to a different thread in f1ec0af.

Let me know what you think and we can merge this!

... since it seems it can block for a couple of seconds.

This fixes the `system` module not being exported in `iced_glutin` when only setting the `iced_winit/system` flag.

Introduce Error::ContextCreationFailed

1e62fdf

derezzedex commented Apr 21, 2022

View reviewed changes

derezzedex marked this pull request as ready for review April 21, 2022 14:03

derezzedex commented Apr 21, 2022

View reviewed changes

hecrj added feature New feature or request compatibility labels Apr 24, 2022

hecrj requested changes Apr 25, 2022

View reviewed changes

derezzedex added 22 commits April 26, 2022 18:54

Add logging to window and context creation

764b424

Add sysinfo crate through feature flag

6e93713

Add new System variant to Action

fed8da1

Introduce QueryInformation to system::Action

6978149

Implement QueryInformation for iced_winit

6295a72

Turn Information fields pub

c2f45a1

Add function helper in iced_winit::system

0b36a55

Export new system module

c8ed318

Add system_information example

53538b6

Add unformated memory total to example

d985316

Export system module in iced_glutin

ced5f50

Introduce GraphicsInformation to iced_graphics

e23e4b8

Implement GraphicsInformation for iced_wgpu

83fec2f

Implement GraphicsInformation for iced_glow

2b4d8a7

Add graphics information to system::Information

5356bb9

Add graphics information to iced_winit

83ab27b

Add graphics information to iced_glutin

75281d2

Add graphics information to example

56065fe

Improve example readability

5bfe887

Add memory usage to Information struct

c9ea1f1

Introduce get_information

053f352

Enumerate adapters based on log level

7a13928

derezzedex added 6 commits April 26, 2022 19:09

Rename GraphicsInformation to Information

5be1ac1

Move compositor module access from window to crate

984d1f3

Rename get_information to fetch_information

005e516

Change ContextCreationFailed to GraphicsCreationFailed

18ecec2

Rename system::information to fetch_information

8643fbe

Use closure internally to fetch graphics_info

6e16767

derezzedex force-pushed the dev/system-information branch from 53eceb6 to e6b09e9 Compare April 27, 2022 20:03

Simplify the QueryInformation Action

5eefa5d

derezzedex force-pushed the dev/system-information branch from e6b09e9 to 5eefa5d Compare April 27, 2022 20:16

derezzedex requested a review from hecrj April 27, 2022 20:18

hecrj added this to the 0.5.0 milestone Apr 30, 2022

hecrj and others added 2 commits April 30, 2022 13:37

Expose system module through feature flag

93bfe2c

Add logging to window and context creation

a447772

derezzedex force-pushed the dev/system-information branch from 49356a8 to a447772 Compare May 2, 2022 19:06

Fix feature name in cfg block in iced_winit

e24f26c

hecrj approved these changes May 4, 2022

View reviewed changes

hecrj force-pushed the dev/system-information branch 3 times, most recently from 480cd13 to e34f1d0 Compare May 4, 2022 13:19

hecrj added 2 commits May 4, 2022 15:21

Run system::information in a different thread

f1ec0af

... since it seems it can block for a couple of seconds.

Show Loading screen when refreshing in system_information example

1aeb8b8

hecrj force-pushed the dev/system-information branch from e34f1d0 to 1aeb8b8 Compare May 4, 2022 13:21

hecrj and others added 4 commits May 4, 2022 17:15

Re-export iced_winit in iced_glutin

3e735f1

This fixes the `system` module not being exported in `iced_glutin` when only setting the `iced_winit/system` flag.

Sort workspace members in Cargo.toml

a97c520

Merge branch 'master' into dev/system-information

27fdc70

Merge branch 'master' into dev/system-information

b440df9

hecrj merged commit 2e7757a into iced-rs:master May 11, 2022

hecrj mentioned this pull request Nov 10, 2022

Release 0.5 — Stabilization of stateless widgets, first-class theming, widget operations, Lazy widget, and more! #1520

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: fetch system information #1314

feat: fetch system information #1314

derezzedex commented Apr 21, 2022

derezzedex Apr 21, 2022

hecrj Apr 25, 2022

derezzedex Apr 27, 2022

derezzedex Apr 21, 2022 •

edited

Loading

hecrj Apr 25, 2022

derezzedex Apr 27, 2022

derezzedex Apr 21, 2022

hecrj Apr 25, 2022

derezzedex Apr 27, 2022 •

edited

Loading

hecrj Apr 30, 2022

derezzedex May 2, 2022

hecrj May 4, 2022

hecrj left a comment

hecrj Apr 25, 2022

derezzedex Apr 27, 2022

hecrj Apr 25, 2022

derezzedex Apr 27, 2022

hecrj Apr 25, 2022

derezzedex Apr 27, 2022

hecrj Apr 25, 2022

derezzedex Apr 27, 2022

hecrj Apr 25, 2022

hecrj Apr 25, 2022

derezzedex Apr 27, 2022

hecrj Apr 25, 2022

hecrj Apr 25, 2022

hecrj left a comment •

edited

Loading

feat: fetch system information #1314

feat: fetch system information #1314

Conversation

derezzedex commented Apr 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derezzedex Apr 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derezzedex Apr 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hecrj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hecrj left a comment • edited Loading

Choose a reason for hiding this comment

derezzedex Apr 21, 2022 •

edited

Loading

derezzedex Apr 27, 2022 •

edited

Loading

hecrj left a comment •

edited

Loading