Differential D26395

[XembedSNIProxy] Hide container window when something shows it
ClosedPublic
Actions

Authored by kmaterka on Jan 3 2020, 2:16 PM.

Details

Reviewers
davidedmundson
Group Reviewers
Plasma: Workspaces
Plasma
Commits
R120:69786a1ff947: [XembedSNIProxy] Hide container window when something shows it
Summary

For each tray icon XEmbedSNIProxy is creating container window 32x32 in size. It is black with opaque set to 0 (fully transparent when compositor is enabled). All of these container windows are stacked below all windows, so normally you can't see them. On creation all container windows are created in top-left corner. When user clicks on the tray icon, container window is moved to the click location (to handle events correctly).
On KWin restart all windows are shuffled, usually KWin is able to restore ordering correctly, but for some reason not it this case. As a result black/transparent container windows are stacked above all other windows and panels.
To solve that, when container window is visible, XembedSNIProxy needs hide it again by stacking the container window below again.

BUG: 357443
FIXED-IN: 5.18.0

Test Plan
  1. Run any application with XEmbed system tray icon, do not click on the icon
  2. Restart KWin
  3. [Optional] Disable compositor - with disable container window is black and easier to spot
  4. Expected:

a) before fix: black/transparent rectangle in the top-left corner, reacts to mouse click
b) after fix: no rectangle, mouse clicks work as expected.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kmaterka requested review of this revision.Jan 3 2020, 2:16 PM
kmaterka created this revision.
kmaterka added a comment.Jan 3 2020, 2:27 PM

This is another approach to fix 357443. It is still a workaround, but more reliable and uses better concept.

xembed-sni-proxy/fdoselectionmanager.cpp
167

XCB_VISIBILITY_UNOBSCURED is rare. During simulated click (when we stack window above) xembedsniproxy receives XCB_VISIBILITY_PARTIALLY_OBSCURED. Obviously we ignore the last possible case: XCB_VISIBILITY_FULLY_OBSCURED.

168

Iteration sucks, but this is the simplest (a in most cases the quickest) method. In this event we receive container window id, we can get its child and get sniproxy using it:

xcb_connection_t *c = QX11Info::connection();
xcb_query_tree_cookie_t cookie = xcb_query_tree(c, event->window);
QScopedPointer<xcb_query_tree_reply_t, QScopedPointerPodDeleter>
    reply(xcb_query_tree_reply(c, cookie, nullptr));

if (reply && xcb_query_tree_children_length(reply.data()) == 1) {
    xcb_window_t *children = xcb_query_tree_children(reply.data());
    const auto sniProxy = m_proxies.value(children[0]);
    if (sniProxy) {
        sniProxy->handleExposeEvent(event->window);
    }
}

IMO this is slower.

The best would be to have second map like m_proxies, but with container windows id (or something similar in the idea).

xembed-sni-proxy/sniproxy.cpp
527

During click, container window is showed for a brief moment (STACK_ABOVE), so it is possible that it will sent visibility event. This is probably not needed, I tested it and all events generated in this method (stack above, click, stack below) were processed before visibility event is received in XEmbedSNIProxy. I added this just in case - I'm not sure how X events are queued and how reliable the order is.

kmaterka retitled this revision from [XembedSNIProxy] Send all container windows to background on KWin restart to [XembedSNIProxy] Hide container window when something shows it.Jan 3 2020, 2:33 PM
kmaterka edited the summary of this revision. (Show Details)
kmaterka edited the summary of this revision. (Show Details)Jan 8 2020, 3:37 PM
davidedmundson accepted this revision.Jan 8 2020, 3:42 PM
This revision is now accepted and ready to land.Jan 8 2020, 3:42 PM
kmaterka added a comment.Jan 8 2020, 3:49 PM

What should be in FIXED-IN? AFAIK 5.18 is frozen. Should I commit it into master?

kmaterka edited the summary of this revision. (Show Details)Jan 8 2020, 4:05 PM
Closed by commit R120:69786a1ff947: [XembedSNIProxy] Hide container window when something shows it (authored by kmaterka). · Explain WhyJan 8 2020, 4:06 PM
This revision was automatically updated to reflect the committed changes.
kmaterka marked 3 inline comments as done.
davidedmundson added a comment.Jan 8 2020, 4:08 PM

What should be in FIXED-IN? AFAIK 5.18 is frozen. Should I commit it into master?

5.18 doesn't exist yet, let alone being frozen

kmaterka added a comment.Jan 8 2020, 4:29 PM
In D26395#590409, @davidedmundson wrote:

What should be in FIXED-IN? AFAIK 5.18 is frozen. Should I commit it into master?

5.18 doesn't exist yet, let alone being frozen

I guess I misunderstood "5.18 repo freeze was a while ago". Pushed.

Revision Contents
Changeset List

Diff 73072

View Options

xembed-sni-proxy/fdoselectionmanager.cpp

Loading...
View Options

xembed-sni-proxy/sniproxy.h

Loading...
View Options

xembed-sni-proxy/sniproxy.cpp

Loading...