All bugs I want to be resolved before Krita 4.2.9
Open, Needs TriagePublic

Description

Straight-forward regressions:

More problematic...:

tymond created this task.Jan 6 2020, 6:26 PM
tymond updated the task description. (Show Details)Jan 6 2020, 6:30 PM
tymond updated the task description. (Show Details)Jan 6 2020, 7:11 PM

415891 is a duplicate of 414818, the file attached has an annotations folder

tymond added a comment.Jan 6 2020, 9:53 PM

Yeah, a random empty file also has an annotations folder. The problem in bug 414818 is that Krita gets in a loop on the Colorize Mask because of Dmitry's changes to assigning color space to layers. Annotations directory contains color profiles (and possibly some other metadata; here there is icc and proofing/icc, in the empty file I've got only icc), so I guess it might be that removing it will help loading the file since some other path is used, I guess just assigning a default color space instead of reading, maybe somehow it skips the infinite loop. The file in bug 41891 however doesn't have any Colorize Masks, only Group layers, File layers and Fill layers. It might be related of course; maybe there is the same reason. But they're not duplicates since the second one doesn't have Colorize Mask that would cause the trouble.

that is probably the case. im debugging 415891 now. Ill let you know what I find

tymond added a comment.Jan 7 2020, 2:59 AM

You could probably make them a duplicate only if you knew one patch will solve both and then add a comment to the main bug report that there are other issues as well (Fill layers, File layers, Group layers).

415891 gets into a infinite wait trying to process al events that never cease to occur. its different than the loop the other bug is. I don't know why it happens but I can at least get out of the waiting and get the file to load.

kis_file_layer.cpp:70 calls reloadImage()
kis_safe_document_loader.cpp:157 calls fileChangedCompressed(true) which gets stuck in a waiting loop when QApplication::processEvents() is called. (sending false to fileChanged avoids the processEvents call). I need still to bisect/search for the commit that induced the error and add it to the original bug report.

rempt updated the task description. (Show Details)Jan 7 2020, 12:37 PM
rempt added a subscriber: rempt.

I removed

Because that one is closed by Ahab as not a bug -- it probably was some form of user error.

Ahh I made a mistake with the bug number... what I meant was this: https://bugs.kde.org/show_bug.cgi?id=415086 it has my comments and I wanted it on this list for a reason, and the reason is that it must be related to input/focus issues, not user errors and I have reasons to think that, and I believe I stated those reasons in the comments.

tymond updated the task description. (Show Details)Jan 7 2020, 12:48 PM
rempt added a comment.Jan 7 2020, 12:49 PM

For that bug, I've seen it happen, too. I could fix it for that user by showing the global selection mask and deleting that. There was something really screwed up in that file.

One of the reasons I believe it's not an user error is that they tell me Select -> Deselect is greyed out, and when I tell them to do Select -> Select All, and then Select -> Deselect, it fixes the issue. It shouldn't work because either there is a selection and you can easily deselect it, or there is no selection and deselection won't change anything. And it happened enough times that I can see a pattern. The only problem is that I cannot reproduce it, and I can't even be sure if all cases when I thought it was this what happened it was the same issue all the time or not...

tymond updated the task description. (Show Details)Jan 7 2020, 12:53 PM
rempt added a comment.Jan 7 2020, 1:02 PM

Yes, that's exactly what I saw, too.

rempt added a comment.Jan 7 2020, 3:25 PM

This looks like the broken onion skin changes:

commit 7b0359f1ccda80589ae4c38af8fed3fea062a4a6
Author: Dmitry Kazakov <dimula73@gmail.com>
Date: Mon Nov 18 18:34:01 2019 +0300

Don't update paint layer when onion skins are disabled

commit 1ba0a6baf17237bdd68959155db944d23a0abb58
Author: Dmitry Kazakov <dimula73@gmail.com>
Date: Mon Nov 18 18:22:14 2019 +0300

Fix color space of onion skins cache device

BUG:407251

commit adafa8b73887cf732ddb5cf41b2305a4f94b0fe6
Author: Dmitry Kazakov <dimula73@gmail.com>
Date: Mon Nov 18 16:54:58 2019 +0300

Fix transforming layers that have Onion Skins enabled

The patch implements a concept of KisDecoratedNodeInterface. All layers
that spit decorations data into the layers stack should inherit from it
and handle the requests properly.

BUG:408152
rempt updated the task description. (Show Details)Jan 8 2020, 10:03 AM
rempt updated the task description. (Show Details)
rempt updated the task description. (Show Details)Jan 8 2020, 10:07 AM
tymond added a comment.Jan 8 2020, 2:14 PM

