Differential D11475

Custom theme flickering and displaying weird background
ClosedPublic
Actions

Authored by stefanyohansson on Mar 19 2018, 12:35 AM.

Details

Reviewers
dfaure
dvratil
Commits
R773:11867cb8d6b0: Replace custom XShape call with QWidget::setMask()
Summary

Testing new kf frameworks port branch, I decide to apply my custom theme on that branch and I see a lot of weirdos like png flick and an annoying gray background (flickering too).

Test Plan
  • checkout frameworks branch
  • unzip theme on your amor local folder (app data local folder)
    amor-hatsune-miku.zip102 KBDownload
  • build/open amor
  • select hatsune theme

Diff Detail

Repository
R773 Amor
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
stefanyohansson requested review of this revision.Mar 19 2018, 12:35 AM
stefanyohansson created this revision.
dvratil added a comment.Apr 2 2018, 2:48 PM

Setting the window attributes is OK, but is the shape mask change needed too? The line is a direct port from the X11 XShapeCombineMask, so it should behave the same. Changing the operation from SET to UNION might break other themes?

stefanyohansson added a comment.EditedApr 2 2018, 5:16 PM
In D11475#238585, @dvratil wrote:

Setting the window attributes is OK, but is the shape mask change needed too? The line is a direct port from the X11 XShapeCombineMask, so it should behave the same. Changing the operation from SET to UNION might break other themes?

yes, setting the window attribute will remove only the background flick. Using SET my theme display missing pixels and weird flick, I think it's related to different width sprites (this is a problem with my theme). Using UNION didn't break default themes and fixed the problem, already tested one by one. (my theme works well with X11 version, I think something is different on xcb internal combine mask logic)

dvratil added a comment.Apr 11 2018, 2:21 PM

Someone else recently sent us a patch that replaces this whole XCB call with simple QWidget:;setMask() call, which Qt internally handles to change shape of the window, that I believe would solve your problem completely. Let's wait if they upload the patch to Phab, I'll link it there.

stefanyohansson updated this revision to Diff 33349.EditedApr 30 2018, 4:53 PM

since no one uploads that solution using QWidget::setMask. I tried and it works.

stefanyohansson updated this revision to Diff 33351.EditedApr 30 2018, 5:03 PM

updating again and use only setMask changeset. There's no need for

+    setAttribute( Qt::WA_TranslucentBackground, true );
+    setAttribute( Qt::WA_OpaquePaintEvent, false );
dvratil accepted this revision.Apr 30 2018, 11:47 PM

Thanks!

This revision is now accepted and ready to land.Apr 30 2018, 11:47 PM
dvratil added a comment.Apr 30 2018, 11:47 PM

Do you have commit access to KDE repos or should I push it for you?

aacid added a subscriber: aacid.May 1 2018, 12:30 AM

He doesn't have commit access.

Closed by commit R773:11867cb8d6b0: Replace custom XShape call with QWidget::setMask() (authored by stefanyohansson, committed by dvratil). · Explain WhyMay 1 2018, 10:03 AM
This revision was automatically updated to reflect the committed changes.
stefanyohansson added a comment.May 1 2018, 10:48 PM
In D11475#256234, @dvratil wrote:

Do you have commit access to KDE repos or should I push it for you?

Hey, I think something is wrong here. I updated my diff before your comment: https://phabricator.kde.org/D11475?id=33353
only one line: QWidget::setMask(mask); and the commit was the first one submitted. Sorry if I do something wrong, it is my first time on phabricator.

dvratil added a comment.May 1 2018, 10:56 PM
In D11475#256836, @stefanyohansson wrote:
In D11475#256234, @dvratil wrote:

Do you have commit access to KDE repos or should I push it for you?

Hey, I think something is wrong here. I updated my diff before your comment: https://phabricator.kde.org/D11475?id=33353
only one line: QWidget::setMask(mask); and the commit was the first one submitted. Sorry if I do something wrong, it is my first time on phabricator.

My bad, I got fooled by the arcanist again :/ I pushed the correct version now. Sorry.

stefanyohansson added a comment.May 1 2018, 10:57 PM
In D11475#256840, @dvratil wrote:
In D11475#256836, @stefanyohansson wrote:
In D11475#256234, @dvratil wrote:

Do you have commit access to KDE repos or should I push it for you?

Hey, I think something is wrong here. I updated my diff before your comment: https://phabricator.kde.org/D11475?id=33353
only one line: QWidget::setMask(mask); and the commit was the first one submitted. Sorry if I do something wrong, it is my first time on phabricator.

My bad, I got fooled by the arcanist again :/ I pushed the correct version now. Sorry.

No problem. You're so fast. haha

Revision Contents
Changeset List

PathPackages
Msrc/amorwidget.cpp (4 lines)

Diff 33376

View Options

src/amorwidget.cpp

Loading...