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

Greeter on Wayland - rebased #1393

Merged
merged 2 commits into from Jun 9, 2021
Merged

Greeter on Wayland - rebased #1393

merged 2 commits into from Jun 9, 2021

Conversation

aleixpol
Copy link
Contributor

Rebased version of #1379

Had to be creative in some places since it changed quite a bit.

src/CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Member

@plfiorini plfiorini left a comment

Choose a reason for hiding this comment

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

the branch contains commits that we don't want anymore.
i would have applied the patch on top of the new develop instead of rebasing it.

status == Auth::HELPER_OTHER_ERROR)
stop();
else
VirtualTerminal::jumpToVt(m_terminalId, true);
Copy link
Member

Choose a reason for hiding this comment

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

we decided not to stop the greeter

src/greeter/GreeterApp.cpp Outdated Show resolved Hide resolved
src/helper/UserSession.cpp Show resolved Hide resolved
@@ -224,6 +248,7 @@ namespace SDDM {
qCritical() << "setgid(" << pw.pw_gid << ") failed for user: " << username;
exit(Auth::HELPER_OTHER_ERROR);
}
qputenv("XDG_RUNTIME_DIR", QByteArrayLiteral("/run/user/") + QByteArray::number(pw.pw_uid));
Copy link
Member

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For WaylandSocketWatcher to work.

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks not great. Either something like pam_systemd creates it and it can be used or sddm has to create its own and use that path. Just guessing that it will be /run/user/<uid> smells like a hack.

Copy link
Member

Choose a reason for hiding this comment

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

pam_systemd does create it

Copy link
Contributor

Choose a reason for hiding this comment

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

Exactly, but then it should only be used in the context that $XDG_RUNTIME_DIR is actually set in

Copy link
Contributor

Choose a reason for hiding this comment

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

In FreeBSD for instance (or with ConsoleKit in general, not sure), XDG_RUNTIME_DIR is actually set up by the session scripts and somewhere below /tmp IIRC.

Copy link

Choose a reason for hiding this comment

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

On FreeBSD both consolekit2 (upstream + downstream) and pam_xdg (downstream) use /var/run/user/<uid>, hardcoded at build-time.

Choose a reason for hiding this comment

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

Right. If something else (e.g. pam_systemd) created XDG_RUNTIME_DIR, then you must READ the environment variable XDG_RUNTIME_DIR to find out its value. Or maybe there is a d-bus interface to ask for it if this is not your current session, I don't know. It would be incorrect to guess what the value might be and set XDG_RUNTIME_DIR to that guessed value.

process, [](int exitCode, QProcess::ExitStatus exitStatus) {
qDebug() << "wayland compositor finished" << exitCode << exitStatus;
if (exitCode != 0 || exitStatus != QProcess::NormalExit)
QCoreApplication::instance()->quit();
Copy link
Member

Choose a reason for hiding this comment

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

this will quit the helper, we don't want this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code is copied from XOrgUserHelper::startProcess.

I guess we'll need to include a start failed signal.

Ultimately we'll need quitting anyway. If the compositor (wayland or xorg) can't be started.

void WaylandHelper::switchVt()
{
int vtNumber = m_environment.value(QStringLiteral("XDG_VTNR")).toInt();
VirtualTerminal::jumpToVt(vtNumber, true);
Copy link
Member

Choose a reason for hiding this comment

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

i think UserSession already do that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to bring back the WaylandSocketWatcher because we need to wait for the compositor to start before we can start the sddm-greeter.

An alternative implementation would be to have the sddm-greeter block at start until the compositor appears.

@aleixpol
Copy link
Contributor Author

aleixpol commented May 4, 2021

Ping?

1 similar comment
@aleixpol
Copy link
Contributor Author

Ping?

@Mazino-Urek
Copy link
Contributor

It would have really great if this feature landed before Plasma 5.23.

plfiorini and others added 2 commits June 8, 2021 08:44
Introduce a new display server option to run the greeter with.
In the future this will be the default display server.

This patch contains the most important parts to get the greeter running
on Wayland, but it's not possible to select a keyboard layout.

At the moment the Wayland socket is assumed to be "wayland-0" but in a
multi-seat configuration we would have more than one greeter running,
each with a diffent socket and the assumption won't work anymore.

In the future we will figure out how to do, after all at the moment this
is considered enough for most people.

Closes: #440
Includes a GreeterEnvironment configuration entry.
@plfiorini
Copy link
Member

Added a couple of environment variables for fullscreen-shell and disable window decoration, then rebased on top of the latest develop. I will give it one more last test later this evening and merge if it works fine.

@plfiorini plfiorini added this to the 0.20 milestone Jun 8, 2021
@plfiorini plfiorini merged commit 56ad91e into develop Jun 9, 2021
@plfiorini plfiorini deleted the wayland-rebased branch June 9, 2021 07:17
@aleixpol
Copy link
Contributor Author

aleixpol commented Jun 9, 2021

Thanks!

@isopix
Copy link

isopix commented Jun 22, 2021

Waylanded SDDM really sucks on mulit-monitor setups, because it’s not cloned across displays, and menu items appeared on internal(closed laptop lid) display, than I’ve been clicking (so it was impossible to change Session for me). It uses Weston by default, and from my research Weston doesn’t support clone-mode-by-default (ass application argument).
Maybe other Wayland compositor(than weston) could do that?

@aleixpol
Copy link
Contributor Author

Maybe other Wayland compositor(than weston) could do that?

Yes, that's what the CompositorCommand setting is for.

@isopix
Copy link

isopix commented Jun 22, 2021 via email

@ppaalanen
Copy link

Was there a discussion somewhere on how you ended up choosing to use Weston's fullscreen-shell protocol extension?

I see that you are planning to run on more than Weston, which I think is a good idea, and I suppose it also means the greeter works on the xdg-shell family of protocol extensions, not just fullscreen-shell.

As for the clone mode, you could have the greeter set up similar windows on all outputs. That does not need clone mode support from Weston, and it will fit diverse monitors much much better than a clone mode because a) resolution aware text rendering instead of raster scaling, and b) you can fit the UI layout to the available screen estate on each monitor.