Bug 414572 - I cannot reproduce it. (zoom works butterly smooth on my Yoga on Linux).
Bug 414672 - I believe it's already fixed.
Bug 415932 - can be reproduced using h)_Chalk_Soft brush.
Bug 415891 - I cannot reproduce it.
Bug 415086 - no known steps to reproduce.
Bug 415773 - I cannot reproduce it; I believe it is related to bug 414572 which I cannot reproduce either.
Bug 415213 - probably fixed already; I'm gonna wait one day and ask them to try Krita Plus version.

mwein added a subscriber: mwein.Jan 9 2020, 2:23 AM

Hm the "cannot draw" bug 415086 sounds a lot like this bug:
https://bugs.kde.org/show_bug.cgi?id=412808

Looks like the bug was fixed just after 4.2.8 beta was tagged, and right now I can't see a backport.

Just tested again, and the 4.2.8 appimage definitely has this undo bug on vector selections that bugs the selection mask, which can lead to this odd situation that you cannot deselect because no pixel counts as selected, but cannot draw either because there is a selection mask that is all black.

tymond added a comment.EditedJan 9 2020, 1:49 PM

@mwein Wow! Thanks! I totally forgot about this bug; I know I knew about it but... and it makes sense with reopening too, since then Krita will just check the global selection mask and see there is nothing there.
Although one person said that when it happens, all other old files are affected. It doesn't fit this description.
I guess I can try to cherry-pick relevant commits (one for the fix and one for the fix to the fix :P ) you mentioned and see what happens next.

tymond updated the task description. (Show Details)Jan 9 2020, 1:50 PM
tymond updated the task description. (Show Details)Jan 9 2020, 1:55 PM
tymond updated the task description. (Show Details)Jan 9 2020, 4:04 PM
rempt updated the task description. (Show Details)Jan 10 2020, 10:08 AM
rempt updated the task description. (Show Details)
tymond added a comment.EditedJan 14 2020, 6:19 PM

Transform Tool, possible partial solutions (partial as in: will prevent crashing, but they're ugly and may have other issues):

  • cancel transformation when switching tools. It means changing the behaviour from useful and conveniant to a bit less conveniant, but still possible to work with. (but for 4.3.0 we need something else, something better.)
  • cancel the stroke from inside of the transform tool strategy: cancelStrokeId().toStrongRef()->cancelStroke()
  • just ignoring cancelling the stroke and initializing the uninitialized arguments that would later cause the boost::optional assert. See this snippet: https://invent.kde.org/snippets/659 (seem to add additional unnecessary undo step)

For the last one, which I'm in favour now because it seems to be working well and it does make sense (although I still believe something should cancel the stroke nicely...), I made some notes in the snippet. I thought it would be good to repeat them here. (https://invent.kde.org/snippets/659)

My thinking process:

  • calling cancelling code seem really weird in this place. But it just calls finishStrokeImpl anyway.
  • if we remove the if, Krita would cause a boost::optional assert because args are uninitialized.
  • so let's initialize them.
  • possible issues: undo steps (because the stroke is *not* cancelled, but just finished with identity (?) transformation)
  • my tests:
    • draw a line
    • switch to transform tool
    • switch to Freehand Tool (before the snippet: ASSERT: "!m_strokeEnded || m_isCancelled" in file /home/tymon/devsec/krita/libs/image/kis_stroke.cpp, line 84; just after removing the if, without initializing the arguments: krita: /usr/include/boost/optional/optional.hpp:1191: boost::optional<T>::reference_type boost::optional<T>::get() [with T = ToolTransformArgs; boost::optional<T>::reference_type = ToolTransformArgs&]: Assertion 'this->is_initialized()' failed., after my change: works just fine and it doesn't create an unnecessary undo step or anything, and I believe it doesn't transform pixels unnecessarily...)
    • switch to transform tool
    • make a few transformations, confirming them using Enter (result: one Undo step - this behaviour seem a bit incorrect considering the next point, but it's exactly how it was in 4.2.7)
    • make a few transformation without confirming (result: undo will undo every transformation separately, but there is no undo step in the Undo history)

Dmitry stepped in and made following commits:
f62910b0
a5d92c04
f88c8b35
dcae8c78
regarding the transform tool crashes and asserts.

tymond updated the task description. (Show Details)Jan 16 2020, 12:07 PM
tymond updated the task description. (Show Details)Jan 17 2020, 1:33 AM
tymond updated the task description. (Show Details)
tymond updated the task description. (Show Details)Jan 21 2020, 3:18 PM
tymond updated the task description. (Show Details)Fri, Jan 31, 2:33 PM