Create Task
Maniphest T8771

PlasmaQuick::Dialog and/or KWin (internal window) problems
Closed, ResolvedPublic
Actions

Description

There seem to be several problems in the PlasmaQuick::Dialog as well as in KWin's internal window code:

Large scale / potentially difficult to solve

  • QWindow geometry is set through KWin for internal windows in ShellClient::syncGeometryToInternalWindow -> might overwrite QML property binding. That is probably happening in bug 386304. Fix: D13084
  • The geometry of a PlasmaQuick::Dialog based KWin internal window is monitored directly through Qt API, but also is influenced by changes through PlasmaShellSurface::setPosition -> ambiguity might induce concurrency problems.

Small scale / probably easy to solve

  • PlasmaShellSurface is set before being sent from Wayland Server in PlasmaQuick::Dialog. Fix: D13037
  • DialogShadows sets shadow through org_kde_kwin_shadow protocol multiple times on single invocation.

Details

Differential Revisions
D13084: Avoid invalid geometry of internal clients through plasma surface interface
D13037: [Dialog] Set PlasmaShellSurface on PlasmaShell being announced

Related Objects

romangg created this task.May 22 2018, 10:33 AM
romangg triaged this task as High priority.
romangg edited projects, added KWin; removed Plasma.May 22 2018, 10:57 AM
romangg updated the task description. (Show Details)May 22 2018, 11:48 AM
mart added a subscriber: mart.May 22 2018, 11:57 AM

perhaps dialog needs a mode where it's not trying to do anything with plasmashellsurface?
(maybe if the positioning would be ported to xdg shell popups that part could go away)

davidedmundson added a subscriber: davidedmundson.May 22 2018, 12:10 PM

might overwrite QML property binding. That is probably happening in bug 386304.

That will overwrite what the property binding is setting it to, it shouldn't the QML bindings though.
I don't think that's a problem.

The geometry of a PlasmaQuick::Dialog based KWin internal window is monitored directly through Qt API, but also is influenced by changes through PlasmaShellSurface::setPosition -> ambiguity might induce concurrency problems.

Yeah, it's pretty silly. two ways of sending the same info is just asking for trouble.
Pretty easy to avoid if we add kwin specific casing into Dialog.

romangg added a comment.May 22 2018, 3:10 PM
In T8771#143386, @davidedmundson wrote:

might overwrite QML property binding. That is probably happening in bug 386304.

That will overwrite what the property binding is setting it to, it shouldn't the QML bindings though.
I don't think that's a problem.

Check out what happens with some debug statements: First the QML binding has some wrong value and this triggers a delayed call to setGeometry (maybe the delay in R108:e0f95fd913b6e7c9106ca153c0dd1e3d46c9e297 is also problematic here). While later on the QML code binds the correct value and sets the current value correctly, the current value later gets overwritten again with KWin's call to setGeometry.

romangg added a comment.May 24 2018, 8:21 AM

I uploaded a patch for the initial bug report 386304, but it's very specific to the problem at hand. The internal window code in general is rather unclean, which will lead to problems in the future.

romangg claimed this task.Sep 3 2018, 11:29 AM
romangg closed this task as Resolved.

The specific issue with TabBox has a temporary work around now with 1fb2eace3fbf for the 5.14 release. But the general problems need to be solved on a broader scope. For that continue in T9600.

romangg moved this task from Backlog to Done on the KWin board.Sep 3 2018, 11:48 AM