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

Iced 0.12 #597

Merged
merged 9 commits into from
May 3, 2024
Merged

Iced 0.12 #597

merged 9 commits into from
May 3, 2024

Conversation

edouardparis
Copy link
Member

@edouardparis edouardparis commented Aug 8, 2023

This PR does the migration from iced 0.9 to iced 0.12.

This new iced version has impact on the fonts size. I reverted the size according to the original UX figma file.

The new backend renderer is the wgpu with tiny-skia as a fallback. wgpu is the first class citizen of the iced renderers, it supports everything. The tiny-skia has some layout problems and does not support some features that is the reason why this PR introduces tiny change in the theme or long string display (ad69711, 88fd0f1).

In order to keep the MSRV as low as possible, a custom patch of the crates iced_winit,iced_style, iced_futures is added to the Cargo.toml

@edouardparis edouardparis force-pushed the iced-0.10 branch 4 times, most recently from 31f31a9 to 609b6cf Compare August 8, 2023 15:07
@darosior
Copy link
Contributor

darosior commented Aug 9, 2023

Iced 0.10 is enabling renderer fallback from wgpu to tiny-skia if wgpu cannot work.

How does this affect our runtime dependency requirements?

@darosior
Copy link
Contributor

darosior commented Aug 9, 2023

Font size are now too big, I guess it is because of the new cosmic-text feature of iced, we need to update it.

Does that mean we could maybe drop fontconfig as a dependency?

@darosior
Copy link
Contributor

darosior commented Aug 9, 2023

Also, leaving a message here as a reminder: we need to check the Guix build and the MSRV here. Also the build documentation may have to be updated.

@darosior
Copy link
Contributor

Just leaving a note that we'll target the next release for this, as we are aiming for a v2 feature freeze at the beginning of next week.

@edouardparis
Copy link
Member Author

This PR sadly reintroduce the needs for X11 users to have the WINIT_X11_SCALE_FACTOR env var set to 1.
See https://docs.rs/winit/latest/winit/dpi/index.html#how-is-the-scale-factor-calculated

@darosior
Copy link
Contributor

darosior commented Jan 5, 2024

What does it mean? Why? Could you be a bit clearer and give more context? It's hard to use our judgement otherwise.

@edouardparis
Copy link
Member Author

iced uses logical pixels. Sizes are scaled by your monitor's scale factor to target physical pixels during rendering.
Scale factor is determined by different ways: https://docs.rs/winit/0.28.7/winit/dpi/index.html
If you have X11, it may be possible that winit makes a wrong guess of the screen scale factor, and you can see in the logs:

2024-01-05T14:10:20.309331Z  INFO winit::platform_impl::platform::x11::window:156: Guessed window scale factor: 1.1666666666666667

which explain why the everything seems too big.
winit does not see it as an issue
The current way to force it to scale it properly at 1, is to set the env var WINIT_X11_SCALE_FACTOR=1

We had the same problem at previous liana releases, see the troubleshooting section of the gui readme
I am asking to iced devs why we do not have the problem with iced-0.9

This issue affects only linux users using X11.

@darosior
Copy link
Contributor

What's left to do here? Would be nice to get it in early so 1) you don't have to maintain such a large diff across conflicts 2) we can catch any bug early on.

@edouardparis
Copy link
Member Author

@kloaec found some problems:
with tiny-skia backend: same problem than iced-rs/iced#2173
with wgpu, image are black rectangle:

Next version of iced will reintroduce support of WebGL
https://github.com/iced-rs/iced/blob/master/Cargo.toml#L49
I hope it will allow us to catch up iced new features and have the current behavior with still the same requirements, while bugs are fixed on their side or downstream (wgpu etc)

@darosior
Copy link
Contributor

Another comment in case we forget about it: we do still rely on your patched iced_futures right?

@edouardparis
Copy link
Member Author

Right, I removed it for tests, but I think we still need it, there was no fix yet on iced side
https://github.com/wizardsardine/liana/pull/597/files#diff-6ec93b2f00bf881b5541dd9ebd0611a05738fcf970422f3ae38f13c3a5414650L51

@edouardparis edouardparis force-pushed the iced-0.10 branch 6 times, most recently from eee4bb2 to ed7eafd Compare March 29, 2024 17:17
@edouardparis edouardparis changed the title Iced 0.10 Iced 0.12 Apr 2, 2024
@edouardparis edouardparis force-pushed the iced-0.10 branch 2 times, most recently from 12e9820 to a512177 Compare April 2, 2024 14:37
edouardparis added a commit that referenced this pull request Apr 2, 2024
d722ca1 Use text size constant (edouardparis)

Pull request description:

  this is preparatory work for #597

ACKs for top commit:
  edouardparis:
    Self-ACK d722ca1

Tree-SHA512: 882fefe1685e34b252e6ca0dba9e2519b4889448cb410c3df542b2ca111baccbd2697b6ca191caa069ffec55dbdc0182c76de75a36f321f4927b6e81229db07b
darosior added a commit that referenced this pull request Apr 29, 2024
860a1ea guix: use Rust 1.70 for release builds (Antoine Poinsot)

