Bug 395262 - Transparency/blur is corrupted on context menu of desktop files
Summary: Transparency/blur is corrupted on context menu of desktop files
Status: RESOLVED DUPLICATE of bug 399680
Alias: None
Product: plasmashell
Classification: Plasma
Component: Desktop Containment (show other bugs)
Version: 5.13.0
Platform: Neon Linux
: NOR minor
Target Milestone: 1.0
Assignee: Alex Nemeth
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-06-12 01:46 UTC by TYY331
Modified: 2019-07-14 11:12 UTC (History)
14 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Render corruption (205.84 KB, image/png)
2018-06-12 01:46 UTC, TYY331
Details
desktop icon menu (135.09 KB, image/png)
2018-07-17 18:28 UTC, Alex Nemeth
Details
video of half-working (2.74 MB, video/webm)
2018-07-17 19:38 UTC, Alex Nemeth
Details
screencast recorded on neon dev unstable (1.21 MB, video/webm)
2018-09-11 12:11 UTC, Patrick Silva
Details

Note You need to log in before you can comment on or make changes to this bug.
Description TYY331 2018-06-12 01:46:48 UTC
Created attachment 113220 [details]
Render corruption

Using breeze widget style with transparency/blur enabled produces strange colors in the context menu of desktop files, contextual menu of the desktop (if you have it enabled) renders correctly, panel's widgets that use the effect and contextual menus inside applications like dolphin all render correctly.
Steps to reproduce:
Set the desktop layout to enable files on desktop
Create a file in the desktop
right click on it to show the menu.
Comment 1 Nate Graham 2018-06-12 13:10:36 UTC
Alex, would you mind having a look here?
Comment 2 Alex Nemeth 2018-06-12 17:57:11 UTC
I can confirm it.
Looking into it...
Comment 3 Alex Nemeth 2018-06-14 19:37:17 UTC
So after investigating I believe this visual bug happens when a QMenu is called with popup() instead of exec()
Maybe this is a Qt bug?

In this file if I change menu->popup() to menu->exec() then there is no visual bug, but after I close the menu plasmashell crashes and I don't know how to prevent it from crashing.
https://github.com/KDE/plasma-desktop/blob/master/containments/desktop/plugins/folder/foldermodel.cpp#L1776

But then changing every instance of popup() to exec() would be a big task.

Can anyone with more experience please help me with this?
Comment 4 Michael D 2018-06-20 07:37:02 UTC
This bug also affects Dolphin's drag and drop context menu. If you drag and drop an item, the context menu that pops up asking to move, copy, etc. exhibits the blur bug. I'm on Manjaro testing, plasma 5.13.1.
Comment 5 Patrick Silva 2018-06-21 20:08:45 UTC
I can confirm the problem on Arch Linux running plasma 5.13.1 under X11.
Blur effect works correctly under Wayland.
Comment 6 Marek M 2018-06-22 07:11:07 UTC
At second screen works very well. But after use many days without restart, blur effect start work also at first screen.
Comment 7 David Edmundson 2018-06-22 10:42:29 UTC
>But then changing every instance of popup() to exec() would be a big task.

This isn't an option for plasmashell.

But there shouldn't be a difference from a kwin side - so we need to figure out what's different from an X POV when the different method is called.

