Differential D1215

Wacom tablet crash in Krita
ClosedPublic
Actions

Authored by fazek on Mar 24 2016, 5:09 PM.

Details

Reviewers
dkazakov
rempt
Summary

This is a patch for the bug I mentioned on the mail list earlier. When closing a window, sometimes the deleted window still receives some tablet events. I solved this problem with comparing the window address with all active top level windows. In Krita, usually there are very few of them so this is not a real bottleneck.

TODO: it seems the program only adds the new windows to the m_windowMapper hash table, but never deletes the invalid windows. Depending on the meaning of the keys and addresses, this is a dangerous strategy: maybe later a different window can get the same key and mess everything. It would be ideal if the windowMapper could contain, for example QWeakPointers to the windows but unfortunately the QWindow is just a simple address here.

Test Plan

For me this is a sure way to crash the program:

  1. Start Krita
  2. Open the new document dialog from the file menu.
  3. Use your tablet and click on the Create button with the stylus.

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
fazek updated this revision to Diff 2943.Mar 24 2016, 5:09 PM
fazek retitled this revision from to Wacom tablet crash in Krita.
fazek updated this object.
fazek edited the test plan for this revision. (Show Details)
fazek set the repository for this revision to R37 Krita.
fazek edited the test plan for this revision. (Show Details)
fazek added reviewers: dkazakov, rempt.
dkazakov edited edge metadata.Mar 25 2016, 8:13 AM

Hi, @fazek!

The fact that we don't delete windows is a really nice catch! Thank you!

In fact all QWindow objects are derived from QObject. So we can use QPointer for them, which is a kind of weak shared pointer. Like that:

QHash<xcb_window_t, QPointer<QWindow>> m_windowMapper;

This should probably solve the original bug. You can also add an additional cleaning up phase to QXcbConnection::notifyEnterEvent() to remove all the QPointer's which became invalid in the meantime.

fazek updated this revision to Diff 2946.Mar 25 2016, 10:57 AM
fazek edited edge metadata.

@dkazakov, your idea worked perfectly! It solved the crash problem too. I also added the cleanup part, please check it.

rempt accepted this revision.Mar 25 2016, 11:48 AM
rempt edited edge metadata.

Yes, that looks good to me, too!

This revision is now accepted and ready to land.Mar 25 2016, 11:48 AM
dkazakov accepted this revision.Mar 25 2016, 12:16 PM
dkazakov edited edge metadata.

Looks perfect, please commit! :)

fazek closed this revision.Mar 25 2016, 12:50 PM

Revision Contents
Changeset List

Diff 2946

View Options

libs/ui/input/wintab/qxcbconnection.cpp

Loading...
View Options

libs/ui/input/wintab/qxcbconnection_xi2.h

Loading...