At Weston upstream, we had kind of abandoned the fullscreen-shell, but now that it actually has a use case, I wonder what to do. Maybe we should open a Weston issue about the future of the fullscreen-shell?

Weston has also kiosk shell plugin which seems more promising going forward and exposes the xdg-shell family of protocol extensions.

@plfiorini
Copy link
Member

Was there a discussion somewhere on how you ended up choosing to use Weston's fullscreen-shell protocol extension?

No, I just wanted something that didn't involve a panel but it's temporary.

I see that you are planning to run on more than Weston, which I think is a good idea, and I suppose it also means the greeter works on the xdg-shell family of protocol extensions, not just fullscreen-shell.

The greeter should work very well with xdg-shell.

As for the clone mode, you could have the greeter set up similar windows on all outputs. That does not need clone mode support from Weston, and it will fit diverse monitors much much better than a clone mode because a) resolution aware text rendering instead of raster scaling, and b) you can fit the UI layout to the available screen estate on each monitor.

I need to investigate what's going on here: SDDM creates a window for each screen that is reported by Qt therefore there is no clone mode here as opposed to X11. This should actually be very good for the use case of @isopix but my guess is that Weston still reports the laptop output even though the lid is closed and that's why the greeter goes into that output.

At Weston upstream, we had kind of abandoned the fullscreen-shell, but now that it actually has a use case, I wonder what to do. Maybe we should open a Weston issue about the future of the fullscreen-shell?

Weston has also kiosk shell plugin which seems more promising going forward and exposes the xdg-shell family of protocol extensions.

At the time there was no kiosk shell, so the fullscreen-shell was the best way to get something without a panel.
I'm on Fedora 33 with Weston 8.0.0 and this shell is not available.
Using the kiosk shell plugin is the best going forward and I'm considering to switch to it now, before the SDDM release.

@davidedmundson
Copy link
Member

The greeter should work very well with xdg-shell.

Released Qt presents a challenge QWindow::setFullscreen() on XDG calls xdg_toplevel.set_fullscreen(null). So on a multi-screen setup you don't know where the window will end up.

Qt's fullscreen-shell did forward the screen.

XDGShell implementation is changed in Qt6 and in KDE's Qt backports.

@plfiorini
Copy link
Member

The greeter should work very well with xdg-shell.

Released Qt presents a challenge QWindow::setFullscreen() on XDG calls xdg_toplevel.set_fullscreen(null). So on a multi-screen setup you don't know where the window will end up.

Oh I forgot about that

Qt's fullscreen-shell did forward the screen.

Yeah I remember I did get that right when I wrote the shell integration plugin :)

XDGShell implementation is changed in Qt6 and in KDE's Qt backports.

Do you know if distros are using the KDE's Qt backports?

@ppaalanen
Copy link

Thanks!

I would assume that Weston does not handle the laptop lid switch at all, so probably the internal panel does not even turn off, unless it also appears disconnected from KMS perspective.

@darkbasic
Copy link

Do you know if distros are using the KDE's Qt backports?

I think Arch does: https://archlinux.org/packages/extra/x86_64/qt5-base/

@isopix
Copy link

isopix commented Jun 29, 2021 via email

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.

None yet

10 participants