Commit 6d87f894 authored by Dmitry Kazakov's avatar Dmitry Kazakov
Browse files

Possible fix for the artifacts when using the line tool

There was a theoretical race condition possible in


1) The image has only one layer with fully transparent default pixel
   (to trigget tryOblidgeChild mechanism)

Mecanics of the issue:

1) KisAsyncMerger requests currentLeaf->parent()->original(), which points
   to the projection() of the only child layer (which equals to
   its paintDevice().

2) KisAsyncMerger thread sleeps

3) Some other thread starts an indirect painting stroke over this
   layer, basically changing projection() of the layer to be separate
   from its paintDevice()

4) KisAsyncMerger thread wakes up

5) The condition `parentOriginal != currentLeaf->projection()` succeeds
   because parentOriginal still points to the layer's paintDevice

6) KisAsyncMerger starts to write into paint layer's paint device,
   which breaks its undo history.

Lots of thanks go to freyalupen for debugging this issue locally and
providing the valuable debugging info.

parent fac1d60b
Pipeline #256418 passed with stage
in 52 minutes and 32 seconds
......@@ -311,9 +311,9 @@ void KisAsyncMerger::resetProjection() {
void KisAsyncMerger::setupProjection(KisProjectionLeafSP currentLeaf, const QRect& rect, bool useTempProjection) {
KisPaintDeviceSP parentOriginal = currentLeaf->parent()->original();
KisPaintDeviceSP parentOriginal = currentLeaf->parent()->lazyDestinationForSubtreeComposition();
if (parentOriginal != currentLeaf->projection()) {
if (parentOriginal) {
if (useTempProjection) {
m_cachedPaintDevice = new KisPaintDevice(parentOriginal->colorSpace());
......@@ -42,6 +42,8 @@ public:
qint32 x;
qint32 y;
bool passThroughMode;
std::tuple<KisPaintDeviceSP, bool> originalImpl() const;
KisGroupLayer::KisGroupLayer(KisImageWSP image, const QString &name, quint8 opacity) :
......@@ -252,7 +254,7 @@ KisPaintDeviceSP KisGroupLayer::tryObligeChild() const
return 0;
KisPaintDeviceSP KisGroupLayer::original() const
std::tuple<KisPaintDeviceSP, bool> KisGroupLayer::originalImpl() const
* We are too lazy! Let's our children work for us.
......@@ -260,15 +262,31 @@ KisPaintDeviceSP KisGroupLayer::original() const
* one in stack and meets some conditions
KisPaintDeviceSP realOriginal = tryObligeChild();
bool ownsOriginal = false;
if (!realOriginal) {
if (!childCount() && !m_d->paintDevice->extent().isEmpty()) {
realOriginal = m_d->paintDevice;
ownsOriginal = true;
return realOriginal;
return std::make_tuple(realOriginal, ownsOriginal);
KisPaintDeviceSP KisGroupLayer::original() const
return std::get<0>(originalImpl());
KisPaintDeviceSP KisGroupLayer::lazyDestinationForSubtreeComposition() const
KisPaintDeviceSP originalDev;
bool ownsOriginal = false;
std::tie(originalDev, ownsOriginal) = originalImpl();
return ownsOriginal ? originalDev : nullptr;
QRect KisGroupLayer::amortizedProjectionRectForCleanupInChangePass() const
......@@ -59,6 +59,13 @@ public:
/// @return the projection of the layers in the group before the masks are applied.
KisPaintDeviceSP original() const override;
* Returns own original device when tryOblidgeChild() mechanism is not triggered.
* When tryOblidgeChild() mechanism is in action, returns null (therefor
* threre is no need to do subtree composition).
KisPaintDeviceSP lazyDestinationForSubtreeComposition() const;
qint32 x() const override;
qint32 y() const override;
void setX(qint32 x) override;
......@@ -98,6 +105,7 @@ public:
KisLayer* onlyMeaningfulChild() const;
KisPaintDeviceSP tryObligeChild() const;
std::tuple<KisPaintDeviceSP, bool> originalImpl() const;
QRect amortizedProjectionRectForCleanupInChangePass() const override;
......@@ -245,6 +245,12 @@ KisPaintDeviceSP KisProjectionLeaf::projection()
return m_d->node->projection();
KisPaintDeviceSP KisProjectionLeaf::lazyDestinationForSubtreeComposition()
const KisGroupLayer *group = qobject_cast<const KisGroupLayer*>(m_d->;
return group ? group->lazyDestinationForSubtreeComposition() : nullptr;
bool KisProjectionLeaf::isRoot() const
return (bool)!m_d->node->parent();
......@@ -35,6 +35,7 @@ public:
KisPaintDeviceSP original();
KisPaintDeviceSP projection();
KisPaintDeviceSP lazyDestinationForSubtreeComposition();
bool isRoot() const;
bool isLayer() const;
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment