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

plymouth: attempt to make flicker-free #84158

Closed
wants to merge 7 commits into from

Conversation

eadwu
Copy link
Member

@eadwu eadwu commented Apr 3, 2020

If you rig up some plymouth prompts it should work fine for decryption and messages now. Patches are a big mess.

Not sure what's actually needed in stage-1 for this to run as early as possible.

Motivation for this change

label-ft: https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/91

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@matthewbauer
Copy link
Member

Awesome work! Haven't tried it yet, but this is definitely nice to have.

On stage 1, I think we may need to use the systemd integration more for this to work with plymouth. I'm not sure what this takes, but it had been brought up at one point.

Related to #26722 and #44965

@eadwu
Copy link
Member Author

eadwu commented Apr 3, 2020

Right now I'm using a makeshift wrapper for reading and showing messages, but the implementation is works only on bash due to the requirement on type.

    if [ ! "$(type -t _read)" = "function" ]; then
      _read() {
        local pwd
        pwd="$(plymouth ask-for-password "$@")"
        printf "$pwd"
      }
    fi

    if [ ! "$(type -t _prompt)" = "function" ]; then
      _prompt() {
        plymouth display-message "$@"
      }
    fi

but this ends up becoming far over-complicated because of the differing usages of outputting within initrd (or unless we enforce a strict format for the function, like for _read plymouth can only do --text "$1" and _prompt can only do --text "$1" which would probably work just fine).

Not sure on how to replicate non-newline displaying of messages, but it can be partially mitigated by having a sort of message history in the plymouth theme itself (bottom right on the image).
Screenshot_2020-04-03_12-49-38

@worldofpeace
Copy link
Contributor

cc @mkg20001

@matthewbauer
Copy link
Member

I'm getting this when building an OVA image:

output '/nix/store/d8yv4bc0kw9j3dg9fc3n2lp8qlljf6i4-extra-utils' is not allowed to refer to the following paths:
  /nix/store/il5jhmg7m01sflpz1g582dvwsii6byb9-plymouth-2020-03-25

@eadwu
Copy link
Member Author

eadwu commented Apr 4, 2020

Should've fixed. Seemed to be a problem that occurred with the default settings.

Tried booting it up with the default theme and it should work fine as long as the password issue is resolved for passwords (without bash-specific workarounds). I'll work on that when I'm able to work on the PR for an extended period of time.

The default theme also has a problem with the password prompt "bullets" which I think is probably due to the font not containing the character.

local.bullet_image = WriteText("•", palette.text.contrast);

@worldofpeace
Copy link
Contributor

Not sure what's actually needed in stage-1 for this to run as early as possible.

I believe from the people I've talked to about this, it would be #74842.

@lukateras lukateras self-requested a review April 8, 2020 10:58
@eadwu eadwu force-pushed the plymouth/less-flicker branch 2 times, most recently from 6df8088 to 039b3ed Compare April 10, 2020 21:35
pkgs/os-specific/linux/plymouth/default.nix Show resolved Hide resolved
pkgs/os-specific/linux/plymouth/default.nix Show resolved Hide resolved
pkgs/os-specific/linux/plymouth/default.nix Outdated Show resolved Hide resolved
# ./only_use_fb_for_cirrus_bochs.patch
] ++ stdenv.lib.optionals withLabelFt [
# https://gitlab.freedesktop.org/plymouth/plymouth/-/issues/45
./v3-0001-ply-label-Don-t-crash-if-label-plugin-fails.patch
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this patch set necessary for flicker-free experience? I would rather not include unmerged patches using legacy libraries (freetype).

Copy link
Member Author

Choose a reason for hiding this comment

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

It isn't necessary but without it, some of the functionality for some themes will be impacted, anything that uses display-message won't be shown since we don't bundle the original label.so since it's far more bloated than label-ft.so.

But if it's just the password prompt (basically a pile of images) and the background, then no it isn't needed.

@berbiche
Copy link
Member

berbiche commented Sep 16, 2020

How can I test this in my configuration?

Am I right to assume I would only need to update my nixpkgs ref to this PR?

@davidak
Copy link
Member

davidak commented Sep 16, 2020

@berbiche you have also to enable plymouth, but then it should work.

[Daemon]
ShowDelay=0
DeviceTimeout=8
Copy link
Member

Choose a reason for hiding this comment

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

Should there be a way to make this configurable?

@davidak
Copy link
Member

davidak commented Dec 26, 2020

I don't see a difference. https://youtu.be/wHbl62EgrlE

The stage1 text is still shown, then a black screen, and after 20 seconds i see the lightdm login.

i'm using Pantheon desktop
EFI systemd-boot
no disk encryption

should i test on a system with disk encryption? or still too early to test?

@eadwu
Copy link
Member Author

eadwu commented Dec 26, 2020

Took a quick glance at the related PRs and still doesn't look like it's possible. The earliest I went was stage-2 I believe (I don't remember)?

I think I broke initrd without plymouth (or at least if there are any secrets needed to be inputted) with the latest commit but I'll just keep it there since I usually just had it as a stash locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants