Introduce a KWin internal on-screen-notification service
ClosedPublic

Authored by graesslin on Dec 18 2016, 11:05 AM.

Details

Summary

Recently we noticed that there are multiple areas where KWin needs to
inform the user about how to operate. Examples are:

  • Screenshot
  • ColorPicker
  • Pointer constraint enabled
  • Pointer constraint about to be removed
  • Kill Window

For Screenshot and ColorPicker we used an EffectFrame to render it. But
this is not an optimal solution as it's lacking many features we would
need. We cannot properly use it from within KWin core, we cannot
implement features like hide on mouse over, etc. etc.

This change introduces an OnScreenNotification which supports:

  • showing an icon
  • showing a message
  • timeout

It is Qml styled, so that it can be easily adjusted. This is a big
improvement over the EffectFrame solution. The Qml file creates a Plasma
Dialog of type OSD. Thus KWin places it like the normal OSD windows and
also looks kind of similar. In the case of KWin the focus is more on the
message, than an icon, so the icon is placed left of the text.

While the OnScreenNotification is supposed to be used like a singleton,
it doesn't use the KWin singleton pattern. Instead a small wrapper
namespace OSD is introduced which provides a convenient API for KWin
internal areas to show/hide the notification. By not using the KWin
singleton pattern, the OnScreenNotification does not depend on any other
parts of KWin and can be easily unit-tested.

A few features are still missing and will be added in further commits:

  • hide-out on mouse over
  • optional skip close animation (needed for screenshot)
  • X11 support (not that important as it's mostly for Wayland features)

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
graesslin updated this revision to Diff 9139.Dec 18 2016, 11:05 AM
graesslin retitled this revision from to Introduce a KWin internal on-screen-notification service.
graesslin updated this object.
graesslin edited the test plan for this revision. (Show Details)
graesslin added reviewers: KWin, Plasma.
Restricted Application added a project: KWin. · View Herald TranscriptDec 18 2016, 11:05 AM
Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript

looks generally good.

1 nitpick about QML icon usage, and one question.

onscreennotification.h
48

Not an actual issue, but technically, this isn't a new virtual, but an override

79

why QPointer?

If this were to get deleted externally, you're still left with a QQmlContext with a dangly pointer internally and a crash if it's used.

osd.cpp
74

I don't get why this line exists, you can render an internal window on either platform..

but if it should exist, why it is not put it for show too?

qml/onscreennotification/plasma/main.qml
39

If you're rendering onto a Plasma Dialog, it should be a Plasma Icon

otherwise you have a potentially white icon on white background situation.

graesslin added inline comments.Dec 19 2016, 4:51 PM
osd.cpp
74

I don't get why this line exists, you can render an internal window on either platform..

Hah, if it were that simple... KWin cannot manage it's own windows on X11, it has to be an override redirect. Thus we need to position manually, the placing through OSD type won't work. That's the obvious part, but there is certainly more. Which means I need to test on X11 what works and what doesn't and adjust. I don't like to have a special handling here at all...

but if it should exist, why it is not put it for show too?

And it is there :-)

graesslin marked 3 inline comments as done.Dec 21 2016, 6:42 PM
graesslin added inline comments.
onscreennotification.h
48

we need to update the template in kdevelop ;-) That was generated code

graesslin updated this revision to Diff 9266.Dec 21 2016, 6:43 PM
graesslin marked an inline comment as done.
  • Changed the qml to use PlasmaCore.IconItem
  • remove useless QPointer usage
  • override instead of virtual for dtor
Restricted Application edited projects, added Plasma; removed KWin. · View Herald TranscriptDec 21 2016, 6:43 PM
graesslin updated this revision to Diff 9267.Dec 21 2016, 6:44 PM

Remove KQuickControlsAddons include

Restricted Application edited projects, added KWin; removed Plasma. · View Herald TranscriptDec 21 2016, 6:44 PM
davidedmundson accepted this revision.Dec 21 2016, 6:52 PM
davidedmundson added a reviewer: davidedmundson.
This revision is now accepted and ready to land.Dec 21 2016, 6:52 PM
This revision was automatically updated to reflect the committed changes.