Review Request 120329: PlasmaShell and PlasmaSurface interfaces

[Pier Luigi Fiorini]

> On Sept. 23, 2014, 5:51 a.m., Martin Gräßlin wrote:
> > src/client/plasma_shell.h, line 162
> > <https://git.reviewboard.kde.org/r/120329/diff/1/?file=314685#file314685line162>
> >
> >     why is this in PlasmaShell? Shouldn't it be in the PlasmaSurface?
> 
> Pier Luigi Fiorini wrote:
>     This was meant to be used for any surface should we need to move it to arbitrary global coords.
> 
> Martin Gräßlin wrote:
>     So this is for any wl_surface from Plasma? I don't think that this is a good solution as it will expose quite some heavy work on the compositor side. I think it's better if Plasma has to create a dedicated PlasmaShellSurface for each window it wants to position.

The compositor shouldn't be subject to heavy work, it's just a surface that needs to be moved, but I'm speaking with Green Island in mind i don't know other compositors.
That said I don't mind removing this method at all, but I suspect the org_kde_plasma_surface.set_position request will need global coordinates.
Can anyone confirm that?


> On Sept. 23, 2014, 5:51 a.m., Martin Gräßlin wrote:
> > src/client/plasma_shell.h, lines 192-210
> > <https://git.reviewboard.kde.org/r/120329/diff/1/?file=314685#file314685line192>
> >
> >     why are these wl_surface and not PlasmaShellSurface/

For the same reason above, but Plasma will probably need to set the scvreen edge only for panels or surfaces that it knows about.
Did I get it right?
In that case these methods should be moved to PlasmaSurface as setGlobalPosition.


> On Sept. 23, 2014, 5:51 a.m., Martin Gräßlin wrote:
> > src/client/plasma_surface.h, line 125
> > <https://git.reviewboard.kde.org/r/120329/diff/1/?file=314687#file314687line125>
> >
> >     I'm not sure whether we need this. Plasma is using kscreen and not QScreen.
> 
> Pier Luigi Fiorini wrote:
>     There are other components to be ported that doesn't use KScreen, for instance ksplashqml.
>     Besides without QScreen how can we know the wl_output?
>     Also, using the QScreen or Wayland backends on Plasma should allow us to map KScreen::Output to QScreen (if I recall plasmashell already do that).
> 
> Martin Gräßlin wrote:
>     > There are other components to be ported that doesn't use KScreen, for instance ksplashqml.
>     
>     I don't think ksplashqml will use PlasmaSurface (why should it?). It's a perfect use case for the fullscreen shell protocol.
> 
> Pier Luigi Fiorini wrote:
>     For the surface role, there's no need for the fullscreen shell protocol when a session compositor just need to know its role to map it above any other window.
>     Plus ksplashqml is already using PlasmaSurface on my branch :)
> 
> Martin Gräßlin wrote:
>     my thought was going in the direction of:
>     * ksplashqml uses fullscreen shell protocol with "system" compositor
>     * kwin has time to startup and takes over the fullscreen shell once everything is setup
>     
>     if ksplashqml connects to KWin, the startup of KWin becomes important while otherwise it would just not matter. We want flicker free startup and using the "system" compositor's fullscreen shell might be the solution to it.

I'm afraid the kwin takeover will result in the splash suddenly disappearing without transition (ie fade-out), a session compositor instead maps the splash leaving the other layers below and can do a smooth transition when the surface is unmapped. There's also the desktop_ready request that a session compositor can use should it need even more control.
With fullscreen-shell a wl_output is still needed and the reason for this method was to take a wl_output from a QScreen which is the same problem we face for wl_surface.


- Pier Luigi


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120329/#review67255
-----------------------------------------------------------


On Sept. 23, 2014, 5:39 a.m., Pier Luigi Fiorini wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120329/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2014, 5:39 a.m.)
> 
> 
> Review request for Plasma and Martin Gräßlin.
> 
> 
> Repository: kwayland
> 
> 
> Description
> -------
> 
> PlasmaShell and PlasmaSurface interfaces
> 
> 
> Diffs
> -----
> 
>   autotests/client/test_wayland_registry.cpp 54aa9a560153d00924d4e73c75f029ed1d1ad788 
>   src/client/CMakeLists.txt e00f4573ad22efc9b5776b5ef900854c04f8afd6 
>   src/client/plasma_shell.h PRE-CREATION 
>   src/client/plasma_shell.cpp PRE-CREATION 
>   src/client/plasma_surface.h PRE-CREATION 
>   src/client/plasma_surface.cpp PRE-CREATION 
>   src/client/registry.h 103be0aec9cae6d76c62fd32481eaaafa5a161f0 
>   src/client/registry.cpp 17d738415e395fb638751ac6429d1fc0e3ededd9 
> 
> Diff: https://git.reviewboard.kde.org/r/120329/diff/
> 
> 
> Testing
> -------
> 
> Work in progress Plasma port to Wayland.
> 
> 
> Thanks,
> 
> Pier Luigi Fiorini
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140924/6def4d81/attachment-0001.html>