Historically there was a difference with when QWidget::polish was called, which affects when QStyle code is triggered, we used to have an issue with oxygen's rounded corners.
Comment 8 Alex Nemeth 2018-06-22 12:12:51 UTC
(In reply to David Edmundson from comment #7)
> But there shouldn't be a difference from a kwin side - so we need to figure
> out what's different from an X POV when the different method is called.

Kvantum and QtCurve themes that support transparent context menus work without any problem, so this must be a Breeze issue then. I looked at their source code but didn't find anything regarding handling these two types differently.

In breezestyle.cpp the correct code runs, regardless of popup() or exec() call, although for popup() it gets called 3 or 4 times(??).

I'll investigate this further...
Comment 9 Matej Mrenica 2018-07-06 08:14:57 UTC
This also happens on ringt-click-menu in vlc.
Comment 10 Patrick Silva 2018-07-16 20:50:42 UTC
*** Bug 396540 has been marked as a duplicate of this bug. ***
Comment 11 David Edmundson 2018-07-16 22:53:25 UTC
@Alex please see https://phabricator.kde.org/D14174 and let me know if we need this in other places

I don't know if it makes sense to fix Qt. QMenu is a horrible piece of code to get changes into as it has so many platform specific behavioural things.
Comment 12 David Edmundson 2018-07-17 09:34:52 UTC
Git commit 42a74c9a79baafb222513f3ffad35b736b708454 by David Edmundson.
Committed on 17/07/2018 at 09:34.
Pushed by davidedmundson into branch 'master'.

Fix blur behind folderview context menus

Summary:
Breeze has

if (!(widget->testAttribute(Qt::WA_WState_Created) ||
widget->internalWinId()))
   KWindowSystem::blur(widget->winId(), ...)

which makes sense as the style should not be creating surfaces in polish
and I don't want to remove it.

This is problematic with QMenu::polish which has a subtle behavioural
change compared to QMenu::exec in that it calls ensurePolish before
creating the surface.

Plasma::ContainmentInterface accidentally has this workaround already
for parent window setting.

Test Plan:
Right clicked on desktop icon
Looked amazing

Reviewers: #plasma, hein

Reviewed By: #plasma, hein

Subscribers: hein, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D14174

M  +1    -0    containments/desktop/plugins/folder/foldermodel.cpp

https://commits.kde.org/plasma-desktop/42a74c9a79baafb222513f3ffad35b736b708454
Comment 13 Matej Mrenica 2018-07-17 16:14:45 UTC
(In reply to David Edmundson from comment #12)
> Git commit 42a74c9a79baafb222513f3ffad35b736b708454 by David Edmundson.
> Committed on 17/07/2018 at 09:34.
> Pushed by davidedmundson into branch 'master'.
> 
> Fix blur behind folderview context menus
> 
> Summary:
> Breeze has
> 
> if (!(widget->testAttribute(Qt::WA_WState_Created) ||
> widget->internalWinId()))
>    KWindowSystem::blur(widget->winId(), ...)
> 
> which makes sense as the style should not be creating surfaces in polish
> and I don't want to remove it.
> 
> This is problematic with QMenu::polish which has a subtle behavioural
> change compared to QMenu::exec in that it calls ensurePolish before
> creating the surface.
> 
> Plasma::ContainmentInterface accidentally has this workaround already
> for parent window setting.
> 
> Test Plan:
> Right clicked on desktop icon
> Looked amazing
> 
> Reviewers: #plasma, hein
> 
> Reviewed By: #plasma, hein
> 
> Subscribers: hein, plasma-devel
> 
> Tags: #plasma
> 
> Differential Revision: https://phabricator.kde.org/D14174
> 
> M  +1    -0    containments/desktop/plugins/folder/foldermodel.cpp
> 
> https://commits.kde.org/plasma-desktop/
> 42a74c9a79baafb222513f3ffad35b736b708454

Will this change make it into Plasma 5.13.4? Or we will have to wait for 5.14?
Comment 14 Alex Nemeth 2018-07-17 18:28:31 UTC
Created attachment 113989 [details]
desktop icon menu

It's very cool that you found a solution. Thank you for helping me out. I would have never guessed this.
However even with the patch applied I still have the glitch.
Comment 15 David Edmundson 2018-07-17 18:51:13 UTC
Heh.
Does this work: 
    menu->setAttribute(Qt::WA_TranslucentBackground);
Comment 16 Alex Nemeth 2018-07-17 19:38:54 UTC
Created attachment 113992 [details]
video of half-working

Adding this solves it for desktop icon menu:
    menu->setAttribute(Qt::WA_TranslucentBackground);
However this is not the only case of using QMenu::popup() (see video)
Some 3rd party apps use it too.
Can this be done in Breeze theme itself?
Is there any technical possibility that doesn't allow it?
Comment 17 David Edmundson 2018-07-17 19:48:57 UTC
Huzzah, I had some stray code setting that when trying different things :/

I'll have a look at whether we can do it in Breeze. I'm not convinced.
Comment 18 Alex Nemeth 2018-07-17 20:27:50 UTC
(In reply to David Edmundson from comment #17)
> Huzzah, I had some stray code setting that when trying different things :/
> 
> I'll have a look at whether we can do it in Breeze. I'm not convinced.

it works in QtCurve and Kvantum too, so we should be able to do it in Breeze too, right?
Comment 19 Patrick Silva 2018-09-11 00:04:35 UTC
Where is the fix? Bug persists on both Arch Linux (plasma 5.13.5) and neon dev unstable under X11.
Comment 20 Matej Mrenica 2018-09-11 05:19:47 UTC
(In reply to Dr. Chapatin from comment #19)
> Where is the fix? Bug persists on both Arch Linux (plasma 5.13.5) and neon
> dev unstable under X11.

AFAIK The fix landed a while ago, and works just fine. 
The bug only persists in VLC. Arch Linux + Plasma 5.13.5 intel GPU
Comment 21 Patrick Silva 2018-09-11 12:11:51 UTC
Created attachment 114898 [details]
screencast recorded on neon dev unstable

Does anyone else confirm that the problem persists?

My Arch and Neon are running on different computers.
Arch computer has intel hd graphics, neon computer has intel hd 4000.
Comment 22 Alex Nemeth 2018-09-11 19:44:30 UTC
I also still have the problem.
Here is an attempt to fix it for the desktop icon context menu: D15435
Comment 23 Alex Nemeth 2018-09-11 19:44:58 UTC
(In reply to Alex Nemeth from comment #22)
> I also still have the problem.
> Here is an attempt to fix it for the desktop icon context menu: D15435

Full link: https://phabricator.kde.org/D15435
Comment 24 Patrick Silva 2018-09-12 12:03:02 UTC
*** Bug 398413 has been marked as a duplicate of this bug. ***
Comment 25 umer 2018-09-14 09:05:23 UTC
"drag and drop" copy/paste menu also have this broken blur effect in kde neon 5.13.5
Comment 26 Patrick Silva 2018-09-14 11:21:41 UTC
(In reply to umer from comment #25)
> "drag and drop" copy/paste menu also have this broken blur effect in kde
> neon 5.13.5

Same thing in plasma 5.14 beta, Arch Linux.
Comment 27 Patrick Silva 2018-09-15 00:47:41 UTC
Under Wayland the bug occurs with the context menu of window decoration. Both neon dev unstable and Arch Linux (plasma 5.14 beta) are affected.

Under X11 the bug occurs with the context menu of plasma notification. For example, press printscreen to open spectacle and save the screenshot. Right click the notification and you can see the bug in the context menu.

Can we reopen this report or we need new reports?
Comment 28 Nate Graham 2018-09-19 22:14:09 UTC
*** Bug 398827 has been marked as a duplicate of this bug. ***
Comment 29 Christoph Feck 2018-10-04 23:47:54 UTC
I would suggest to file individual reports for the cases where it (still) does not work correctly.
Comment 30 Ievgen Sobko 2018-10-05 11:56:48 UTC
I created new issue but Nate marked it as duplicate of this and closed. Maybe it is better to reopen this one.
Comment 31 Alex Nemeth 2018-10-05 12:54:30 UTC
Fixing them individually is actually a workaround of the real problem.
I introduced the transparent menu feature. I'm sorry but I have no idea how to really fix this. 
I looked at Kvantum and QSvgStyle where this actually works, but can't figure out what their devs did to achieve this. 
I think currently our best option may be to fix this as workaround in the most obvious places (like we did it with the desktop context menu).
Comment 32 Patrick Silva 2018-10-20 12:28:12 UTC
bug 399680 is related
Comment 33 Patrick Silva 2018-10-21 17:40:31 UTC
another related report: bug 400092
Comment 34 trmdi 2019-07-14 00:00:28 UTC
The issue is still not resolved completely.
Comment 35 Nate Graham 2019-07-14 11:12:28 UTC

*** This bug has been marked as a duplicate of bug 399680 ***