[graphics/krita/krita/5.0] libs/pigment: Fix linking color space of Alpha color spaces
Dmitry Kazakov
null at kde.org
Mon Oct 11 15:19:45 BST 2021
Git commit 44ab8cc19e58b634762578236269232309918207 by Dmitry Kazakov.
Committed on 11/10/2021 at 14:19.
Pushed by dkazakov into branch 'krita/5.0'.
Fix linking color space of Alpha color spaces
Before the patch, the alpha color spaces could be interpreted in
multiple different ways, depending on the conversion destination.
When it was converted into rgb8 or graya8, it was interpreted as
srgb-gamma corrected graya space. But when the conversion was
requested for rgb16 or graya16 color space (e.g. in color smudge
tool), the conversion happened through the LabA color space, which
is linear. It caused rendering issues, when painting with color
smudge brushes on transparency masks (or any alpha8-based layers).
This patch fixes the issues and makes "Gray-D50-elle-V2-srgbtrc" to
be the only linking color space for alpha colorspaces. Which basically
means that all the flavours of alpha color space are now basically
flattened versions of srgb-gamma-corrected graya color space.
BUG:443422
CC:kimageshop at kde.org
M +22 -0 libs/pigment/KoColorConversionSystem.cpp
M +12 -0 libs/pigment/KoColorConversionSystem.h
M +33 -2 libs/pigment/KoColorConversionSystem_p.h
M +12 -4 libs/pigment/colorspaces/KoAlphaColorSpace.cpp
M +1 -8 libs/pigment/colorspaces/KoAlphaColorSpace.h
M +164 -6 libs/pigment/tests/TestColorConversionSystem.cpp
M +1 -0 libs/pigment/tests/TestColorConversionSystem.h
https://invent.kde.org/graphics/krita/commit/44ab8cc19e58b634762578236269232309918207
diff --git a/libs/pigment/KoColorConversionSystem.cpp b/libs/pigment/KoColorConversionSystem.cpp
index a2ef343bab..3d06b7f022 100644
--- a/libs/pigment/KoColorConversionSystem.cpp
+++ b/libs/pigment/KoColorConversionSystem.cpp
@@ -375,6 +375,28 @@ bool KoColorConversionSystem::existsGoodPath(const QString& srcModelId, const QS
return existAndGood;
}
+KoColorConversionSystem::Path KoColorConversionSystem::findBestPath(const QString &srcModelId, const QString &srcDepthId, const QString &srcProfileName, const QString &dstModelId, const QString &dstDepthId, const QString &dstProfileName) const
+{
+ const Node *srcNode = nodeFor(srcModelId, srcDepthId, srcProfileName);
+ const Node *dstNode = nodeFor(dstModelId, dstDepthId, dstProfileName);
+
+ KIS_ASSERT(srcNode);
+ KIS_ASSERT(dstNode);
+
+ return findBestPath(srcNode, dstNode);
+}
+
+KoColorConversionSystem::Path KoColorConversionSystem::findBestPath(const KoColorConversionSystem::NodeKey &src, const KoColorConversionSystem::NodeKey &dst) const
+{
+ const Node *srcNode = nodeFor(src);
+ const Node *dstNode = nodeFor(dst);
+
+ KIS_ASSERT(srcNode);
+ KIS_ASSERT(dstNode);
+
+ return findBestPath(srcNode, dstNode);
+}
+
QString KoColorConversionSystem::bestPathToDot(const QString& srcKey, const QString& dstKey) const
{
diff --git a/libs/pigment/KoColorConversionSystem.h b/libs/pigment/KoColorConversionSystem.h
index eb558c7e51..a82586b648 100644
--- a/libs/pigment/KoColorConversionSystem.h
+++ b/libs/pigment/KoColorConversionSystem.h
@@ -106,6 +106,18 @@ public:
* @return true if there is a good path between two color spaces
*/
bool existsGoodPath(const QString& srcModelId, const QString& srcDepthId, const QString& srcProfileName, const QString& dstModelId, const QString& dstDepthId, const QString& dstProfileName) const;
+
+ /**
+ * @return the best path for the specified color spaces. Used for
+ * testing purposes only
+ */
+ Path findBestPath(const QString& srcModelId, const QString& srcDepthId, const QString& srcProfileName, const QString& dstModelId, const QString& dstDepthId, const QString& dstProfileName) const;
+
+ /**
+ * @return the best path for the specified color spaces. Used for
+ * testing purposes only
+ */
+ Path findBestPath(const NodeKey &src, const NodeKey &dst) const;
private:
QString vertexToDot(Vertex* v, const QString &options) const;
private:
diff --git a/libs/pigment/KoColorConversionSystem_p.h b/libs/pigment/KoColorConversionSystem_p.h
index fac72a28a0..d3aafe40aa 100644
--- a/libs/pigment/KoColorConversionSystem_p.h
+++ b/libs/pigment/KoColorConversionSystem_p.h
@@ -12,10 +12,12 @@
#include "KoColorModelStandardIds.h"
#include "KoColorConversionTransformationFactory.h"
#include "KoColorSpaceEngine.h"
+#include <boost/operators.hpp>
#include <QList>
-struct KoColorConversionSystem::Node {
+struct KoColorConversionSystem::Node : boost::equality_comparable<KoColorConversionSystem::Node>
+{
Node()
: isHdr(false)
@@ -57,6 +59,14 @@ struct KoColorConversionSystem::Node {
return modelId + " " + depthId + " " + profileName;
}
+ NodeKey key() const;
+
+ friend bool operator==(const Node &lhs, const Node &rhs) {
+ return lhs.modelId == rhs.modelId &&
+ lhs.depthId == rhs.depthId &&
+ lhs.profileName == rhs.profileName;
+ }
+
QString modelId;
QString depthId;
QString profileName;
@@ -70,8 +80,15 @@ struct KoColorConversionSystem::Node {
bool isEngine;
const KoColorSpaceEngine* engine;
};
+
Q_DECLARE_TYPEINFO(KoColorConversionSystem::Node, Q_MOVABLE_TYPE);
+QDebug operator<<(QDebug dbg, const KoColorConversionSystem::Node &node)
+{
+ dbg.nospace() << "Node(" << node.modelId << ", " << node.depthId << ", " << node.profileName << ")";
+ return dbg.space();
+}
+
struct KoColorConversionSystem::Vertex {
Vertex(Node* _srcNode, Node* _dstNode)
@@ -123,7 +140,9 @@ private:
};
-struct KoColorConversionSystem::NodeKey {
+struct KoColorConversionSystem::NodeKey
+ : public boost::equality_comparable<NodeKey>
+{
NodeKey(const QString &_modelId, const QString &_depthId, const QString &_profileName)
: modelId(_modelId)
@@ -140,6 +159,18 @@ struct KoColorConversionSystem::NodeKey {
};
Q_DECLARE_TYPEINFO(KoColorConversionSystem::NodeKey, Q_MOVABLE_TYPE);
+QDebug operator<<(QDebug dbg, const KoColorConversionSystem::NodeKey &key)
+{
+ dbg.nospace() << "NodeKey(" << key.modelId << ", " << key.depthId << ", " << key.profileName << ")";
+ return dbg.space();
+}
+
+
+inline KoColorConversionSystem::NodeKey KoColorConversionSystem::Node::key() const
+{
+ return NodeKey(modelId, depthId, profileName);
+}
+
struct KoColorConversionSystem::Path {
Path()
diff --git a/libs/pigment/colorspaces/KoAlphaColorSpace.cpp b/libs/pigment/colorspaces/KoAlphaColorSpace.cpp
index b8ee58ef89..62b6e34996 100644
--- a/libs/pigment/colorspaces/KoAlphaColorSpace.cpp
+++ b/libs/pigment/colorspaces/KoAlphaColorSpace.cpp
@@ -271,16 +271,24 @@ template class KoAlphaColorSpaceImpl<AlphaF32Traits>;
template <class _CSTrait>
QList<KoColorConversionTransformationFactory *> KoAlphaColorSpaceFactoryImpl<_CSTrait>::colorConversionLinks() const
{
+ /**
+ * Out Alpha color space is defined as "a flattened representation of a GrayA color space with sRGB tone curve".
+ * Therefore we do **not** define direct conversions to/from LabA, because LabA has a linear tone curve.
+ */
+
QList<KoColorConversionTransformationFactory*> factories;
factories << new KoColorConversionFromAlphaTransformationFactoryImpl<channels_type>(GrayAColorModelID.id(), Integer8BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc");
factories << new KoColorConversionToAlphaTransformationFactoryImpl<channels_type>(GrayAColorModelID.id(), Integer8BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc");
- factories << new KoColorConversionFromAlphaTransformationFactoryImpl<channels_type>(LABAColorModelID.id(), Integer16BitsColorDepthID.id(), "default");
- factories << new KoColorConversionToAlphaTransformationFactoryImpl<channels_type>(LABAColorModelID.id(), Integer16BitsColorDepthID.id(), "default");
+ factories << new KoColorConversionFromAlphaTransformationFactoryImpl<channels_type>(GrayAColorModelID.id(), Integer16BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc");
+ factories << new KoColorConversionToAlphaTransformationFactoryImpl<channels_type>(GrayAColorModelID.id(), Integer16BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc");
+
+ factories << new KoColorConversionFromAlphaTransformationFactoryImpl<channels_type>(GrayAColorModelID.id(), Float16BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc");
+ factories << new KoColorConversionToAlphaTransformationFactoryImpl<channels_type>(GrayAColorModelID.id(), Float16BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc");
- factories << new KoColorConversionFromAlphaTransformationFactoryImpl<channels_type>(LABAColorModelID.id(), Integer16BitsColorDepthID.id(), "Lab identity built-in");
- factories << new KoColorConversionToAlphaTransformationFactoryImpl<channels_type>(LABAColorModelID.id(), Integer16BitsColorDepthID.id(), "Lab identity built-in");
+ factories << new KoColorConversionFromAlphaTransformationFactoryImpl<channels_type>(GrayAColorModelID.id(), Float32BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc");
+ factories << new KoColorConversionToAlphaTransformationFactoryImpl<channels_type>(GrayAColorModelID.id(), Float32BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc");
return factories;
}
diff --git a/libs/pigment/colorspaces/KoAlphaColorSpace.h b/libs/pigment/colorspaces/KoAlphaColorSpace.h
index b966f7603a..8863ae54a5 100644
--- a/libs/pigment/colorspaces/KoAlphaColorSpace.h
+++ b/libs/pigment/colorspaces/KoAlphaColorSpace.h
@@ -202,16 +202,9 @@ public:
false,
AlphaColorModelID,
colorDepthIdForChannelType<channels_type>(),
- qMin(16, int(sizeof(channels_type) * 8)), // DIRTY HACK ALERT: see a comment below!
+ sizeof(channels_type) * 8,
100000)
{
- /**
- * Note about a hack with reference bit depth: right now all the color
- * conversions to/from Alpha colorspace are done via LabAU16 16-bit color space,
- * therefore, the conversions are lossy! Better conversions are yet to be implemented,
- * but for now we just define this color space as having 16-bit reference depth
- * (the problem arises with AlphaF32 only of course, which is hardly used anywhere).
- */
}
KoColorSpace *createColorSpace(const KoColorProfile *) const override {
diff --git a/libs/pigment/tests/TestColorConversionSystem.cpp b/libs/pigment/tests/TestColorConversionSystem.cpp
index 739d267f04..406038432e 100644
--- a/libs/pigment/tests/TestColorConversionSystem.cpp
+++ b/libs/pigment/tests/TestColorConversionSystem.cpp
@@ -59,6 +59,164 @@ void TestColorConversionSystem::testGoodConnections()
#include <KoColor.h>
+#include <KoColorConversionSystem_p.h>
+#include <kis_debug.h>
+
+namespace QTest {
+inline bool qCompare(const std::vector<KoColorConversionSystem::NodeKey> &t1,
+ const std::vector<KoColorConversionSystem::NodeKey> &t2,
+ const char *actual, const char *expected,
+ const char *file, int line) {
+
+ bool result = t1 == t2;
+
+ if (!result) {
+ QString actualStr;
+ QDebug act(&actualStr);
+ act.nospace() << actual << ": " << t1;
+
+ QString expectedStr;
+ QDebug exp(&expectedStr);
+ exp.nospace() << expected << ": " << t2;
+
+ QString message = QString("Compared paths are not the same:\n Expected: %1\n Actual: %2").arg(expectedStr).arg(actualStr);
+ QTest::qFail(message.toLocal8Bit(), file, line);
+ }
+
+ return t1 == t2;
+}
+}
+
+void TestColorConversionSystem::testAlphaConnectionPaths()
+{
+ const KoColorSpace *alpha8 = KoColorSpaceRegistry::instance()->alpha8();
+
+ using Path = KoColorConversionSystem::Path;
+ using Vertex = KoColorConversionSystem::Vertex;
+ using Node = KoColorConversionSystem::Node;
+ using NodeKey = KoColorConversionSystem::NodeKey;
+
+ std::vector<NodeKey> expectedPath;
+
+ auto calcPath = [] (const std::vector<NodeKey> &expectedPath) {
+
+ const KoColorConversionSystem *system = KoColorSpaceRegistry::instance()->colorConversionSystem();
+
+ Path path =
+ system->findBestPath(expectedPath.front(), expectedPath.back());
+
+ std::vector<NodeKey> realPath;
+
+ Q_FOREACH (const Vertex *vertex, path.vertexes) {
+ if (!vertex->srcNode->isEngine) {
+ realPath.push_back(vertex->srcNode->key());
+ }
+ }
+ realPath.push_back(path.vertexes.last()->dstNode->key());
+
+ return realPath;
+ };
+
+ // to Alpha8 conversions. Everything should go via GrayA color space,
+ // we expect alpha colorspace be just a flattened of graya color space
+ // with srgb tone curve.
+
+ expectedPath =
+ {{GrayAColorModelID.id(), Integer8BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"},
+ {alpha8->colorModelId().id(), alpha8->colorDepthId().id(), alpha8->profile()->name()}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+
+ expectedPath =
+ {{GrayAColorModelID.id(), Integer16BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"},
+ {alpha8->colorModelId().id(), alpha8->colorDepthId().id(), alpha8->profile()->name()}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+
+#ifdef HAVE_OPENEXR
+ expectedPath =
+ {{GrayAColorModelID.id(), Float16BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"},
+ {alpha8->colorModelId().id(), alpha8->colorDepthId().id(), alpha8->profile()->name()}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+#endif
+
+ expectedPath =
+ {{GrayAColorModelID.id(), Float32BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"},
+ {alpha8->colorModelId().id(), alpha8->colorDepthId().id(), alpha8->profile()->name()}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+
+ expectedPath =
+ {{RGBAColorModelID.id(), Integer8BitsColorDepthID.id(), KoColorSpaceRegistry::instance()->p709SRGBProfile()->name()},
+ {GrayAColorModelID.id(), Integer8BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"},
+ {alpha8->colorModelId().id(), alpha8->colorDepthId().id(), alpha8->profile()->name()}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+
+ expectedPath =
+ {{RGBAColorModelID.id(), Integer16BitsColorDepthID.id(), KoColorSpaceRegistry::instance()->p709SRGBProfile()->name()},
+ {GrayAColorModelID.id(), Integer8BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"},
+ {alpha8->colorModelId().id(), alpha8->colorDepthId().id(), alpha8->profile()->name()}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+
+ expectedPath =
+ {{RGBAColorModelID.id(), Integer8BitsColorDepthID.id(), KoColorSpaceRegistry::instance()->p709SRGBProfile()->name()},
+ {GrayAColorModelID.id(), Integer8BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"},
+ {AlphaColorModelID.id(), Integer16BitsColorDepthID.id(), alpha8->profile()->name()}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+
+ expectedPath =
+ {{RGBAColorModelID.id(), Integer16BitsColorDepthID.id(), KoColorSpaceRegistry::instance()->p709SRGBProfile()->name()},
+ {GrayAColorModelID.id(), Integer16BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"},
+ {AlphaColorModelID.id(), Integer16BitsColorDepthID.id(), alpha8->profile()->name()}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+
+ // from Alpha8 conversions. Everything should go via GrayA color space
+
+ expectedPath =
+ {{alpha8->colorModelId().id(), alpha8->colorDepthId().id(), alpha8->profile()->name()},
+ {GrayAColorModelID.id(), Integer8BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+
+ expectedPath =
+ {{alpha8->colorModelId().id(), alpha8->colorDepthId().id(), alpha8->profile()->name()},
+ {GrayAColorModelID.id(), Integer16BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+
+#ifdef HAVE_OPENEXR
+ expectedPath =
+ {{alpha8->colorModelId().id(), alpha8->colorDepthId().id(), alpha8->profile()->name()},
+ {GrayAColorModelID.id(), Float16BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+#endif
+
+ expectedPath =
+ {{alpha8->colorModelId().id(), alpha8->colorDepthId().id(), alpha8->profile()->name()},
+ {GrayAColorModelID.id(), Float32BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+
+ expectedPath =
+ {{alpha8->colorModelId().id(), alpha8->colorDepthId().id(), alpha8->profile()->name()},
+ {GrayAColorModelID.id(), Integer8BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"},
+ {RGBAColorModelID.id(), Integer8BitsColorDepthID.id(), KoColorSpaceRegistry::instance()->p709SRGBProfile()->name()}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+
+
+ expectedPath =
+ {{alpha8->colorModelId().id(), alpha8->colorDepthId().id(), alpha8->profile()->name()},
+ {GrayAColorModelID.id(), Integer8BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"},
+ {RGBAColorModelID.id(), Integer16BitsColorDepthID.id(), KoColorSpaceRegistry::instance()->p709SRGBProfile()->name()}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+
+ expectedPath =
+ {{AlphaColorModelID.id(), Integer16BitsColorDepthID.id(), alpha8->profile()->name()},
+ {GrayAColorModelID.id(), Integer8BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"},
+ {RGBAColorModelID.id(), Integer8BitsColorDepthID.id(), KoColorSpaceRegistry::instance()->p709SRGBProfile()->name()}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+
+ expectedPath =
+ {{AlphaColorModelID.id(), Integer16BitsColorDepthID.id(), alpha8->profile()->name()},
+ {GrayAColorModelID.id(), Integer16BitsColorDepthID.id(), "Gray-D50-elle-V2-srgbtrc.icc"},
+ {RGBAColorModelID.id(), Integer16BitsColorDepthID.id(), KoColorSpaceRegistry::instance()->p709SRGBProfile()->name()}};
+ QCOMPARE(calcPath(expectedPath), expectedPath);
+}
+
void TestColorConversionSystem::testAlphaConversions()
{
const KoColorSpace *alpha8 = KoColorSpaceRegistry::instance()->alpha8();
@@ -87,7 +245,7 @@ void TestColorConversionSystem::testAlphaConversions()
c.convertTo(rgb8);
QCOMPARE(c.toQColor(), QColor(128,128,128,255));
c.convertTo(alpha8);
- QCOMPARE(c.opacityU8(), quint8(137)); // alpha is linear, so the value increases
+ QCOMPARE(c.opacityU8(), quint8(128));
}
{
@@ -112,7 +270,7 @@ void TestColorConversionSystem::testAlphaConversions()
c.convertTo(rgb16);
QCOMPARE(c.toQColor(), QColor(128,128,128,255));
c.convertTo(alpha8);
- QCOMPARE(c.opacityU8(), quint8(137)); // alpha is linear, so the value increases
+ QCOMPARE(c.opacityU8(), quint8(128));
}
}
@@ -145,14 +303,14 @@ void TestColorConversionSystem::testAlphaU16Conversions()
c.convertTo(rgb8);
QCOMPARE(c.toQColor(), QColor(128,128,128,255));
c.convertTo(alpha16);
- QCOMPARE(c.opacityU8(), quint8(137)); // alpha is linear, so the value increases
+ QCOMPARE(c.opacityU8(), quint8(128));
}
{
KoColor c(QColor(255,255,255,255), alpha16);
QCOMPARE(c.opacityU8(), quint8(255));
c.convertTo(rgb16);
- QCOMPARE(c.toQColor(), QColor(254,255,255));
+ QCOMPARE(c.toQColor(), QColor(255,255,255));
c.convertTo(alpha16);
QCOMPARE(c.opacityU8(), quint8(255));
}
@@ -160,7 +318,7 @@ void TestColorConversionSystem::testAlphaU16Conversions()
{
KoColor c(QColor(255,255,255,0), alpha16);
c.convertTo(rgb16);
- QCOMPARE(c.toQColor(), QColor(0,0,1,255));
+ QCOMPARE(c.toQColor(), QColor(0,0,0,255));
c.convertTo(alpha16);
QCOMPARE(c.opacityU8(), quint8(0));
}
@@ -168,7 +326,7 @@ void TestColorConversionSystem::testAlphaU16Conversions()
{
KoColor c(QColor(255,255,255,128), alpha16);
c.convertTo(rgb16);
- QCOMPARE(c.toQColor(), QColor(118,120,120,255));
+ QCOMPARE(c.toQColor(), QColor(128,128,128,255));
c.convertTo(alpha16);
QCOMPARE(c.opacityU8(), quint8(128));
}
diff --git a/libs/pigment/tests/TestColorConversionSystem.h b/libs/pigment/tests/TestColorConversionSystem.h
index b86447fb72..d12c3fd74f 100644
--- a/libs/pigment/tests/TestColorConversionSystem.h
+++ b/libs/pigment/tests/TestColorConversionSystem.h
@@ -28,6 +28,7 @@ public:
private Q_SLOTS:
void testConnections();
void testGoodConnections();
+ void testAlphaConnectionPaths();
void testAlphaConversions();
void testAlphaU16Conversions();
void benchmarkAlphaToRgbConversion();
More information about the kimageshop
mailing list