Differential D2819

Pop-up Palette crashes after switching Canvas on OSX
ClosedPublic
Actions

Authored by beelzy on Sep 20 2016, 2:39 PM.

Details

Reviewers
rempt
Summary

Whenever you switch the canvas (eg, by changing OpenGL mode), accessing the pop-up palette crashes. This might also work for OSX users who can access the pop-up palette, but not the brushHud feature.

Apparently, this is caused by the brushHud object being referenced by KisPopupPalette. m_brushHud is destroyed instead of having its parent updated as well during the popup palette setParent call while setting the new canvas. (I don't entirely know why this only happens on OSX...)

So I just override the setParent function in QWidget for KisPopupPalette and set m_brushHud's new parent object as well.

Sorry about removing the extra spaces; feel free to revert those changes.

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
beelzy updated this revision to Diff 6832.Sep 20 2016, 2:39 PM
beelzy retitled this revision from to Pop-up Palette crashes after switching Canvas on OSX.
beelzy updated this object.
beelzy edited the test plan for this revision. (Show Details)
beelzy added a reviewer: rempt.
beelzy set the repository for this revision to R37 Krita.
beelzy updated this object.Sep 20 2016, 2:42 PM
pkouame added a subscriber: pkouame.Sep 22 2016, 3:43 PM

1 - New window
2 - openGl defaults
3 - brushHud works
4 - change display openGL setting HQ in my case
5 - brushHud crashes

  1. fairly reproducible

My environment:
krita master 3.0.2 beta git (b9c7f56)
ext_qt qt5.7 rebuilt and patched
xcode 8
macOS Sierra
macPro 3,1
console: Segmentation fault: 11 (console log also attached)

krita-brushHud-crash.txt81 KBDownload

krita-brushHud-crash-console.txt9 KBDownload

rempt edited edge metadata.Sep 26 2016, 9:22 AM

The crash also happens on other operating systems (of course). Do you know whether there's a bug report for it as well? I'll push the patch now.

rempt accepted this revision.Sep 26 2016, 9:23 AM
rempt edited edge metadata.
This revision is now accepted and ready to land.Sep 26 2016, 9:23 AM
pkouame added a comment.Sep 26 2016, 2:44 PM

Ok - may just be filing this for posterity's sake. You decide.

I can fonfirm that beelzy's fix will work on macOS (same build and environment as above). Implemented the same thing via patches below. My bad for not scrolling down enough. Geez.

While I had pop-up open, I refactored the class a bit to follow a better (inmho) private class patern.
I also highly recommend switching on clang's analyzer via -DECM_ENABLE_SANITIZERS='address', the resultant trace (also attached) was immediately useful.

Later.

pkouame added a comment.Sep 26 2016, 2:48 PM

patches

popup-brush-patch.diff22 KBDownload

clang analyzer trace

popup-brush-asan-trace.txt90 KBDownload

pkouame added a comment.Sep 26 2016, 3:31 PM

stripped stupid comments out of patch...

popup-brush-patch-2.diff22 KBDownload

Revision Contents
Changeset List

Diff 6832

View Options

libs/ui/kis_popup_palette.h

Loading...
View Options

libs/ui/kis_popup_palette.cpp

Loading...