Review Request 125871: WIP: task geometries to wayland for minimize effect

Fri Oct 30 13:35:55 UTC 2015

> On Oct. 29, 2015, 8:32 p.m., Thomas Lübking wrote:
> > What's the point of client relative geometries?
> > 
> > Afaics, there'll be two "problematic" scenarios.
> > 
> > 1) >= 2 taskbars on >= 1 screens, the window is minimized from the taskbar
> > 2) >= 2 taskbars on >= 2 screens, the window is NOT minimized from the taskbar (but the deco button, or wmctrl etc.)
> > 
> > (1) can be _easily_ resolved by the minimizing taskbar applet updating the (one is actually sufficient here) geometry right before minimizing the client
> > 
> > The challenge of (2) is to select a taskbar where to minimize to itfp (I guess one does not want to minimize to two regions at once, seems confusing)
> > The reasonable approach seems to minimize to any icon rect that is on the same screen the window is on (WMs free choice), so we need a list of global (or tagged screen relative) geometries where the WM can look for eg. the closes match on the current ("active") or the windows screen.
> > 
> > The only benefit of a window relative geometry would be (less overhead on) tracking a moving client while the window minimizes - but the only scenario I see here is an autohiding panel and then the only overhead (if one wants to buy into such feature at all) would be an update of the icon rect from the on-screen to the off-screen position of the panel.
> > 
> > Otoh, keeping a reference to a surface around
> > a) requires a surface itfp
> > b) sounds *extremely* bug prone, eg.:
> >    - I don't see where the surface removal is tracked in the patch, so we could end up with a dangeling pointer.
> >    - Another case is where the taskbar is moved from eg. a panel to the desktop - we'd have it in the desktop and -outdated- the panel at the same time)
> >    - Also what if I add a taskbar to the top and the bottom of my desktop? That's not supported, is it?
> > 
> > => Imo we *need* one icon rect per screen.
> > It would be cool to have one "per taskbar" (calc closest distance on unrelated minimizations)
> > But I object the approach to assign the rects to particular surfaces.
> 
> Martin Gräßlin wrote:
>     > It would be cool to have one "per taskbar" (calc closest distance on unrelated minimizations)
>     
>     this is kind of the idea here. Marco and I had discussed the needs and came up with "each panel needs to set the geometry". So that KWin knows all the possible geometries (we cannot use it yet, but well that's a different problem). That's what the patch does: Plasma can set for different windows (WindowManagement) an icon geometry relative to the a PlasmaSurface. My thought was doing it the other way around, but that sounds to be details.
> 
> Thomas Lübking wrote:
>     But surface != taskbar, is it?
>     A systematic problem is the cooperative approach to keep the list clean (ie. a buggy or crashing client shall not leave false rects behind forever, kwin cannot be restarted to clear the list)
>     
>     Assigning rects to windows/surfaces should be capable of catching crashy, but not buggy clients - nor can it cover multiple taskbars per surface.
>     Also I'd avoid resolving cached client pointers here (or you'd have to look them up)
>     
>     - I guess we won't get around resolving the surface (since it doesn't/isn't supposed to know its position on screen?)
>     - I'm not sure whether "one taskbar per surface" is a reasonable restriction (my opinion still is "zero taskbars per nowhere" ;-)
>     - The patch definitively lacks cleanup functionality, ie. at least a cooperative solution to withdraw the rect (taskbar -or entry- removed from panel) and hash maintainace ("is the surface pointer still valid at all"?)
>     - If the panel <=> taskbar assumption isn't sufficient (additional systray entry case?) we'll require further mapping (UID on top of surface) and maintainance mechanisms (to not end up with 100 rects on the same surface)

I can see the problem of having leftover geometries due to clients misbehaving.
in the latest version at least removes those for panels that go away.
An alternative may be instead the hash plasmawindow/geometry in plasmasurfaceinterface instead, on kwayland side it may be slightly more elegant.
however i don't know how to make it work correctly from kwin, it shuld iterate over all the existing plasmashellsurfaceinterfaces, that doesn't seem much pretty

- Marco

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

On Oct. 30, 2015, 1:24 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125871/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2015, 1:24 p.m.)
> 
> 
> Review request for kwin and Plasma.
> 
> 
> Repository: kwayland
> 
> 
> Description
> -------
> 
> this exposes the geometry of taskbar entries in plasma-windowmanagement, in order to make the minimize effects possible.
> unlike on X11, it takes relative positions and it has one geometry per panel, making possible to have multiple taskbars working.
> 
> however this is still not completely exposed, as internally kwin has still only one geometry, it will need a change in kwin itself (suggestions welcome) probably the rotocol will need also a minimizeTo function that takes the panel as argument.
> 
> another thing completely missing is tests: unfortunately the whole plasma-windowmanagement doesn't have any autotest yet :/
> 
> 
> Diffs
> -----
> 
>   autotests/client/CMakeLists.txt 1556c47 
>   autotests/client/test_wayland_windowmanagement.cpp PRE-CREATION 
>   src/client/plasmawindowmanagement.h abd8fa6 
>   src/client/plasmawindowmanagement.cpp 1f9674c 
>   src/client/plasmawindowmodel.h 5a6aac4 
>   src/client/plasmawindowmodel.cpp 355ef53 
>   src/client/protocols/plasma-window-management.xml ca6a7cc 
>   src/server/plasmashell_interface.h 9db3f52 
>   src/server/plasmashell_interface.cpp d29d7bc 
>   src/server/plasmawindowmanagement_interface.h 6ccc22e 
>   src/server/plasmawindowmanagement_interface.cpp ad714a5 
> 
> Diff: https://git.reviewboard.kde.org/r/125871/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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