Pull request description:

  This is prep work for #597, which bumps the MSRV of the GUI to 1.70.

  We are being pulled over in two different directions when it comes to our reproducible builds. On the one hand we need to target reasonably old glibc versions in order to be compatible with older systems. On the other hand the immaturity of the Rust ecosystem makes us require bleeding edge versions of the compiler. With Guix to get the newer versions of the compiler we need to also bump the glibc version.

  This was not a sustainable situation. I was planning for a long time to cleanup our reproducible builds. To create a proper Guix package for both the daemon and the GUI using the build system they provide. I had envisioned this way i could rewrite the inputs of the Guix package to use an older glibc, while being able to bump the Guix time-machine. It would even have allowed us to perform Windows builds inside Guix! And who knows i could even have attempted to perform Apple ones too.

  Unfortunately it turned out to be more complicated than that. I couldn't manage to get my package to compile using an older glibc. Some details about some of my failed attempts can be found there: https://lists.gnu.org/archive/html/help-guix/2024-04/msg00056.html.

  Instead of wasting more time on this, backport the newer Rust declarations from up-to-date Guix to Guix-of-our-time-machine.

ACKs for top commit:
  edouardparis:
    utACK 860a1ea

Tree-SHA512: ee0a753376b380c5b39d9cefd6ac49c95d818b3233183f6832df256b93fe5627bf5d9193de1340ae12c0e8e8ec8e5c869674e975d5066723d9d38e66a509cbef
@darosior
Copy link
Contributor

This can be rebased now that #1080 was merged.

@darosior
Copy link
Contributor

darosior commented Apr 29, 2024

The new branch you use in your Github repo also needs to be updated in the Guix config:

diff --git a/contrib/reproducible/guix/build.sh b/contrib/reproducible/guix/build.sh
index 1499723..482632a 100755
--- a/contrib/reproducible/guix/build.sh
+++ b/contrib/reproducible/guix/build.sh
@@ -20,7 +20,7 @@ replace-with = "vendored_sources"
 
 [source."https://github.com/edouardparis/iced"]
 git = "https://github.com/edouardparis/iced"
-branch = "fix-futures-recipe"
+branch = "patch-0.12.3"
 replace-with = "vendored_sources"
 EOF

@darosior
Copy link
Contributor

@edouardparis a simple cargo vendor doesn't work anymore with the new iced branch, could you check what's wrong?

@edouardparis
Copy link
Member Author

I have no error running cargo vendor both with rust 1.70 and nightly

@darosior
Copy link
Contributor

Ha! It's the version of Cargo used to vendor which is too old..

@darosior
Copy link
Contributor

Please update the PR to include my patch from above #597 (comment).

Here are the hashes i get on this PR with my patch by running

BUILD_ROOT=/tmp/tmp.Geqp4c0ail ./contrib/reproducible/guix/guix-build.sh && sha256sum /tmp/tmp.Geqp4c0ail/out/release/lianad && sha256sum /tmp/tmp.Geqp4c0ail/out/gui/release/liana-gui
Build successful. Output available at /tmp/tmp.Geqp4c0ail/out
779d1ae77ff7baf18df1dc9d1f48d4abb918491d0cd39a28b61d5ea0f8ede46d  /tmp/tmp.Geqp4c0ail/out/release/lianad
8914b89458d6af2983347964eeab66b8188dbb1d7dc90718130ee4f7ec324836  /tmp/tmp.Geqp4c0ail/out/gui/release/liana-gui

@edouardparis
Copy link
Member Author

edouardparis commented Apr 30, 2024

sha256sum /tmp/tmp.5xnaVVlBAx/out/release/lianad && sha256sum /tmp/tmp.5xnaVVlBAx/out/gui/release/liana-gui
779d1ae77ff7baf18df1dc9d1f48d4abb918491d0cd39a28b61d5ea0f8ede46d  /tmp/tmp.5xnaVVlBAx/out/release/lianad
8914b89458d6af2983347964eeab66b8188dbb1d7dc90718130ee4f7ec324836  /tmp/tmp.5xnaVVlBAx/out/gui/release/liana-gui

@darosior
Copy link
Contributor

darosior commented May 1, 2024

Can you update OP? It's quite confusing to have TODO items while this is going to be merged very soon.

@edouardparis
Copy link
Member Author

I updated the OP

@darosior
Copy link
Contributor

darosior commented May 2, 2024

For what it's worth i'm waiting on #1085 to fix CI..

@edouardparis edouardparis force-pushed the iced-0.10 branch 2 times, most recently from 4a0db63 to 641985e Compare May 2, 2024 12:54
@darosior
Copy link
Contributor

darosior commented May 3, 2024

Needs rebase, the last commit is now in master

to enable the fallback:
ICED_BACKEND=tiny-skia cargo run
View is broken because of the long line of text of the
descriptor, current fix is to put it behind a scrollable
until we find a better way to display it to the user.
@darosior
Copy link
Contributor

darosior commented May 3, 2024

ACK 52e32b6 -- it's been tested a bunch, in particular by Kevin.

@darosior darosior merged commit b602640 into wizardsardine:master May 3, 2024
21 checks passed
edouardparis added a commit that referenced this pull request May 3, 2024
9cd36a7 gui: enable advanced text shaping (edouardparis)

Pull request description:

  Set text shaping and font fallback to text widget.
  It enable Emojis

  based on #597

ACKs for top commit:
  edouardparis:
    Self-ACK 9cd36a7

Tree-SHA512: 1ed1621135c5bfbd624a9cf4bd96385db42d021e8432334429b2ec376d769715920d90d76cc000f43625367bdac85444b131be249197e4fab4d5629c7a0138e1
@nondiremanuel nondiremanuel added this to the Liana v6 milestone Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants