[needs testing] First testing packages for the lager-based brush editor

Develop Artists Feedback & Testing
21

Windows 10 - Color smudge engine test

1st test
I had tested 3 brushes, everything worked well. I did all brush switching from inside the brush editor with Color Smudge as the selected engine. I tested all brushes on the canvas while the brush editor was open and also on the scratchpad. Then I attempted to switch from wojtryb6 N 16 painting smudge pattern to wojtryb6 N 23 blur and Krita crashed. (Krita never crashes on this machine, I don’t know if that info is helpful or not.)

2nd test:
Restarted Krita. Did everything again - got a second crash. This time I had been typing my test notes into a comment on KA on the laptop’s main screen and when I switched back to monitor #2 which is where I always use Krita, that’s when it crashed. I’m pretty sure this was the same sequence during the first crash as well but I’m not 100% sure.

3rd test:
Started Krita a 3rd time. Switched to Color Smudge Engine. Krita crashed as soon as I switched from Pixel Engine to Color Smudge in the brush editor.

Weird thing I noticed: following all the crashes, Krita started up with a brush I had not touched at all (a) Eraser Circle - Pixel Engine). I had never opened that brush in this version of Krita.

3 Crash log files (Google docs)

4th test:
No crash. Used 4 Color Smudge brushes and put them through the paces but couldn’t make it crash a 4th time. Everything worked as expected.

Summary:
Tested in a total of 4 sessions. 1st 3 sessions resulted in crash. 4th session, no crash. I even switched back and forth between Krita on monitor 2 to KA commenting screen on main laptop monitor to try and produce a crash. Maybe it was a coincidence and switching monitors had nothing to do with previous 3 crashes. (I wish I knew how to read a crash log - I’m so curious).

1 Like
22

Looking at the first and third crashlogs, it seems to be related to the Threshold spinbox found by right-clicking the Instant Preview checkbox. So I tested that and found that trying to change the value will sometimes crash Krita, but I’m not sure of the certain circumstances. From your crashes it seems that certain brushes can trigger the value to change.

I think this would be a good topic to discuss (in some other section of the forum somewhere). But, briefly: the important part is the backtrace; the list of functions that were called, most recent first. Ignore all the numbers and .dlls and look for familiar phrases; in this case, I can pick out these functions:

QDoubleSpinBox::setValue
KisLodAvailabilityModel::lodSizeThresholdChanged
KisPaintOpPresetsEditor::readOptionSetting
KisPaintopBox::resourceSelected
QWidgetWindow::handleMouseEvent

and get an idea of “a value was set in a spinbox, because the LoD (Instant Preview) Threshold changed, because of an option that was read from the paintop (brush) preset, which resource was selected in the paintop (brush) box, by activating it with the mouse”.
The second crashlog only mentions handling a mouse event - specifically, releasing the mouse from some kind of list. (QAbstractItemView::mouseReleaseEvent)

2 Likes
23

Hi, @TheFlow, @freyalupen and @sooz!

Thanks a lot for your testing! I think I have fixed all the issues you mentioned:

  • assert in colorsmudge-lightness mode
  • crash when changing the brush after altering “Instant Preview Availability Threshold”
  • fixed instant preview options to mark the brush as dirty
  • fixed non-synchronized instant preview availability checkbox when reloading the brush

Packages:

PS:
Sorry for ignoring the thread for some time. I was dragged into fixing bugs for the release :slight_smile:

4 Likes
24

Fixing bugs for a release is higher priority than fixing bugs in a WIP branch after all. :slight_smile:

I’ve confirmed the previously mentioned bugs are fixed.
But I found something which I failed to notice before (it’s in both package 3 and 4); when opening a document I get the following warnings:

Filtering HUD properties: property " “smudge_length” " does not exist!
Filtering HUD properties: property " “smudge_color_rate” " does not exist!

1 Like
25

I tested the new package and could not find anything wrong. No crashes at all. :kiki:

1 Like
26

Hi, @freyalupen and @sooz!

Thanks a lot for your testing! It looks like @freyalupen has found a real bug. I have just pushed a fix into my branch.

If you want to test again, here is a package for Linux. Though it is not strictly necessary now :slight_smile:

Linux AppImage X64: krita-5.2.0-prealpha-c2669a36c24-x86_64.appimage — Яндекс.Диск

My current plan is to devote a bit of time to documentation, so there might be no updates in the next couple of weeks. And then I’ll return to porting the rest of the brush engines.

2 Likes
27

Hi, all!

Now we have reached another milestone in the Lager port: MyPaint brush engine is ported now! It required really a lot of work, but now it seems to be finished and works fine :slight_smile:

