Bug 386726 - Brush engine crashes when switching smoothing options while drawing
Summary: Brush engine crashes when switching smoothing options while drawing
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: Brush engines (show other bugs)
Version: 3.3.1
Platform: Other Microsoft Windows
: NOR crash
Target Milestone: ---
Assignee: Krita Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-11-10 22:27 UTC by Isaac Zuniga
Modified: 2018-06-26 15:37 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
The Krita log file from the %appdata% folder. (194.80 KB, text/plain)
2017-11-10 22:32 UTC, Isaac Zuniga
Details
Backtraces (8.35 KB, text/plain)
2017-11-24 14:25 UTC, Nicholas LaPointe
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Isaac Zuniga 2017-11-10 22:27:19 UTC
While I am drawing, if I switch from any of the listed brush smoothing engines below:

- Basic Smoothing
- Simple Smoothing
- Weighted Smoothing

then if I switch to the Stabilizer while I am currently in a brushstroke, Krita crashes. I am not sure if there is a workaround for this, but it is somewhat of annoyance. I am open to any options/workaround possible; if I am doing something wrong, please do let me know.

I have a Google Drive video showing how to reproduce the issue: https://drive.google.com/file/d/1jR-l63ciNWxDewEkUQt8RvJBV8j98STv/view?usp=sharing
Comment 1 wolthera 2017-11-10 22:28:55 UTC
Ooh, a video.

Can I tempt you to see if you can make a backtrace as well? That would point us to the exact line that crashes! https://docs.krita.org/Dr._Mingw_debugger
Comment 2 Isaac Zuniga 2017-11-10 22:32:24 UTC
Created attachment 108779 [details]
The Krita log file from the %appdata% folder.

Sure thing, I didn't even know that a log file existed for this. Yes, that would truly be helpful.

