Note, I finally managed to harmonize all toolbar buttons and icons sizes, yay! :-D

Link to the merge request for the first prototype:

From IRC:

We seemed to have removed Karbon's gradients: https://invent.kde.org/graphics/krita/-/commit/91c359e1977a8c5ff5c6f5be1c5bd618baafc5f2

Ok, so with the stop gradients icc-color merged, and with the HDR gradients patch merged, Krita can now do StopGradients in all the colorspaces it supports, including unbounded color gradients (So rec2020 12.0, 0.5, 0.5 is now possible).

I finally gave up trying to set the size of all the buttons in KToolBar, there must be a way to do it but it's definitely beyond my very limited c++ skills...
I'll let someone more knowledgeable do it.

I'm personally OK with using standard sizes in other places ;)

Well, as I said on IRC I'm fine with making toolbar "an exception" and making all the widgets fixed size. But for other places in Krita, we would better use standard sizes.

To complete my previous comment, I would add that there are also several other component on the toolbar that have a defined size that looks best if all the buttons are 32x32:
-The Gradient button and the Patterns button (which are 32x32)
-The Foreground/Background colors widget (this one is 28x28, but looks good when components around are 32x32)
-The blending mode combobox, and the sliders (they have a fixed height of 32)

Thanks for the feedback :)
Though actually I would prefer to do the other way around, that is change KToolBar to use the same values as we have in the paintopbox, and in the paintopbox do both setFixedSize(32,32) and setIconSize(22,22). This looks much better I feel, as then the buttons are the same height as the combobox and as the sliders (else, with just setting the icon size, the buttons are a bit smaller than the sliders with fusion). But for this I need to dig more into KToolBar to see how to properly set the size of buttons, and the height of the toolbar itself (actually the height of the toolbar is adapted automatically, I was confused thinking it would not by setting its size by mistake in my tests...)

Okay, I have spent some time on investigating into the code. It looks like for the toolbar we need to use values from KToolBar. It has a complicated system for determining the icon size. I still don't know how to fetch this value. (I haven't checked actually).

While I agree that "technically" it would be nice to can use those QStyle values, in practice it doesn't seem very good...

I believe that in all the places where we use custom sizes (except of the few really custom widgets, like layers docker and onion skins widget), we should try to use values like style()->pixelMetric(QStyle::PM_ToolBarIconSize) instead. I didn't do any good research for it though. We used custom values because we were too lazy or didn't know about existence of QStyle-defined values. So I believe we can just replace them in most of the cases.

I first made this proposal after noticing that we already define our own sizes for buttons in several places, and so thought that it could make sense to do it everywhere the same way to be more consistent.

We should probably not reuse Qt-provided constants

Documented KPL (which was on my todo forever as well...)

Then I think we should do that.

In T14093#249262, @rempt wrote:

Could we just ignore the parts of the standards with weird colorspaces and refuse to load gradients that use those?

Could we just ignore the parts of the standards with weird colorspaces and refuse to load gradients that use those?

Inkscape's way of handling lab and xyz is rather strange :/

Possible solutions:

Thanks Tusooa for your hard and useful work. I think Snapshot docker is a good addition for complex images.
For your week 6 if you think this could be doable:
I used that in PS long time ago and i remember there was a feature that allowed you to paint from history. (history mean ,snapshot) It is only a note if you are going to work more in this feature with more options. Right now in my tests works pretty well. Again Thanks

I did some tests just now regarding rotation in the brush texture:

Allow KoPatterns to carry a QTransform, this is necessary here because svg patterns will have their own transforms, so we should load those in and use them as default values for pattern transforms.

PoC
https://invent.kde.org/snippets/928

Okay, having slept on it, and looking at the code again, Cairo is not doing a per pixel transformation, but rather is just passing on the matrix to... wherever, and I for the live of me cannot find the place where the transform is applied to the pixel. I do suspect the that per-pixel transform might be a strategy to pursue. (so, pixel to paint x, invert transform on x, what do we find in the original tiled pattern at that location, paint that at x), but I am not smart enough to understand how we'd apply transform filters here.

https://gitlab.com/inkscape/inkscape/blob/32e9ba119663b9d5800392ba34700053eaaa7f90/src%2Fsp-pattern.cpp#L526 < inkscape's pattern implementation

In T10901#193043, @tusooaw wrote:

https://invent.kde.org/tusooaw/krita/commit/24355db3272a02230a14217da6023a953ec8f119

The crash does not come from KisCanvas2 but KisNode actually, as I said that it happens only when the undo command is not created (so KisNode is deleted by KisNodeReplaceBasedStrokeStrategy::Private's destructor, which is executed in the image thread (by the destructor of KisStroke)).

Ref: https://invent.kde.org/tusooaw/krita/blob/24355db3272a02230a14217da6023a953ec8f119/libs/global/kis_thread_safe_signal_compressor.h

The crash does not come from KisCanvas2 but KisNode actually, as I said that it happens only when the undo command is not created (so KisNode is deleted by KisNodeReplaceBasedStrokeStrategy::Private's destructor, which is executed in the image thread (by the destructor of KisStroke)).

In T10901#192951, @tusooaw wrote:

On the crash upon deletion of stroke strategy without creating an undo command:

It happens only when (1) we are not using debugger (so it is probably some timing issue); (2) the mouse is released immediately after pressing; and (3) no undo command is created.

https://invent.kde.org/snippets/335 indicates that the problem probably lies in KisCanvas2, whose m_d->canvasUpdateCompressor is of type KisSignalCompressor instead of the thread-safe one.

But I did also get another error message complaining from QObject::~QObject().

On the crash upon deletion of stroke strategy without creating an undo command:

A note on KisNode::copyFromNode():

Currently the methods return a KUndo2Command that will do the changes. However, it should be possible to make them only do the changes, by taking a clone of the active layer before making undoable changes to it.

Refactor out q-pointers and derived d-pointers in Flake