Commit 0f9c5472 authored by Dmitry Kazakov's avatar Dmitry Kazakov

Fix crash after cropping a specific image

This patch ends the era of the custom "empty extent" value
QRect(quint32_MAX, quint32_MAX, 0, 0). When I started refactoring
tile engine ten years ago in 2009, this "empty extent" flag was
already present. It was just a result of the way how extent was
calculated those days. I guess it was also considered as
an "optimization", or as "a flag" that could ease debugging a bit
(and it actually did help debugging a couple of times).

Years passed by and I considered to remove this custom value multiple
times. It was always causing troubles, because the QRect is not safe
against integer oveflows. But the problem was, there was code outside
tiles engine that relied on this behavior. So I was always scared of
the actual removal. Even when the tile engine was rewritten again
to become lockfree in 2017, I insisted on keeping the old behavior...

So, it looks like now all code that relies on this custom value is gone,
so it should be safe to remove it.

Funny side, I guess it was one of few artifacts that were still kept
in Krita since 'tiles' and 'tiles_new' days (has anyone ever wondered
why the engine's folder is called 'tiles3'?) ;)

BUG:411536
parent 46cc3890
......@@ -67,7 +67,7 @@ void KisIteratorTest::writeBytes(const KoColorSpace * colorSpace)
KisPaintDevice dev(colorSpace);
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
// Check allocation on tile boundaries
......@@ -82,7 +82,7 @@ void KisIteratorTest::writeBytes(const KoColorSpace * colorSpace)
QCOMPARE(dev.exactBounds(), QRect(0, 0, 64 * 5, 64 * 2));
dev.clear();
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
dev.clear();
// Covers three by three tiles
......@@ -104,7 +104,7 @@ void KisIteratorTest::fill(const KoColorSpace * colorSpace)
KisPaintDevice dev(colorSpace);
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
QScopedArrayPointer<quint8> bytes(allocatePixels(colorSpace, 1));
......@@ -134,11 +134,11 @@ void KisIteratorTest::hLineIter(const KoColorSpace * colorSpace)
QScopedArrayPointer<quint8> bytes(allocatePixels(colorSpace, 1));
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
KisHLineConstIteratorSP cit = dev.createHLineConstIteratorNG(0, 0, 128);
do {} while (cit->nextPixel());
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
QCOMPARE(dev.exactBounds(), QRect(QPoint(0, 0), QPoint(-1, -1)));
{
......@@ -199,11 +199,11 @@ void KisIteratorTest::vLineIter(const KoColorSpace * colorSpace)
KisPaintDevice dev(colorSpace);
QScopedArrayPointer<quint8> bytes(allocatePixels(colorSpace, 1));
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
KisVLineConstIteratorSP cit = dev.createVLineConstIteratorNG(0, 0, 128);
do {} while (cit->nextPixel());
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
QCOMPARE(dev.exactBounds(), QRect(QPoint(0, 0), QPoint(-1, -1)));
{
......@@ -249,7 +249,7 @@ void KisIteratorTest::randomAccessor(const KoColorSpace * colorSpace)
KisPaintDevice dev(colorSpace);
QScopedArrayPointer<quint8> bytes(allocatePixels(colorSpace, 1));
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
KisRandomConstAccessorSP acc = dev.createRandomConstAccessorNG(0, 0);
for (int y = 0; y < 128; ++y) {
......@@ -257,7 +257,7 @@ void KisIteratorTest::randomAccessor(const KoColorSpace * colorSpace)
acc->moveTo(x, y);
}
}
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
KisRandomAccessorSP ac = dev.createRandomAccessorNG(0, 0);
for (int y = 0; y < 128; ++y) {
......
......@@ -65,7 +65,7 @@ void KisIteratorNGTest::writeBytes(const KoColorSpace * colorSpace)
KisPaintDevice dev(colorSpace);
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
// Check allocation on tile boundaries
......@@ -80,7 +80,7 @@ void KisIteratorNGTest::writeBytes(const KoColorSpace * colorSpace)
QCOMPARE(dev.exactBounds(), QRect(0, 0, 64 * 5, 64 * 2));
dev.clear();
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
dev.clear();
// Covers three by three tiles
......@@ -102,7 +102,7 @@ void KisIteratorNGTest::fill(const KoColorSpace * colorSpace)
KisPaintDevice dev(colorSpace);
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
QScopedArrayPointer<quint8> bytes(allocatePixels(colorSpace, 1));
......@@ -131,13 +131,13 @@ void KisIteratorNGTest::sequentialIter(const KoColorSpace * colorSpace)
KisPaintDeviceSP dev = new KisPaintDevice(colorSpace);
QCOMPARE(dev->extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev->extent(), QRect());
// Const does not extend the extent
{
KisSequentialConstIterator it(dev, QRect(0, 0, 128, 128));
while (it.nextPixel());
QCOMPARE(dev->extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev->extent(), QRect());
QCOMPARE(dev->exactBounds(), QRect(QPoint(0, 0), QPoint(-1, -1)));
}
......@@ -264,11 +264,11 @@ void KisIteratorNGTest::hLineIter(const KoColorSpace * colorSpace)
QScopedArrayPointer<quint8> bytes(allocatePixels(colorSpace, 1));
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
KisHLineConstIteratorSP cit = dev.createHLineConstIteratorNG(0, 0, 128);
while (!cit->nextPixel());
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
QCOMPARE(dev.exactBounds(), QRect(QPoint(0, 0), QPoint(-1, -1)));
dev.clear();
......@@ -333,11 +333,11 @@ void KisIteratorNGTest::vLineIter(const KoColorSpace * colorSpace)
KisPaintDevice dev(colorSpace);
QScopedArrayPointer<quint8> bytes(allocatePixels(colorSpace, 1));
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
KisVLineConstIteratorSP cit = dev.createVLineConstIteratorNG(0, 0, 128);
while (cit->nextPixel());
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
QCOMPARE(dev.exactBounds(), QRect(QPoint(0, 0), QPoint(-1, -1)));
cit.clear();
......@@ -383,7 +383,7 @@ void KisIteratorNGTest::randomAccessor(const KoColorSpace * colorSpace)
KisPaintDevice dev(colorSpace);
QScopedArrayPointer<quint8> bytes(allocatePixels(colorSpace, 1));
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
KisRandomConstAccessorSP acc = dev.createRandomConstAccessorNG(0, 0);
for (int y = 0; y < 128; ++y) {
......@@ -391,7 +391,7 @@ void KisIteratorNGTest::randomAccessor(const KoColorSpace * colorSpace)
acc->moveTo(x, y);
}
}
QCOMPARE(dev.extent(), QRect(qint32_MAX, qint32_MAX, 0, 0));
QCOMPARE(dev.extent(), QRect());
KisRandomAccessorSP ac = dev.createRandomAccessorNG(0, 0);
for (int y = 0; y < 128; ++y) {
......
......@@ -238,7 +238,7 @@ void KisTiledExtentManager::Data::updateMax()
KisTiledExtentManager::KisTiledExtentManager()
{
QWriteLocker l(&m_extentLock);
m_currentExtent = QRect(qint32_MAX, qint32_MAX, 0, 0);
m_currentExtent = QRect();
}
void KisTiledExtentManager::notifyTileAdded(qint32 col, qint32 row)
......@@ -286,7 +286,7 @@ void KisTiledExtentManager::clear()
m_rowsData.clear();
QWriteLocker lock(&m_extentLock);
m_currentExtent = QRect(qint32_MAX, qint32_MAX, 0, 0);
m_currentExtent = QRect();
}
QRect KisTiledExtentManager::extent() const
......@@ -297,17 +297,17 @@ QRect KisTiledExtentManager::extent() const
void KisTiledExtentManager::updateExtent()
{
qint32 minX, maxX, minY, maxY;
qint32 minX, width, minY, height;
{
QReadLocker cl(&m_colsData.m_extentLock);
if (m_colsData.isEmpty()) {
minX = qint32_MAX;
maxX = 0;
minX = 0;
width = 0;
} else {
minX = m_colsData.min() * KisTileData::WIDTH;
maxX = (m_colsData.max() + 1) * KisTileData::WIDTH - minX;
width = (m_colsData.max() + 1) * KisTileData::WIDTH - minX;
}
}
......@@ -315,14 +315,14 @@ void KisTiledExtentManager::updateExtent()
QReadLocker rl(&m_rowsData.m_extentLock);
if (m_rowsData.isEmpty()) {
minY = qint32_MAX;
maxY = 0;
minY = 0;
height = 0;
} else {
minY = m_rowsData.min() * KisTileData::HEIGHT;
maxY = (m_rowsData.max() + 1) * KisTileData::HEIGHT - minY;
height = (m_rowsData.max() + 1) * KisTileData::HEIGHT - minY;
}
}
QWriteLocker lock(&m_extentLock);
m_currentExtent = QRect(minX, minY, maxX, maxY);
m_currentExtent = QRect(minX, minY, width, height);
}
......@@ -54,10 +54,20 @@ public:
}
inline void extent(qint32 &x, qint32 &y, qint32 &w, qint32 &h) {
x = m_extentMinX;
y = m_extentMinY;
w = (m_extentMaxX >= m_extentMinX) ? m_extentMaxX - m_extentMinX + 1 : 0;
h = (m_extentMaxY >= m_extentMinY) ? m_extentMaxY - m_extentMinY + 1 : 0;
const bool extentIsValid =
m_extentMaxX >= m_extentMinX && m_extentMaxY >= m_extentMinY;
if (extentIsValid) {
x = m_extentMinX;
y = m_extentMinY;
w = m_extentMaxX - m_extentMinX + 1;
h = m_extentMaxY - m_extentMinY + 1;
} else {
x = 0;
y = 0;
w = 0;
h = 0;
}
}
inline QRect extent() {
......
......@@ -95,7 +95,7 @@ void KisTiledDataManagerTest::testUndoingNewTiles()
{
// "growing extent bug"
const QRect nullRect(qint32_MAX,qint32_MAX,0,0);
const QRect nullRect;
quint8 defaultPixel = 0;
KisTiledDataManager srcDM(1, &defaultPixel);
......
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