Please note, that I did open Krita up a couple more times after I recorded the video, mostly to make sure that the settings were the way I wanted them to be the next time I wanted to use the program.
Comment 3 Isaac Zuniga 2017-11-11 06:36:44 UTC
(In reply to wolthera from comment #1)
> Ooh, a video.
> 
> Can I tempt you to see if you can make a backtrace as well? That would point
> us to the exact line that crashes! https://docs.krita.org/Dr._Mingw_debugger


Sorry about the posting of the comment, I meant to have sent it as a reply. (This is my first time using this bug report system; I'm not too knowledgeable about it.)

I attached the log file, and I really hope it can be of help to you and the other developers.
Comment 4 Halla Rempt 2017-11-21 10:37:55 UTC
Hm, that crash log isn't related to anything to do with strokes or smoothing; something weird happens when updating the display. I'm wondering what that might be, though. How do you exactly switch stabilizer mode when drawing? With a shortcut?
Comment 5 Isaac Zuniga 2017-11-21 13:35:37 UTC
(In reply to Boudewijn Rempt from comment #4)
> Hm, that crash log isn't related to anything to do with strokes or
> smoothing; something weird happens when updating the display. I'm wondering
> what that might be, though. How do you exactly switch stabilizer mode when
> drawing? With a shortcut?

That is correct, I use the CTRL + (1,2,3,4) shortcuts to switch between smoothing engines. This problem does not happen if I stop drawing and manually switch the smoothing engine by clicking on the dropdown menu.
Comment 6 Nicholas LaPointe 2017-11-24 14:25:47 UTC
Created attachment 109035 [details]
Backtraces

I was able to reproduce this with two different backtraces.
I assigned the stabilizers to the CTRL + (1,2,3,4) shortcuts as per Isaac's comment, then switched between them while painting. It may take a bit, and you may need to release the mouse button and then start painting again a few times, but it does always seem to crash.
Comment 7 Nicholas LaPointe 2017-11-24 14:39:14 UTC
Sorry for the quick double post!

Selecting "Stabilizer" and then switching to any other smoothing type while drawing will almost always crash when the user stops painting, giving the first backtrace that I provided. This is the opposite order of what was described in original post in this report.
(The behavior of painting isn't correct in this state, either -- the cursor stops and lines are drawn from the real cursor position to the stuck cursor.)
Comment 8 Isaac Zuniga 2017-11-24 17:02:26 UTC
It's alright man, I'm glad you double-posted. It shows that you discovered something.

I read your first comment then the second one, is there a workaround for this, or do I just have to stop the current brushstroke, change the stabilizer (1,2,3, or 4), then start again?

Also, I'd like to ask is this something that's still being looked into, or has it been confirmed that this is just simply how the program is?
Comment 9 Nicholas LaPointe 2017-11-24 19:30:16 UTC
(In reply to Isaac from comment #8)
> I read your first comment then the second one, is there a workaround for
> this, or do I just have to stop the current brushstroke, change the
> stabilizer (1,2,3, or 4), then start again?
Yes, I think you will have to wait, perhaps until the brushstroke has fully finished drawing on the screen, then start again.

> Also, I'd like to ask is this something that's still being looked into, or
> has it been confirmed that this is just simply how the program is?
I marked the bug as "confirmed," and the list of bugs marked that way could be considered a kind of to-do list.
I don't have the knowledge to fix it myself, but it may get fixed at any time by someone who does.
The good thing is that it doesn't crash only for you, so it should be a little easier for them to study what's wrong. :)
Comment 10 Halla Rempt 2017-11-28 08:08:11 UTC
the easy way out is to check whether the stroke is still valid, instead of asserting on it:

void KisStrokesQueue::addJob(KisStrokeId id, KisStrokeJobData *data)
{
    QMutexLocker locker(&m_d->mutex);

    KisStrokeSP stroke = id.toStrongRef();
    Q_ASSERT(stroke);

The stroke is probably 0 because changing the smoothing option starts a new stroke.

Thread 1 "krita" received signal SIGSEGV, Segmentation fault.
KisStroke::lodBuddy (this=this@entry=0x0) at src/krita_3.3/libs/image/kis_stroke.cpp:301
301         return m_lodBuddy;
(gdb) bt
#0  KisStroke::lodBuddy (this=this@entry=0x0) at src/krita_3.3/libs/image/kis_stroke.cpp:301
#1  0x00007ffff38596ec in KisStrokesQueue::addJob (this=<optimized out>, id=..., data=0x86f2940) at src/krita_3.3/libs/image/kis_strokes_queue.cpp:344
#2  0x00007ffff38653a5 in KisUpdateScheduler::addJob (this=0x86bb108, id=..., data=data@entry=0x86f2940) at src/krita_3.3/libs/image/kis_update_scheduler.cpp:187

We already decided to change all asserts into KIS_SAFE_ASSERT too.
Comment 11 Halla Rempt 2017-11-28 08:12:23 UTC
This should mitigate the issue:

diff --git a/libs/image/kis_strokes_queue.cpp b/libs/image/kis_strokes_queue.cpp
index a916b78472..23520f0104 100644
--- a/libs/image/kis_strokes_queue.cpp
+++ b/libs/image/kis_strokes_queue.cpp
@@ -345,7 +345,7 @@ void KisStrokesQueue::addJob(KisStrokeId id, KisStrokeJobData *data)
     QMutexLocker locker(&m_d->mutex);
 
     KisStrokeSP stroke = id.toStrongRef();
-    Q_ASSERT(stroke);
+    KIS_SAFE_ASSERT_RECOVER_RETURN(stroke);
 
     KisStrokeSP buddy = stroke->lodBuddy();
     if (buddy) {
@@ -364,7 +364,7 @@ void KisStrokesQueue::addMutatedJobs(KisStrokeId id, const QVector<KisStrokeJobD
     QMutexLocker locker(&m_d->mutex);
 
     KisStrokeSP stroke = id.toStrongRef();
-    Q_ASSERT(stroke);
+    KIS_SAFE_ASSERT_RECOVER_RETURN(stroke);
 
     stroke->addMutatedJobs(list);
 }
@@ -374,7 +374,7 @@ void KisStrokesQueue::endStroke(KisStrokeId id)
     QMutexLocker locker(&m_d->mutex);
 
     KisStrokeSP stroke = id.toStrongRef();
-    Q_ASSERT(stroke);
+    KIS_SAFE_ASSERT_RECOVER_RETURN(stroke);
     stroke->endStroke();
     m_d->openedStrokesCounter--;
 
lines 1-31/31 (END)


Note that I suspect that even with this patch, you will get really weird stuff happening, like a total restart of the stroke.
Comment 12 Halla Rempt 2017-11-28 08:34:51 UTC
Yeah, still crashes, which is not too surprising.
Comment 13 Isaac Zuniga 2017-11-28 14:49:34 UTC
(In reply to Boudewijn Rempt from comment #12)
> Yeah, still crashes, which is not too surprising.

Is there anything that I can do to help things along, or are things pretty much out of my hands at this point? (I'm open to anything that I can do to help.)
Comment 14 Halla Rempt 2017-11-28 14:52:48 UTC
Well... The reason it crashes is that our stroke system isn't designed to suddenly start doing another type of stroke. So, basically, while painting, these shortcuts should be disabled. Going the other way, making this possible, is a huge refactoring. For now, I recommend not using those shortcuts while painting...
Comment 15 Isaac Zuniga 2017-11-28 14:55:53 UTC
(In reply to Boudewijn Rempt from comment #14)
> Well... The reason it crashes is that our stroke system isn't designed to
> suddenly start doing another type of stroke. So, basically, while painting,
> these shortcuts should be disabled. Going the other way, making this
> possible, is a huge refactoring. For now, I recommend not using those
> shortcuts while painting...

I understand, and it seems if I stop my current stroke and then use a shortcut, there's no issue. (Compared to if I continue using the same stroke then activate a shortcut to change the stabilization.) I'm assuming that is fine, correct?

I didn't mean for this to turn into a long dig into code, rather I thought it was a bug. (Though, if others report it as well, I am curious what will become of this.) Thanks for helping me out with the issue, I really do appreciate it, especially all the developers/people who looked into this bug report. Much appreciated!
Comment 16 Halla Rempt 2017-11-28 15:03:42 UTC
It definitely _is_ a bug. Krita shouldn't crash when pressing a key while in the middle of making a stroke.
Comment 17 mvowada 2018-01-21 17:41:35 UTC
I can confirm this happens also with with Krita 4.0.0-beta1.1.appimage (probably as expected due to the nature of the problem)
Comment 18 Antti Savolainen 2018-06-26 15:33:02 UTC
According to Boudewijn this has been accidentally fixed at some point and I am unable to reproduce it.
Comment 19 Isaac Zuniga 2018-06-26 15:37:25 UTC
Really? That's awesome! Thanks for letting me know! :*)