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

Smoother session start #2124

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Smoother session start #2124

wants to merge 2 commits into from

Conversation

leolost2605
Copy link
Member

@leolost2605 leolost2605 commented Nov 24, 2024

For easier review do the individual commits (or rather 87b7321 only since the other one is just #2109 squashed)

Rebase merge if merged before #2109

Ok this is now mostly for wayland because on X the window was already shown when we know it's a panel so we can't really say wait for anything.

On wayland the way it now works is we wait for all clients that have requested being a panel to show and then immediately reveal them in sync. As a fail safe the WM lets the ShellClientsManager know when the stage is ready which will start a 5 second timer which will reveal all clients that are ready. After the panels have been revealed the first time we won't do anything like that again until the next session start so e.g. when a panel crashes and restarts it will show as soon as it's ready.

Unfortunately I can't make a recording since it's at session start but if you try it you should definitely see it :)

@leolost2605 leolost2605 requested a review from a team November 24, 2024 13:47
@vjr
Copy link
Member

vjr commented Nov 24, 2024

I tried this but don't see any difference. Does this work under Wayland?

I ran sudo gala --replace and got Error initializing: Unsupported session type also tried simply rebooting but don't see any difference though.

@leolost2605 leolost2605 force-pushed the leolost/smoother-start branch 2 times, most recently from 8ada0ac to 7360823 Compare November 24, 2024 16:51
@leolost2605
Copy link
Member Author

leolost2605 commented Nov 24, 2024

sudo gala --replace only works under X and not wayland. It should apply after logging out though.
How did you install it? Since AFAIK Gala doesn't build on current OS8

Does this work under Wayland?

It should work on wayland and X

@vjr
Copy link
Member

vjr commented Nov 25, 2024

I did the usual steps after cloning and checkout your branch:

sudo apt build-dep gala
meson build --prefix=/usr
cd build
ninja
sudo ninja install
sudo gala --replace (also did a relogin and also a full reboot)

Let me try this branch again with your latest changes.

@vjr
Copy link
Member

vjr commented Nov 25, 2024

I grabbed the latest, looks like there's unresolved merge conflicts checked in but I just resolved them locally.

Why do you say it won't build on OS8? I am running the daily and it seems to build without errors.

Also, Wayland doesn't seem to work but when I choose X session this works and looks good to me (not a code review) so I am in favour of this PR - hopefully it can be made to work for Wayland too.

@leolost2605
Copy link
Member Author

Why do you say it won't build on OS8? I am running the daily and it seems to build without errors.

Hmm for some reason I thought OS8 didn't have a recent enough vala, causing #2119 🤷

Wayland doesn't seem to work

Can you describe how it behaves? Do the panels just pop up without animation? Do they animate but not in sync? Something else?

@leolost2605 leolost2605 force-pushed the leolost/smoother-start branch from 7360823 to b27f228 Compare November 25, 2024 11:43
@leolost2605
Copy link
Member Author

I grabbed the latest, looks like there's unresolved merge conflicts checked in but I just resolved them locally.

Like on the latest commit?

@vjr
Copy link
Member

vjr commented Nov 25, 2024

Wayland doesn't seem to work

Can you describe how it behaves? Do the panels just pop up without animation? Do they animate but not in sync? Something else?

Yep I meant it behaves as usual - both panel and dock pop up without delay and without animation.

@vjr
Copy link
Member

vjr commented Nov 25, 2024

I grabbed the latest, looks like there's unresolved merge conflicts checked in but I just resolved them locally.

Like on the latest commit?

Yes, the latest commit at the time of my comment :-)

Are you not able to log in to Wayland session and observe yourself? I will only be able to come back to check this PR tomorrow though.

@leolost2605
Copy link
Member Author

Yes, the latest commit at the time of my comment :-)

Hmm that's strange there shouldn't have been any and I don't remember any.. (I take it you mean an unresolved conflict commited in and not that it wasn't up to date with main?)

Are you not able to log in to Wayland session and observe yourself? I will only be able to come back to check this PR tomorrow though.

I'm currently running it and just logged out and in again a few times but for me it works on wayland 🤷

@vjr
Copy link
Member

vjr commented Nov 26, 2024

I'm currently running it and just logged out and in again a few times but for me it works on wayland 🤷

Oh my bad - it is working for Wayland too - it's just that my monitor goes blank for I guess more than the 1.5 seconds when I login so I was not able to see the new effect. I hardcoded the delay to 5 seconds in my local checkout and yes I can see it working on Wayland too. Login to X session also monitor goes blank but for shorter interval so I was able to see the new effect.

@tintou
Copy link
Member

tintou commented Nov 26, 2024

Having an hard-coded empirical value that delays the startup is a no-go to me. Each application sends a "ready" signal to the session that we would have to acknowledge

@stsdc
Copy link
Member

stsdc commented Nov 26, 2024

What if my system has less RAM and components will load longer then 1,5s?

@leolost2605 leolost2605 force-pushed the leolost/smoother-start branch from b27f228 to 140829a Compare November 26, 2024 15:10
@leolost2605
Copy link
Member Author

leolost2605 commented Nov 26, 2024

What if my system has less RAM and components will load longer then 1,5s?

Each application sends a "ready" signal to the session that we would have to acknowledge

We are waiting for the clients to be ready anyways, the 1.5 was just because I saw somewhere a mockup with quite some delay and also to not stall the other if one of the clients is malfunctioning.
But fair enough I guess we can show immediately once clients are ready and introduce something like 5 seconds as backup to not stall if one client fails.

@leolost2605 leolost2605 force-pushed the leolost/smoother-start branch 2 times, most recently from 049a670 to 87b7321 Compare November 26, 2024 15:57
@leolost2605
Copy link
Member Author

leolost2605 commented Nov 26, 2024

Ok this is now mostly for wayland because on X the window was already shown when we know it's a panel so we can't really wait for anything.

On wayland the way it now works is we wait for all clients that have requested being a panel to show and then immediately reveal them in sync. As a fail safe the WM lets the ShellClientsManager know when the stage is ready which will start a 5 second timer which will reveal all clients that are ready. After the panels have been revealed the first time we won't do anything like that again until the next session start so e.g. when a panel crashes and restarts it will show as soon as it's ready.

@leolost2605 leolost2605 force-pushed the leolost/smoother-start branch from bf97b6f to f20fe7a Compare November 29, 2024 15:06
@leolost2605
Copy link
Member Author

Please don't merge main if it's intended to be rebase merged, instead rebase on main :)

@leolost2605 leolost2605 force-pushed the leolost/smoother-start branch from f20fe7a to 050fe55 Compare December 5, 2024 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants