| Summary: | [PATCH] Fix details screen not showing when fbcon is loaded after starting plymouth | ||
|---|---|---|---|
| Product: | plymouth | Reporter: | Hans de Goede <jwrdegoede> |
| Component: | general | Assignee: | Ray Strode [halfline] <rstrode> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | medium | CC: | rstrode |
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
[PATCH 1/3] ply-renderer: Fix activate handling
[PATCH 2/3] main: Do not activate renderers when showing details [PATCH 3/3] main: Move ply_device_manager_deactivate_renderers() into hide_splash() [PATCH] show_theme: Only activate renderers if the splash uses pixel-displays [PATCH] main: Consistently only (de)activate renderers if the splash uses pixel-displays |
||
|
Description
Hans de Goede
2018-06-26 18:53:47 UTC
Created attachment 140349 [details] [review] [PATCH 2/3] main: Do not activate renderers when showing details Created attachment 140350 [details] [review] [PATCH 3/3] main: Move ply_device_manager_deactivate_renderers() into hide_splash() Comment on attachment 140349 [details] [review] [PATCH 2/3] main: Do not activate renderers when showing details Review of attachment 140349 [details] [review]: ----------------------------------------------------------------- so your patches make sense to me, but I don't think this one is good enough. At the moment showing_details is just used to determine what action pressing the escape key should do..which way to toggle. You want to use it to know whether or not text is showing on screen, but the problem is, that details aren't the only case where text is showing on screen. we also show text for other text based splash plugins. I think probably we should add a function `ply_boot_splash_needs_graphics` or `ply_boot_splash_uses_pixel_displays` or something like that just does: `return splash->plugin_interface->add_pixel_display != NULL;` then activate_renderers would only get called when that function returns true. i'm going to be gone next week, but feel free to build these patches in Fedora in the interim. i pushed two of these: To ssh://git.freedesktop.org/git/plymouth 014c215..778e0fb master -> master Thank you for the review and for pushing the 2 patches. Monday (tomorrow), I will modify the third patch as you suggested, retest and then attach it here. I will also kick of a F29 plymouth build with the 3 patches added then. Created attachment 140422 [details] [review] [PATCH] show_theme: Only activate renderers if the splash uses pixel-displays Here is a patch replacing the "[PATCH 2/3] main: Do not activate renderers when showing details" patch as requested. This patch introduces and uses a new ply_boot_splash_uses_pixel_displays() function as suggested. While working on this I noticed that there were quite a few compiler warning when building plymouth on Fedora 28, wrote 2 patches fixing these, see bug 107086. Also a gentle reminder that I also have some details mode related patches pending in bug 107048. Created attachment 140444 [details] [review] [PATCH] main: Consistently only (de)activate renderers if the splash uses pixel-displays Hi, As discussed I've done a plymouth build for F29/rawhide including the 3 patches, using the 2nd version of the patch to conditionally activate the renderers. In hindsight that patch (as used in Fedora atm) is incomplete. I should have guarded all (de)activate)renderers calls with the same check. Both for consistency reasons and because not doing so actually causes problems (in some corner cases). Here is a patch to consistently have the guard everywhere. Regards, Hans Thanks, squashed the two together and pushed: To ssh://git.freedesktop.org/git/plymouth 778e0fb..447c783 master -> master |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.