Skip to content

Do not set Qt::WA_OpaquePaintEvent - #7643

Merged
tresf merged 1 commit into
LMMS:masterfrom
sakertooth:do-no-set-opaque-pe
Feb 20, 2025
Merged

Do not set Qt::WA_OpaquePaintEvent#7643
tresf merged 1 commit into
LMMS:masterfrom
sakertooth:do-no-set-opaque-pe

Conversation

@sakertooth

Copy link
Copy Markdown
Contributor

From the Qt docs, Qt:WA_OpaquePaintEvent:

Indicates that the widget paints all its pixels when it receives a paint event. Thus, it is not required for operations like updating, resizing, scrolling and focus changes to erase the widget before generating paint events. The use of WA_OpaquePaintEvent provides a small optimization by helping to reduce flicker on systems that do not support double buffering and avoiding computational cycles necessary to erase the background prior to painting. Note: Unlike WA_NoSystemBackground, WA_OpaquePaintEvent makes an effort to avoid transparent window backgrounds. This flag is set or cleared by the widget's author.

Removing calls to set this attribute seems to have fixed the infamous glitch problem where child widgets (like clips from the song editor) start drawing on top on other, completely different widgets. Maybe there's something I'm missing though, so extra insight would be appreciated.

@sakertooth sakertooth added needs testing This pull request needs more testing and removed needs testing This pull request needs more testing labels Jan 5, 2025
@tresf

tresf commented Feb 18, 2025

Copy link
Copy Markdown
Member

@sakertooth I'm fine merging this as-is (it's easy enough to revert if it causes regressions) however I believe the issue with this patch is that the bug that it describes doesn't have obvious steps to reproduce.

Since the code is straightforward, testing is about all that should be holding this up, but you've removed the needs testing tag, but that's all that I can see needing before merge.

Is there a quick way to reproduce this or is it too sporadic? The docs mention "systems that do not support double buffering", so I also fear that this behavior could be dependant on outside factors.

@qnebra

qnebra commented Feb 18, 2025

Copy link
Copy Markdown
Collaborator

Is there a quick way to reproduce this or is it too sporadic? The docs mention "systems that do not support double buffering", so I also fear that this behavior could be dependant on outside factors.

Have bunch of patterns in Song Editor, open instrument window, have it on top of song editor, go 'brrrr' with zooming in Song Editor using ctrl + mousewheel, Instrument Window must be on top just as an active window. It is easier to do than to describe.
Instrument Window used as an example, it glitched out with other lmms windows also.

@tresf

tresf commented Feb 18, 2025

Copy link
Copy Markdown
Member

Is there a quick way to reproduce this or is it too sporadic? The docs mention "systems that do not support double buffering", so I also fear that this behavior could be dependant on outside factors.

Have bunch of patterns in Song Editor, open instrument window, have it on top of song editor, go 'brrrr' with zooming in Song Editor using ctrl + mousewheel, Instrument Window must be on top just as an active window. It is easier to do than to describe. Instrument Window used as an example, it glitched out with other lmms windows also.

@qnebra can you confirm that this PR fixes the issue?

@qnebra

qnebra commented Feb 19, 2025

Copy link
Copy Markdown
Collaborator

@qnebra can you confirm that this PR fixes the issue?

In my case it fixes issue, when I did tests when this PR came out

@tresf tresf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving on behalf of @qnebra. @qnebra if you want to be added to the project team so you can sign-off on stuff, just let me know.

@tresf
tresf merged commit ea02b3a into LMMS:master Feb 20, 2025
@qnebra

qnebra commented Feb 20, 2025

Copy link
Copy Markdown
Collaborator

@qnebra if you want to be added to the project team so you can sign-off on stuff, just let me know.

Let's say do it. Will use it in rare cases, when some pull request interest me enough to test it. Also will do only functional reviews, not code reviews. Code reviews are just outside of my skillset at this moment.

@tresf

tresf commented Feb 20, 2025

Copy link
Copy Markdown
Member

@qnebra if you want to be added to the project team so you can sign-off on stuff, just let me know.

Let's say do it. Will use it in rare cases, when some pull request interest me enough to test it. Also will do only functional reviews, not code reviews. Code reviews are just outside of my skillset at this moment.

Invite sent.

@sakertooth
sakertooth deleted the do-no-set-opaque-pe branch February 20, 2025 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants