Commit 607fca8d authored by Dmitry Kazakov's avatar Dmitry Kazakov
Browse files

Fix updates when transforming a group layer with a tranparency mask

Before the patch, the updates were issues by individual transformation
commands, that leads to weird results when the group is under an effect
of a transparency mask. So now the updates are gathered in buckets and
are emitted all at once.

The patch is still a bit clumsy, because undo/redo do not call
image->disableDirtyRequests()/enableDirtyRequests(), whereas the
main transformation action does. Though it shouldn't cause too many
issues, just a bit more updates than needed. It will still be better
than before.

BUG:431756
parent adc22b13
......@@ -117,6 +117,7 @@ set(kritaimage_LIB_SRCS
commands_new/KisMergeLabeledLayersCommand.cpp
commands_new/KisAsynchronouslyMergeableCommandInterface.cpp
commands_new/KisSimpleModifyTransformMaskCommand.cpp
commands_new/KisUpdateCommandEx.cpp
processing/kis_do_nothing_processing_visitor.cpp
processing/kis_simple_processing_visitor.cpp
processing/kis_convert_color_space_processing_visitor.cpp
......
/*
* SPDX-FileCopyrightText: 2021 Dmitry Kazakov <dimula73@gmail.com>
*
* SPDX-License-Identifier: GPL-2.0-or-later
*/
#include "KisUpdateCommandEx.h"
#include "kis_image_interfaces.h"
#include "kis_node.h"
KisUpdateCommandEx::KisUpdateCommandEx(SharedDataSP updateData,
KisUpdatesFacade *updatesFacade,
State state,
QWeakPointer<boost::none_t> blockUpdatesCookie)
: FlipFlopCommand(state),
m_updateData(updateData),
m_blockUpdatesCookie(blockUpdatesCookie),
m_updatesFacade(updatesFacade)
{
}
KisUpdateCommandEx::~KisUpdateCommandEx()
{
}
KisUpdateCommandEx::KisUpdateCommandEx(SharedDataSP updateData, KisUpdatesFacade *updatesFacade, State state)
: KisUpdateCommandEx(updateData, updatesFacade, state, QWeakPointer<boost::none_t>())
{
}
void KisUpdateCommandEx::partB() {
if (m_blockUpdatesCookie) return;
for (auto it = m_updateData->begin(); it != m_updateData->end(); ++it) {
m_updatesFacade->refreshGraphAsync(it->first, it->second);
}
}
/*
* SPDX-FileCopyrightText: 2021 Dmitry Kazakov <dimula73@gmail.com>
*
* SPDX-License-Identifier: GPL-2.0-or-later
*/
#ifndef KISUPDATECOMMANDEX_H
#define KISUPDATECOMMANDEX_H
#include <kritaimage_export.h>
#include <kis_command_utils.h>
#include <kis_types.h>
#include <boost/none.hpp>
class KisUpdatesFacade;
class KRITAIMAGE_EXPORT KisUpdateCommandEx : public KisCommandUtils::FlipFlopCommand
{
public:
using SharedData = std::vector<std::pair<KisNodeSP, QRect>>;
using SharedDataSP = QSharedPointer<SharedData>;
public:
KisUpdateCommandEx(SharedDataSP updateData,
KisUpdatesFacade *updatesFacade,
State state);
KisUpdateCommandEx(SharedDataSP updateData,
KisUpdatesFacade *updatesFacade,
State state,
QWeakPointer<boost::none_t> blockUpdatesCookie);
~KisUpdateCommandEx();
void partB() override;
private:
SharedDataSP m_updateData;
QWeakPointer<boost::none_t> m_blockUpdatesCookie;
KisUpdatesFacade *m_updatesFacade;
};
#endif // KISUPDATECOMMANDEX_H
......@@ -504,7 +504,12 @@ void InplaceTransformStrokeStrategy::notifyAllCommandsDone()
for (auto it = m_d->commands.begin(); it != m_d->commands.end(); ++it) {
if (it->first == Transform) {
notifyCommandDone(it->second, KisStrokeJobData::CONCURRENT, KisStrokeJobData::NORMAL);
KisStrokeJobData::Sequentiality seq =
it->second.dynamicCast<KisUpdateCommandEx>() ?
KisStrokeJobData::BARRIER : KisStrokeJobData::CONCURRENT;
notifyCommandDone(it->second, seq, KisStrokeJobData::NORMAL);
}
}
}
......@@ -539,16 +544,35 @@ void InplaceTransformStrokeStrategy::undoTransformCommands(int levelOfDetail)
}
}
void InplaceTransformStrokeStrategy::postAllUpdates(int levelOfDetail)
void InplaceTransformStrokeStrategy::postAllUpdates(int levelOfDetail, KisUpdateCommandEx::SharedDataSP updateData)
{
QHash<KisNodeSP, QRect> &dirtyRects = m_d->effectiveDirtyRects(levelOfDetail);
QHash<KisNodeSP, QRect> &prevDirtyRects = m_d->effectivePrevDirtyRects(levelOfDetail);
Q_FOREACH (KisNodeSP node, m_d->processedNodes) {
const QRect dirtyRect = dirtyRects[node] | prevDirtyRects[node];
KisNodeList nodes = KisLayerUtils::sortAndFilterMergableInternalNodes(m_d->processedNodes, true);
Q_FOREACH (KisNodeSP node, nodes) {
QRect dirtyRect;
KisLayerUtils::recursiveApplyNodes(node,
[&dirtyRect, &dirtyRects, &prevDirtyRects] (KisNodeSP node) {
KIS_SAFE_ASSERT_RECOVER_NOOP(dirtyRects.contains(node) == prevDirtyRects.contains(node));
if (dirtyRects.contains(node)) {
dirtyRect |= dirtyRects[node];
}
if (prevDirtyRects.contains(node)) {
dirtyRect |= prevDirtyRects[node];
}
});
if (dirtyRect.isEmpty()) continue;
m_d->updatesFacade->refreshGraphAsync(node, dirtyRect);
updateData->push_back(std::make_pair(node, dirtyRect));
}
prevDirtyRects.clear();
......@@ -740,11 +764,14 @@ void InplaceTransformStrokeStrategy::reapplyTransform(ToolTransformArgs args,
args.scale3dSrcAndDst(KisLodTransform::lodToScale(levelOfDetail));
}
KisUpdateCommandEx::SharedDataSP updateData(new KisUpdateCommandEx::SharedData());
KritaUtils::addJobBarrier(mutatedJobs, levelOfDetail,
[this, args, levelOfDetail]() {
[this, args, levelOfDetail, updateData]() {
m_d->updatesFacade->disableDirtyRequests();
m_d->updatesDisabled = true;
undoTransformCommands(levelOfDetail);
executeAndAddCommand(new KisUpdateCommandEx(updateData, m_d->updatesFacade, KisUpdateCommandEx::INITIALIZING, m_d->commandUpdatesBlockerCookie), Transform);
});
Q_FOREACH (KisNodeSP node, m_d->processedNodes) {
......@@ -754,10 +781,12 @@ void InplaceTransformStrokeStrategy::reapplyTransform(ToolTransformArgs args,
});
}
KritaUtils::addJobBarrier(mutatedJobs, levelOfDetail, [this, levelOfDetail]() {
KritaUtils::addJobBarrier(mutatedJobs, levelOfDetail, [this, levelOfDetail, updateData]() {
executeAndAddCommand(new KisUpdateCommandEx(updateData, m_d->updatesFacade, KisUpdateCommandEx::FINALIZING, m_d->commandUpdatesBlockerCookie), Transform);
m_d->updatesFacade->enableDirtyRequests();
m_d->updatesDisabled = false;
postAllUpdates(levelOfDetail);
postAllUpdates(levelOfDetail, updateData);
});
}
......
......@@ -19,6 +19,7 @@
#include <transform_transaction_properties.h>
#include "KisAsyncronousStrokeUpdateHelper.h"
#include <commands_new/KisUpdateCommandEx.h>
class KisPostExecutionUndoAdapter;
class TransformTransactionProperties;
......@@ -78,6 +79,33 @@ private:
public:
/**
* The transformation pipeline usually looks like that:
*
* 1) Apply Clear commands for all the layers. Some clear commands might
* be "temporary", that is, they do not go to the final history, e.g. when
* clearing a shape layer's projection.
*
* 2) Apply TransoformLod commands to generate preview of the
* transformation. Some commands may be declared as "temporary", that is,
* they do not go to the final history, e.g. for the shape layer, for
* which we just write to the projection device explicitly.
*
* 3) When transformation is changed we undo all TransformLod and
* TransformTemporary commands to recover the old state. The temporary
* command recovers the state of shape layers' projection device.
*
* 4) Repeat steps 2) and 3) until the user is satisfied.
*
* 5) When "Apply" button is pressed, all transform commands are undone
* like in step 2).
*
* 6) All Transform commands are applied at Lod0-level. TransformTemporary
* is not used atm.
*
* 7) All non-temporary commands go to the undo history.
*/
enum CommandGroup {
Clear = 0,
ClearTemporary,
......@@ -127,7 +155,7 @@ private:
void undoAllCommands();
void undoTransformCommands(int levelOfDetail);
void postAllUpdates(int levelOfDetail);
void postAllUpdates(int levelOfDetail, KisUpdateCommandEx::SharedDataSP updateData);
void transformNode(KisNodeSP node, const ToolTransformArgs &config, int levelOfDetail);
void createCacheAndClearNode(KisNodeSP node);
......
......@@ -41,6 +41,8 @@
#include "KisRunnableStrokeJobUtils.h"
#include "commands_new/KisHoldUIUpdatesCommand.h"
#include "KisDecoratedNodeInterface.h"
#include "kis_paint_device_debug_utils.h"
#include "kis_layer_utils.h"
TransformStrokeStrategy::TransformStrokeStrategy(ToolTransformArgs::TransformMode mode,
......@@ -262,7 +264,7 @@ void TransformStrokeStrategy::doStrokeCallback(KisStrokeJobData *data)
KisStrokeJobData::CONCURRENT,
KisStrokeJobData::NORMAL);
td->node->setDirty(oldExtent | td->node->extent());
m_updateData->push_back(std::make_pair(td->node, oldExtent | td->node->extent()));
} else if (KisExternalLayer *extLayer =
dynamic_cast<KisExternalLayer*>(td->node.data())) {
......@@ -270,6 +272,8 @@ void TransformStrokeStrategy::doStrokeCallback(KisStrokeJobData *data)
(td->config.mode() == ToolTransformArgs::PERSPECTIVE_4POINT &&
extLayer->supportsPerspectiveTransform())) {
QRect oldDirtyRect = oldExtent | extLayer->theoreticalBoundingRect();
QVector3D transformedCenter;
KisTransformWorker w = KisTransformUtils::createTransformWorker(td->config, 0, 0, &transformedCenter);
QTransform t = w.transform();
......@@ -277,6 +281,16 @@ void TransformStrokeStrategy::doStrokeCallback(KisStrokeJobData *data)
runAndSaveCommand(KUndo2CommandSP(extLayer->transform(t)),
KisStrokeJobData::CONCURRENT,
KisStrokeJobData::NORMAL);
/**
* Shape layer's projection may not be yet ready right
* after transformation, because it need to do that in
* the GUI thread, so we should approximate that.
*/
const QRect theoreticalNewDirtyRect =
kisGrowRect(t.mapRect(oldDirtyRect), 1);
m_updateData->push_back(std::make_pair(td->node, oldDirtyRect | td->node->extent() | extLayer->theoreticalBoundingRect() | theoreticalNewDirtyRect));
}
} else if (KisTransformMask *transformMask =
......@@ -288,6 +302,8 @@ void TransformStrokeStrategy::doStrokeCallback(KisStrokeJobData *data)
new KisTransformMaskAdapter(td->config)))),
KisStrokeJobData::CONCURRENT,
KisStrokeJobData::NORMAL);
m_updateData->push_back(std::make_pair(td->node, oldExtent | td->node->extent()));
}
} else if (m_selection) {
......@@ -323,12 +339,10 @@ void TransformStrokeStrategy::doStrokeCallback(KisStrokeJobData *data)
if (selection) {
selection->setVisible(false);
m_deactivatedSelections.append(selection);
mask->setDirty();
}
}
} else if (KisExternalLayer *externalLayer = dynamic_cast<KisExternalLayer*>(csd->node.data())) {
externalLayer->projectionLeaf()->setTemporaryHiddenFromRendering(true);
externalLayer->setDirty();
m_hiddenProjectionLeaves.append(csd->node);
} else if (KisTransformMask *transformMask =
dynamic_cast<KisTransformMask*>(csd->node.data())) {
......@@ -351,9 +365,7 @@ void TransformStrokeStrategy::clearSelection(KisPaintDeviceSP device)
if (m_selection) {
device->clearSelection(m_selection);
} else {
QRect oldExtent = device->extent();
device->clear();
device->setDirty(oldExtent);
}
runAndSaveCommand(KUndo2CommandSP(transaction.endAndTake()),
KisStrokeJobData::SEQUENTIAL,
......@@ -475,10 +487,21 @@ void TransformStrokeStrategy::initStrokeCallback()
extraInitJobs << new PreparePreviewData();
KisUpdateCommandEx::SharedDataSP sharedData(new KisUpdateCommandEx::SharedData());
KisNodeList filteredRoots = KisLayerUtils::sortAndFilterMergableInternalNodes(m_processedNodes, true);
Q_FOREACH (KisNodeSP root, filteredRoots) {
sharedData->push_back(std::make_pair(root, root->extent()));
}
extraInitJobs << new Data(new KisUpdateCommandEx(sharedData, m_updatesFacade, KisUpdateCommandEx::INITIALIZING), false, Data::BARRIER);
Q_FOREACH (KisNodeSP node, m_processedNodes) {
extraInitJobs << new ClearSelectionData(node);
}
extraInitJobs << new Data(new KisUpdateCommandEx(sharedData, m_updatesFacade, KisUpdateCommandEx::FINALIZING), false, Data::BARRIER);
/// recover back visibility of decorated nodes
KritaUtils::addJobBarrier(extraInitJobs, [this]() {
Q_FOREACH (KisDecoratedNodeInterface *decoratedNode, m_disabledDecoratedNodes) {
......@@ -534,6 +557,14 @@ void TransformStrokeStrategy::finishStrokeImpl(bool applyTransform, const ToolTr
if (applyTransform) {
m_savedTransformArgs = args;
m_updateData.reset(new KisUpdateCommandEx::SharedData());
KritaUtils::addJobBarrier(mutatedJobs, [this] () {
runAndSaveCommand(toQShared(new KisUpdateCommandEx(m_updateData, m_updatesFacade, KisUpdateCommandEx::INITIALIZING)), KisStrokeJobData::BARRIER, KisStrokeJobData::NORMAL);
m_updatesDisabled = true;
m_updatesFacade->disableDirtyRequests();
});
Q_FOREACH (KisNodeSP node, m_processedNodes) {
mutatedJobs << new TransformData(TransformData::PAINT_DEVICE,
args,
......@@ -542,6 +573,38 @@ void TransformStrokeStrategy::finishStrokeImpl(bool applyTransform, const ToolTr
mutatedJobs << new TransformData(TransformData::SELECTION,
args,
m_rootNode);
KritaUtils::addJobBarrier(mutatedJobs, [this] () {
{
/// Here is a bit of code-duplication from
/// InplaceTransformStrokeStrategy. This code reduced the
/// amount of updates we do in the course of transformation.
/// Basically, we find the minimum set of parents for the
/// selected layers and do full refresh for them
KisNodeList filteredNodes = m_processedNodes;
KisLayerUtils::sortAndFilterMergableInternalNodes(filteredNodes);
KisUpdateCommandEx::SharedData newUpdateData;
Q_FOREACH (KisNodeSP root, filteredNodes) {
QRect dirtyRect;
for (auto it = m_updateData->begin(); it != m_updateData->end(); ++it) {
if (KisLayerUtils::checkIsChildOf(it->first, {root})) {
dirtyRect |= it->second;
}
}
newUpdateData.push_back(std::make_pair(root, dirtyRect));
}
*m_updateData = newUpdateData;
}
m_updatesFacade->enableDirtyRequests();
m_updatesDisabled = false;
runAndSaveCommand(toQShared(new KisUpdateCommandEx(m_updateData, m_updatesFacade, KisUpdateCommandEx::FINALIZING)), KisStrokeJobData::BARRIER, KisStrokeJobData::NORMAL);
});
}
KritaUtils::addJobBarrier(mutatedJobs, [this, applyTransform]() {
......@@ -580,5 +643,9 @@ void TransformStrokeStrategy::finishStrokeCallback()
void TransformStrokeStrategy::cancelStrokeCallback()
{
if (m_updatesDisabled) {
m_updatesFacade->enableDirtyRequests();
}
finishStrokeImpl(!m_initialTransformArgs.isUnchanging(), m_initialTransformArgs);
}
......@@ -16,7 +16,7 @@
#include <kis_processing_visitor.h>
#include <kritatooltransform_export.h>
#include <boost/optional.hpp>
#include "commands_new/KisUpdateCommandEx.h"
class KisPostExecutionUndoAdapter;
class TransformTransactionProperties;
......@@ -116,6 +116,8 @@ private:
private:
KisUpdatesFacade *m_updatesFacade;
KisUpdateCommandEx::SharedDataSP m_updateData;
bool m_updatesDisabled = false;
ToolTransformArgs::TransformMode m_mode;
QString m_filterId;
bool m_forceReset;
......
Markdown is supported
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