[graphics/krita] plugins/paintops: [critical] Fixed opacity being applied twice(!) in some brushes
Dmitry Kazakov
null at kde.org
Fri Dec 13 12:13:32 GMT 2024
Git commit ea2372b28fbde357ed65ad332777ae0a4eb9d775 by Dmitry Kazakov.
Committed on 13/12/2024 at 12:13.
Pushed by dkazakov into branch 'master'.
[critical] Fixed opacity being applied twice(!) in some brushes
It seem like during the brushes port to lager in 2022, I made a
mistake in understanding what is `m_useSeparateStrengthValue` in
`KisCurveOption`. It caused `KisOpacityOption` to apply opacity twice
for some of the brushes.
It means that for two years when the user selected any opacity for
the brush, the actual opacity used for the brush was **squared**.
I.e. when the user selected opacity 0.5, the real opacity was 0.25.
This patch fixes the issue, but it **changes behavior** of existing
brush presets.
The list of affected brush engines:
* Curve
* Clone
* Deform
* Hairy
* Hatching
* Sketch
* Spray
The most popular brush engines, like Pixel and Color Smudge are **not**
affected.
I'm not sure this change should be backported to krita/5.2 since it
changes behavior.
BUG:466368
CC:kimageshop at kde.org
M +1 -1 plugins/paintops/colorsmudge/kis_colorsmudgeop.cpp
M +2 -3 plugins/paintops/curvebrush/kis_curve_paintop.cpp
M +2 -4 plugins/paintops/defaultpaintops/duplicate/kis_duplicateop.cpp
M +2 -3 plugins/paintops/deform/kis_deform_paintop.cpp
M +2 -3 plugins/paintops/hairy/kis_hairy_paintop.cpp
M +2 -3 plugins/paintops/hatching/kis_hatching_paintop.cpp
M +1 -1 plugins/paintops/libpaintop/KisFlowOpacityOption.cpp
M +17 -9 plugins/paintops/libpaintop/KisOpacityOption.cpp
M +6 -2 plugins/paintops/libpaintop/KisOpacityOption.h
M +2 -3 plugins/paintops/sketch/kis_sketch_paintop.cpp
M +2 -3 plugins/paintops/spray/kis_spray_paintop.cpp
https://invent.kde.org/graphics/krita/-/commit/ea2372b28fbde357ed65ad332777ae0a4eb9d775
diff --git a/plugins/paintops/colorsmudge/kis_colorsmudgeop.cpp b/plugins/paintops/colorsmudge/kis_colorsmudgeop.cpp
index a89324ef491..b4dfcd5a6d4 100644
--- a/plugins/paintops/colorsmudge/kis_colorsmudgeop.cpp
+++ b/plugins/paintops/colorsmudge/kis_colorsmudgeop.cpp
@@ -50,7 +50,7 @@ KisColorSmudgeOp::KisColorSmudgeOp(const KisPaintOpSettingsSP settings, KisPaint
, m_firstRun(true)
, m_sizeOption(settings.data())
, m_ratioOption(settings.data())
- , m_opacityOption(settings.data())
+ , m_opacityOption(settings.data(), node)
, m_spacingOption(settings.data())
, m_rateOption(settings.data())
, m_rotationOption(settings.data())
diff --git a/plugins/paintops/curvebrush/kis_curve_paintop.cpp b/plugins/paintops/curvebrush/kis_curve_paintop.cpp
index f5d3fd06331..e7284e77259 100644
--- a/plugins/paintops/curvebrush/kis_curve_paintop.cpp
+++ b/plugins/paintops/curvebrush/kis_curve_paintop.cpp
@@ -24,7 +24,7 @@
KisCurvePaintOp::KisCurvePaintOp(const KisPaintOpSettingsSP settings, KisPainter * painter, KisNodeSP node, KisImageSP image)
: KisPaintOp(painter)
- , m_opacityOption(settings.data())
+ , m_opacityOption(settings.data(), node)
, m_lineWidthOption(settings.data())
, m_curvesOpacityOption(settings.data())
, m_painter(0)
@@ -68,10 +68,9 @@ void KisCurvePaintOp::paintLine(const KisPaintInformation &pi1, const KisPaintIn
QRect rc = m_dab->extent();
- qreal origOpacity = m_opacityOption.apply(painter(), pi2);
+ m_opacityOption.apply(painter(), pi2);
painter()->bitBlt(rc.topLeft(), m_dab, rc);
painter()->renderMirrorMask(rc, m_dab);
- painter()->setOpacityF(origOpacity);
}
void KisCurvePaintOp::paintLine(KisPaintDeviceSP dab, const KisPaintInformation &pi1, const KisPaintInformation &pi2)
diff --git a/plugins/paintops/defaultpaintops/duplicate/kis_duplicateop.cpp b/plugins/paintops/defaultpaintops/duplicate/kis_duplicateop.cpp
index c74b987182c..6981c900b23 100644
--- a/plugins/paintops/defaultpaintops/duplicate/kis_duplicateop.cpp
+++ b/plugins/paintops/defaultpaintops/duplicate/kis_duplicateop.cpp
@@ -56,7 +56,7 @@ KisDuplicateOp::KisDuplicateOp(const KisPaintOpSettingsSP settings, KisPainter *
, m_node(node)
, m_settings(static_cast<KisDuplicateOpSettings*>(const_cast<KisPaintOpSettings*>(settings.data())))
, m_sizeOption(settings.data())
- , m_opacityOption(settings.data())
+ , m_opacityOption(settings.data(), node)
, m_rotationOption(settings.data())
{
Q_ASSERT(settings);
@@ -109,7 +109,7 @@ KisSpacingInformation KisDuplicateOp::paintAt(const KisPaintInformation& info)
qreal rotation = m_rotationOption.apply(info);
- qreal opacity = m_opacityOption.apply(painter(),info);
+ m_opacityOption.apply(painter(),info);
qreal scale = m_sizeOption.apply(info);
@@ -269,8 +269,6 @@ KisSpacingInformation KisDuplicateOp::paintAt(const KisPaintInformation& info)
painter()->renderMirrorMaskSafe(dstRect, m_srcdev, 0, 0, dab,
!m_dabCache->needSeparateOriginal());
- painter()->setOpacityF(opacity);
-
return effectiveSpacing(scale);
}
diff --git a/plugins/paintops/deform/kis_deform_paintop.cpp b/plugins/paintops/deform/kis_deform_paintop.cpp
index 99986586d1f..d0ad7caf9f1 100644
--- a/plugins/paintops/deform/kis_deform_paintop.cpp
+++ b/plugins/paintops/deform/kis_deform_paintop.cpp
@@ -40,7 +40,7 @@
KisDeformPaintOp::KisDeformPaintOp(const KisPaintOpSettingsSP settings, KisPainter * painter, KisNodeSP node, KisImageSP image)
: KisPaintOp(painter)
, m_sizeOption(settings.data())
- , m_opacityOption(settings.data())
+ , m_opacityOption(settings.data(), node)
, m_rotationOption(settings.data())
, m_rateOption(settings.data())
{
@@ -120,10 +120,9 @@ KisSpacingInformation KisDeformPaintOp::paintAt(const KisPaintInformation& info)
return updateSpacingImpl(info);
}
- qreal origOpacity = m_opacityOption.apply(painter(), info);
+ m_opacityOption.apply(painter(), info);
painter()->bltFixedWithFixedSelection(x, y, dab, mask, mask->bounds().width() , mask->bounds().height());
painter()->renderMirrorMask(QRect(QPoint(x, y), QSize(mask->bounds().width() , mask->bounds().height())), dab, mask);
- painter()->setOpacityF(origOpacity);
return updateSpacingImpl(info);
}
diff --git a/plugins/paintops/hairy/kis_hairy_paintop.cpp b/plugins/paintops/hairy/kis_hairy_paintop.cpp
index 713c802809f..e59bfcf8652 100644
--- a/plugins/paintops/hairy/kis_hairy_paintop.cpp
+++ b/plugins/paintops/hairy/kis_hairy_paintop.cpp
@@ -31,7 +31,7 @@
KisHairyPaintOp::KisHairyPaintOp(const KisPaintOpSettingsSP settings, KisPainter * painter, KisNodeSP node, KisImageSP image)
: KisPaintOp(painter)
- , m_opacityOption(settings.data())
+ , m_opacityOption(settings.data(), node)
, m_sizeOption(settings.data())
, m_rotationOption(settings.data())
{
@@ -139,7 +139,7 @@ void KisHairyPaintOp::paintLine(const KisPaintInformation &pi1, const KisPaintIn
qreal scale = m_sizeOption.apply(pi);
scale *= KisLodTransform::lodToScale(painter()->device());
qreal rotation = m_rotationOption.apply(pi);
- qreal origOpacity = m_opacityOption.apply(painter(), pi);
+ m_opacityOption.apply(painter(), pi);
const bool mirrorFlip = pi1.canvasMirroredH() != pi1.canvasMirroredV();
@@ -153,7 +153,6 @@ void KisHairyPaintOp::paintLine(const KisPaintInformation &pi1, const KisPaintIn
QRect rc = m_dab->extent();
painter()->bitBlt(rc.topLeft(), m_dab, rc);
painter()->renderMirrorMask(rc, m_dab);
- painter()->setOpacityF(origOpacity);
// we don't use spacing in hairy brush, but history is
// still important for us
diff --git a/plugins/paintops/hatching/kis_hatching_paintop.cpp b/plugins/paintops/hatching/kis_hatching_paintop.cpp
index 4e0504e2041..0db7f04a13f 100644
--- a/plugins/paintops/hatching/kis_hatching_paintop.cpp
+++ b/plugins/paintops/hatching/kis_hatching_paintop.cpp
@@ -38,7 +38,7 @@ KisHatchingPaintOp::KisHatchingPaintOp(const KisPaintOpSettingsSP settings, KisP
, m_crosshatchingOption(settings.data())
, m_separationOption(settings.data())
, m_thicknessOption(settings.data())
- , m_opacityOption(settings.data())
+ , m_opacityOption(settings.data(), node)
, m_sizeOption(settings.data())
{
Q_UNUSED(node);
@@ -89,7 +89,7 @@ KisSpacingInformation KisHatchingPaintOp::paintAt(const KisPaintInformation& inf
if ((scale * brush->width()) <= 0.01 || (scale * brush->height()) <= 0.01) return KisSpacingInformation(1.0);
KisDabShape shape(scale, 1.0, 0.0);
- qreal origOpacity = m_opacityOption.apply(painter(), info);
+ m_opacityOption.apply(painter(), info);
/*----Fetch the Dab----*/
static const KoColorSpace *cs = KoColorSpaceRegistry::instance()->alpha8();
@@ -166,7 +166,6 @@ KisSpacingInformation KisHatchingPaintOp::paintAt(const KisPaintInformation& inf
painter()->bitBltWithFixedSelection(x, y, m_hatchedDab, maskDab, sw, sh);
painter()->renderMirrorMaskSafe(QRect(QPoint(x, y), QSize(sw, sh)), m_hatchedDab, 0, 0, maskDab,
!m_dabCache->needSeparateOriginal());
- painter()->setOpacityF(origOpacity);
return effectiveSpacing(scale);
}
diff --git a/plugins/paintops/libpaintop/KisFlowOpacityOption.cpp b/plugins/paintops/libpaintop/KisFlowOpacityOption.cpp
index bfb6aa12b50..affe41c6405 100644
--- a/plugins/paintops/libpaintop/KisFlowOpacityOption.cpp
+++ b/plugins/paintops/libpaintop/KisFlowOpacityOption.cpp
@@ -14,7 +14,7 @@
KisFlowOpacityOption2::KisFlowOpacityOption2(const KisPropertiesConfiguration *setting, KisNodeSP currentNode)
- : m_opacityOption(setting),
+ : m_opacityOption(setting, currentNode),
m_flowOption(setting)
{
if (currentNode &&
diff --git a/plugins/paintops/libpaintop/KisOpacityOption.cpp b/plugins/paintops/libpaintop/KisOpacityOption.cpp
index 7bad8b1d75a..8c43ea7ff2d 100644
--- a/plugins/paintops/libpaintop/KisOpacityOption.cpp
+++ b/plugins/paintops/libpaintop/KisOpacityOption.cpp
@@ -5,18 +5,26 @@
*/
#include "KisOpacityOption.h"
+#include <kis_properties_configuration.h>
#include <kis_painter.h>
+#include <kis_node.h>
+#include <kis_indirect_painting_support.h>
-qreal KisOpacityOption::apply(KisPainter* painter, const KisPaintInformation& info) const
+
+KisOpacityOption::KisOpacityOption(const KisPropertiesConfiguration *setting, KisNodeSP currentNode)
+ : BaseClass(setting)
{
- if (!isChecked()) {
- return painter->opacityF();
- }
- qreal origOpacity = painter->opacityF();
+ if (currentNode &&
+ setting->getString(KisPropertiesConfiguration::extractedPrefixKey()).isEmpty()) {
- qreal opacity = origOpacity * computeSizeLikeValue(info);
- qreal opacity2 = qBound<qreal>(OPACITY_TRANSPARENT_F, opacity, OPACITY_OPAQUE_F);
+ KisIndirectPaintingSupport *indirect =
+ dynamic_cast<KisIndirectPaintingSupport*>(currentNode.data());
+ m_indirectPaintingActive = indirect && indirect->hasTemporaryTarget();
+ }
+}
- painter->setOpacityUpdateAverage(opacity2);
- return origOpacity;
+void KisOpacityOption::apply(KisPainter* painter, const KisPaintInformation& info) const
+{
+ const qreal opacity = isChecked() ? computeSizeLikeValue(info, !m_indirectPaintingActive) : 1.0;
+ painter->setOpacityUpdateAverage(opacity);
}
diff --git a/plugins/paintops/libpaintop/KisOpacityOption.h b/plugins/paintops/libpaintop/KisOpacityOption.h
index c50826ecabf..452d018555d 100644
--- a/plugins/paintops/libpaintop/KisOpacityOption.h
+++ b/plugins/paintops/libpaintop/KisOpacityOption.h
@@ -15,10 +15,14 @@ class PAINTOP_EXPORT KisOpacityOption : public KisStandardOption<KisOpacityOptio
public:
using BaseClass = KisStandardOption<KisOpacityOptionData>;
- using BaseClass::BaseClass;
+ KisOpacityOption(const KisPropertiesConfiguration *setting, KisNodeSP currentNode);
+
using BaseClass::apply;
- qreal apply(KisPainter* painter, const KisPaintInformation& info) const;
+ void apply(KisPainter* painter, const KisPaintInformation& info) const;
+
+private:
+ bool m_indirectPaintingActive = false;
};
#endif // KISOPACITYOPTION_H
diff --git a/plugins/paintops/sketch/kis_sketch_paintop.cpp b/plugins/paintops/sketch/kis_sketch_paintop.cpp
index b493702c882..2b50f4d1399 100644
--- a/plugins/paintops/sketch/kis_sketch_paintop.cpp
+++ b/plugins/paintops/sketch/kis_sketch_paintop.cpp
@@ -53,7 +53,7 @@
KisSketchPaintOp::KisSketchPaintOp(const KisPaintOpSettingsSP settings, KisPainter *painter, KisNodeSP node, KisImageSP image)
: KisPaintOp(painter)
- , m_opacityOption(settings.data())
+ , m_opacityOption(settings.data(), node)
, m_sizeOption(settings.data())
, m_rotationOption(settings.data())
, m_rateOption(settings.data())
@@ -299,11 +299,10 @@ void KisSketchPaintOp::doPaintLine(const KisPaintInformation &pi1, const KisPain
m_count++;
QRect rc = m_dab->extent();
- qreal origOpacity = m_opacityOption.apply(painter(), pi2);
+ m_opacityOption.apply(painter(), pi2);
painter()->bitBlt(rc.x(), rc.y(), m_dab, rc.x(), rc.y(), rc.width(), rc.height());
painter()->renderMirrorMask(rc, m_dab);
- painter()->setOpacityF(origOpacity);
}
diff --git a/plugins/paintops/spray/kis_spray_paintop.cpp b/plugins/paintops/spray/kis_spray_paintop.cpp
index 50682c026e7..0a54bf064b1 100644
--- a/plugins/paintops/spray/kis_spray_paintop.cpp
+++ b/plugins/paintops/spray/kis_spray_paintop.cpp
@@ -30,7 +30,7 @@ KisSprayPaintOp::KisSprayPaintOp(const KisPaintOpSettingsSP settings, KisPainter
, m_isPresetValid(true)
, m_rotationOption(settings.data())
, m_sizeOption(settings.data())
- , m_opacityOption(settings.data())
+ , m_opacityOption(settings.data(), node)
, m_rateOption(settings.data())
, m_node(node)
@@ -97,7 +97,7 @@ KisSpacingInformation KisSprayPaintOp::paintAt(const KisPaintInformation& info)
}
qreal rotation = m_rotationOption.apply(info);
- qreal origOpacity = m_opacityOption.apply(painter(), info);
+ m_opacityOption.apply(painter(), info);
// Spray Brush is capable of working with zero scale,
// so no additional checks for 'zero'ness are needed
const qreal scale = m_sizeOption.apply(info);
@@ -115,7 +115,6 @@ KisSpacingInformation KisSprayPaintOp::paintAt(const KisPaintInformation& info)
QRect rc = m_dab->extent();
painter()->bitBlt(rc.topLeft(), m_dab, rc);
painter()->renderMirrorMask(rc, m_dab);
- painter()->setOpacityF(origOpacity);
return computeSpacing(info, lodScale);
}
More information about the kimageshop
mailing list