The packages also include the work done by @tiar, @freyalupen on porting Experiment, Quick and Curve brush engines.

Could you please test the ported brush engines?

Packages:

Test Plan

  1. Check if MyPaint brushes still look the same as in the previous versions of Krita
  2. Check if MyPaint engine reacts on option changes in an “expected” way
  3. MyPaint brushes use denormalized option strength and curves, which makes the GUI a bit more complicated. It would be nice to compare the ranges in Krita with the ranges in MyPaint
    • check if the range in the top “Base value” slider. It should be not smaller than in MyPaint
    • check the range of “Y-limit” slider for every MyPaint option. It should be not smaller than in MyPaint
    • check the ranges of “X min” and “X max” sliders for every MyPaint sensor (they are the same in all the options).The total range should be not smaller than in MyPaint. Please take it into account that in Kirta you cannot make “X min” higher than 0.0 and “X max” lower than 1.0. That is an expected behavior. If you think it is wrong, please yell! :slight_smile:
  4. Notice that the curve control points are floating-point values in MyPaint (in constrast to Krita’s sensors, which use integer values). Check if they behave in a sane way.
  5. Check if the “Instant Preview” checkbox still works correctly. It should react on the following options:
    • brush size
    • and “Random” sensor in any option
    • when “Offset by Random” has value higher than 0.05
    • when “Radius by Random” has value higher than 0.05
  6. Check Experiment brush engine
  7. Check Quick brush engine
  8. Check Curve brush engine

Illustration:

image
image836×644 41.3 KB

4 Likes
28

Just a quick report- I think commit 0b3a70dd “Fix a multithreaded DB access in PredefinedBrushData’s ctor” caused a bug where spacing is broken after modifying a brush.
Before and after slightly modifying the diameter of of “Basic-3 Flow” from the brush editor, on the package above:

spacing-bug
spacing-bug640×485 11.7 KB

29

Hi, @freyalupen!

Thank you for your report. I have fixed the issue. Only Auto-brushes were affected. Here are the updated packages:

1 Like
30

Hi, @freyalupen!

I think I have finally fixed that predefined brush spacing issue. Thank you for the quick report yesterday! :slight_smile:

2 Likes
31

Yes, the edge case with having a predefined brush loaded on startup and then switching to an auto brush is now fixed.

I spent a little time going through the MyPaint options attempting to figure out what they do, and they seemed to work properly from what I could tell.
The one thing I did notice was that when I saved a MyPaint brush in the Lager version and attempted to load it in 5.1.3, some of the values did not get read. The Radius, Hardness, and Opacity are set to the minimum (0.01, 0.02, and 0.00).

  1. The Instant Preview checkbox behaves as expected, being disabled by brush size and giving a warning with random options.

      1. The Shape, Quick, and Curve engines work as expected.
32

Hi, @freyalupen!

…when I saved a MyPaint brush in the Lager version and attempted to load it in 5.1.3, some of the values did not get read. The Radius, Hardness, and Opacity are set to the minimum (0.01, 0.02, and 0.00)

I have fixed the issue locally. If you need a package, please let me know for what OS you need it :slight_smile:

33

Hi, all!

We joint the efforts of all Krita developers and finally finished porting the entire Krita’s brush editor into the Lager framework! Now the branch is ready for final testing and merging!

Please test the brush editor for the last time and we will merge the branch into master! :slight_smile:

Please pay additional attention to Spray and Grid brushes, especially to the “Proportional” property of the “Spray Shape” option.

Packages:

PS:

Special thanks for the help with finishing the branch to @freyalupen and @tiar! :slight_smile:

13 Likes
35

Yey \o/ Good job everybody!

36

A few minor things:

  • When loading the default Spray brush “spraybrush” (using the +), there’s SAFE ASSERT (krita): "data" in file C:/dev/env-4-lager/krita/plugins/paintops/libpaintop/kis_brush_option_widget.cpp, line 109.
  • The Spray engine’s Spray Shape width/height previously only went to 100% proportional, now it goes to 1000%. I’m not sure there’s any problem with that, though.
  • Logged at startup is kf5.ki18n: Trying to convert empty KLocalizedString to QString.x6 from some placeholder i18n("")s in MyPaintSensorPack.cpp.

And something which is apparently an existing bug on Stable, but I thought I’d mention it anyway; using a Sketch brush with the drawing angle sensor prints many krita.general: KisPaintInformation::drawingAngleSafe() DirectionHistoryInfo object is not available.

1 Like
38

Do we expect performance improvements in this port?

39

As far as I know, there is no radical performance improvement, there is some but @dkazakov can answer you better. This port enables the GUI of the brush editor and the backend to be decoupled

1 Like
40

For what I understood the testes are to see if everything is running as it should