Update iced to 13.0

[jeremieb24]

update iced dependency to 13.0
This is the release note, but I'll try to explain the changes required as best as I can:

• iced::advanced::Hasher was moved to iced::advanced::Subscription::Hasher.
• image::Handle::from_memory was renamed to image::Handle::from_bytes.
• Command API was changed to Task API which resulted in changing Command to Task. For most of the uses, not much was required, but I do have some concerns about the cmd() function that is used in the installer/updater/gui since I haven't worked a lot with the Command/Task API.
• there is no more Application implementation. I think they changed it so it is easier to understand and build small apps. I had to remove the Application implementation and change a bit the way the apps are launched. But in short, they replaced the Application::run with iced::application().run_with() and added a bunch of builders to set the settings and default state for each app.
• They removed the Id parameter from the CloseRequested event and changed the Id structure (the Id::MAIN constant was removed), so I had to find another way to close the window.
• seems like iced_runtime was no longer used after my changes so I removed it.
• I had to add wgpu to the feature flags since the app was no longer rendered correctly after I bumped the version. I saw that tiny-skia was used as a fallback in the doc, so I added it too just in case.

That's pretty much it, so my biggest concern is the changes to cmd functions that are related to the Task API switch.

I haven't fully tested all the ui apps yet, but they seem to work as intended. I'll try to do some more testing.
I think they also changed the default styling a bit, so there might be some minor styling differences, the only ones i found are the radio buttons that are a bit smaller (the left app is iced 13.0), and some padding/margin.

This version introduced the Stack widget, and I intended to use it to make a searchable picklist for the room name selection (since combobox menu was a bit janky when updating the room list in real-time). most of the work for that feature is already in progress on my side. I realized that this version bump was bigger than I anticipated. so I preferred not to include those changes as well.

leaving this as a draft until you have time to check it first. thanks

[jeremieb24]

also FYI, you can ignore the first commit about switching picklist to combobox, I messed up my branch creation and I removed those changes manually

[fenhl]

This looks pretty good at first glance! I'll give it a more detailed review later.

• Command API was changed to Task API which resulted in changing Command to Task. For most of the uses, not much was required, but I do have some concerns about the cmd() function that is used in the installer/updater/gui since I haven't worked a lot with the Command/Task API.

The cmd function seems good to me.

• They removed the Id parameter from the CloseRequested event and changed the Id structure (the Id::MAIN constant was removed), so I had to find another way to close the window.

That seems weird, I'll have to take a look at the changelog later to see what's up with that.

• I had to add wgpu to the feature flags since the app was no longer rendered correctly after I bumped the version. I saw that tiny-skia was used as a fallback in the doc, so I added it too just in case.

I've had to disable the wgpu feature previously since it was causing crashes for some users. Can you check if it still works if only the tiny-skia feature is enabled?

[jeremieb24]

I've had to disable the wgpu feature previously since it was causing crashes for some users

yeah, I figured there was a reason for already being disabled...

Can you check if it still works if only the tiny-skia feature is enabled?

I think I tested it as well since it was supposed to be a fallback or something, I just retested it this morning and it works with only tiny-skia flag. I'll try asking/looking around at the repo to see why we need that now. But without it, the app render a white screen

That seems weird, I'll have to take a look at the changelog later to see what's up with that.

I agree that it is strange, since I haven't found any way to get the id of the window. it could also just be me who misinterpreted the docs.
Now that I think about it, we can build an Id on the window when calling application::run_with() so we can probably add one and use that when calling close(id)

[jeremieb24]

I did find something about the rendering issue in the iced discord, I just searched for "white screen" and found some posts/threads about it. https://discord.com/channels/628993209984614400/1021828532189528094/1253448493247369348

what renderer are we using in MHMW if it's not wgpu or tiny-skia?

[fenhl]

We're using the tiny-skia renderer. It just wasn't behind a feature gate before iced 13.

[jeremieb24]

i've looked at the documentation about the removal of the Id::MAIN constant, seems like window::getLatest() is how to get the latest window id. was there a reason to condition on id != window::Id::MAIN when a CloseRequested event is triggered? it seems like there is only one window at a time in the GUI

I think the next step would be to define what manual tests are required before merging this.

[fenhl]

i've looked at the documentation about the removal of the Id::MAIN constant, seems like window::getLatest() is how to get the latest window id. was there a reason to condition on id != window::Id::MAIN when a CloseRequested event is triggered? it seems like there is only one window at a time in the GUI

I was just doing it that way because I was considering moving to a multi-window design at one point. I think we can just remove the check for now, and revisit it in case we ever do add extra windows.

I think the next step would be to define what manual tests are required before merging this.

I'm going to be honest, the amount of testing you've already done is more than I typically do for dependency upgrades 😅

[jeremieb24]

I'm going to be honest, the amount of testing you've already done is more than I typically do for dependency upgrades 😅

Thats fair, my bad... work habits I guess. I'm used to doing lots of testing before merging stuff, I wouldn't want to break anything on master branch... but I appreciate you letting me know about it

Then ill remove the draft. thanks

[fenhl]

Finally got time to properly review this now. I found the relevant change for the removal of window ID fields: iced-rs/iced#2456 — it seems like the expected way to get window IDs now is using iced::event::listen_with. I've pushed a commit to this branch to that effect. I've also changed the other two instances of code that previously closed the main window, which you changed to use iced::window::get_latest, to call iced::exit instead, which more closely matches the original intent of completely exiting the application rather than just closing one window. This doesn't make a difference right now but will be more correct in case we ever move to a multi-window GUI.

[fenhl]

Okay, there was another small issue that I've fixed, the PR was changing the window size of the updater (probably copy/paste error). This seems good to go now, thank you again for leading the effort on this upgrade!

[jeremieb24]

Sounds good, thanks for looking into it!

there was another small issue that I've fixed, the PR was changing the window size of the updater (probably copy/paste error)

this does ring a bell indeed. probably copy/paste like you said, my bad.

[jeremieb24]

ill rebase my fork and try to sneak in the searchable room name picklist before your next release