Support passing generic QIcon through PlasmaWindow interface

Authored by graesslin on Oct 13 2016, 11:57 AM.



Especially for Xwayland windows the compositor might not have a themed
icon name. Resulting in a task manager not having dedicated icons for
Xwayland windows.

This change deprecates the way how a compositor is supposed to set the
window icon. Instead of passing the themed icon name, it is now supposed to
pass the QIcon. In case it's a themed icon the existing way to pass to
the client is used.

Otherwise a new event is used to inform the client that there is an icon

  • no data is transmitted at this point. The client can then create a

file descriptor and pass it to the compositor. The compositor serializes
the icon into the file descriptor and the client can read from it. This
all happens transparently on client side there is no api change at all.

The writing and reading of the icon is done in a thread. Due to that
Qt5::Concurrent is now a required dependency instead of an optional

Diff Detail

R127 KWayland
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
graesslin updated this revision to Diff 7376.Oct 13 2016, 11:57 AM
graesslin retitled this revision from to Support passing generic QIcon through PlasmaWindow interface.
graesslin updated this object.
graesslin edited the test plan for this revision. (Show Details)
graesslin added reviewers: Plasma on Wayland, hein.
Restricted Application added a project: Plasma on Wayland. · View Herald TranscriptOct 13 2016, 11:57 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
roberts added inline comments.

EAGAIN comes from reads of O_NONBLOCK fd's, but this seems to be a blocking pipe (no calls to fcntl() or pipe2()).

As such the read call could block the thread for significant or unbounded time - depending on whether QT_READ swallows EINTR - if the other side of the pipe failed to close the fd.


pipeFd could leak here.


Is there anything protecting this from being unsynchronised parallel access to m_icon (and p)?

Simplest solution might be passing the icon by value into the lambda.

graesslin added inline comments.Oct 14 2016, 8:36 AM

I assumed that reading from the icon is thread save. But you are right: better save than sorry. Especially accessing the p pointer could be dangerous as the PlasmaWindowInterface instance might get deleted.

graesslin marked 3 inline comments as done.Oct 14 2016, 8:42 AM
graesslin updated this revision to Diff 7384.Oct 14 2016, 8:42 AM
  • Pass icon as argument to the runnable
  • use pipe2
  • close pipefd in error handling branch
roberts accepted this revision.Oct 14 2016, 6:12 PM
roberts added a reviewer: roberts.

Looks good to me now.

This revision is now accepted and ready to land.Oct 14 2016, 6:12 PM
apol added a subscriber: apol.Oct 14 2016, 8:56 PM
apol added inline comments.

maybe worth adding a if (m_icon == icon) return;

graesslin added inline comments.Oct 15 2016, 12:09 PM

in theory yes, in practice: operator== doesn't exist for QIcon so all you get is a compile error :-(

romangg accepted this revision.Oct 16 2016, 10:57 AM
romangg added a reviewer: romangg.
romangg added a subscriber: romangg.

Told you, I had problems compiling at first. Works now for unknown reason and icons are displayed together with D3050.


What about comparing QIcon::cacheKey() values?

This revision was automatically updated to reflect the committed changes.