Commit d7542a79 authored by Agata Cacko's avatar Agata Cacko

Fix Sequential Iterator assert on invalid rectangles

Summary:
Before Quick Brush engine (roundmarker) was crashing
because Sequential Iterator throwed out asserts
when the rectangle was not "empty" according to Qt
but  still not valid (for example has intmin value on
width or height). This commit fixes that behaviour by
providing additional checks.
Commit includes also benchmarks for roundmarker.

BUG:404179

Test Plan:
- benchmarks included in the commit
- painting with Quick Brush (i.e. b) Basic-1)
with size <1 px

Reviewers: dkazakov, #krita

Reviewed By: dkazakov, #krita

Subscribers: rempt

Tags: #krita

Differential Revision: https://phabricator.kde.org/D19881
parent 1db77fe7
......@@ -52,6 +52,7 @@ inline double drand48()
//#define SAVE_OUTPUT
static const int LINES = 20;
static const int RECTANGLES = 20;
const QString OUTPUT_FORMAT = ".png";
void KisStrokeBenchmark::initTestCase()
......@@ -76,6 +77,8 @@ void KisStrokeBenchmark::initTestCase()
initCurvePoints(width, height);
// for the lines test
initLines(width,height);
// for the rectangles test
initRectangles(width, height);
}
void KisStrokeBenchmark::init()
......@@ -114,6 +117,32 @@ void KisStrokeBenchmark::initLines(int width, int height)
}
}
void KisStrokeBenchmark::initRectangles(int width, int height)
{
qreal margin = 0.5;
qreal skip = 0.01;
qreal marginWidth = margin*width;
qreal marginHeight = margin*height;
qreal skipWidth = skip*width >= 1 ? skip*width : 1;
qreal skipHeight = skip*width >= 1 ? skip*width : 1;
// "concentric" rectangles
for (int i = 0; i < RECTANGLES; i++){
QPoint corner1 = QPoint(marginWidth + i*skipWidth, marginHeight + i*skipHeight);
QPoint corner2 = QPoint(width - marginWidth - i*skipWidth, height - marginHeight - i*skipHeight);
if(corner1.x() < corner2.x() && corner1.y() < corner2.y()) {
// if the rectangle is not empty
m_rectangleLeftLowerCorners.append(corner1);
m_rectangleRightUpperCorners.append(corner2);
}
}
}
void KisStrokeBenchmark::cleanupTestCase()
{
......@@ -327,6 +356,52 @@ void KisStrokeBenchmark::colorsmudgeRL()
benchmarkStroke(presetFileName);
}
void KisStrokeBenchmark::roundMarker()
{
// Quick Brush engine ( b) Basic - 1 brush, size 40px)
QString presetFileName = "roundmarker40px.kpp";
benchmarkStroke(presetFileName);
}
void KisStrokeBenchmark::roundMarkerRandomLines()
{
// Quick Brush engine ( b) Basic - 1 brush, size 40px)
QString presetFileName = "roundmarker40px.kpp";
benchmarkRandomLines(presetFileName);
}
void KisStrokeBenchmark::roundMarkerRectangle()
{
// Quick Brush engine ( b) Basic - 1 brush, size 40px)
QString presetFileName = "roundmarker40px.kpp";
benchmarkStroke(presetFileName);
}
void KisStrokeBenchmark::roundMarkerHalfPixel()
{
// Quick Brush engine ( b) Basic - 1 brush, size 0.5px)
QString presetFileName = "roundmarker05px.kpp";
benchmarkStroke(presetFileName);
}
void KisStrokeBenchmark::roundMarkerRandomLinesHalfPixel()
{
// Quick Brush engine ( b) Basic - 1 brush, size 0.5px)
QString presetFileName = "roundmarker05px.kpp";
benchmarkRandomLines(presetFileName);
}
void KisStrokeBenchmark::roundMarkerRectangleHalfPixel()
{
// Quick Brush engine ( b) Basic - 1 brush, size 0.5px)
QString presetFileName = "roundmarker05px.kpp";
benchmarkStroke(presetFileName);
}
/*
void KisStrokeBenchmark::predefinedBrush()
{
......@@ -462,6 +537,37 @@ void KisStrokeBenchmark::benchmarkRandomLines(QString presetFileName)
#endif
}
void KisStrokeBenchmark::benchmarkRectangle(QString presetFileName)
{
KisPaintOpPresetSP preset = new KisPaintOpPreset(m_dataPath + presetFileName);
bool loadedOk = preset->load();
if (!loadedOk){
dbgKrita << "The preset was not loaded correctly. Done.";
return;
}else{
dbgKrita << "preset : " << presetFileName;
}
m_painter->setPaintOpPreset(preset, m_layer, m_image);
int rectangleNumber = m_rectangleLeftLowerCorners.size(); // see initRectangles
QBENCHMARK{
KisDistanceInformation currentDistance;
for (int i = 0; i < rectangleNumber; i++){
QPainterPath path;
QRect rect = QRect(m_rectangleLeftLowerCorners[i], m_rectangleRightUpperCorners[i]);
path.addRect(rect);
m_painter->paintPainterPath(path);
}
}
#ifdef SAVE_OUTPUT
m_layer->paintDevice()->convertToQImage(0).save(m_outputPath + presetFileName + "_rectangle" + OUTPUT_FORMAT);
#endif
}
void KisStrokeBenchmark::benchmarkStroke(QString presetFileName)
{
KisPaintOpPresetSP preset = new KisPaintOpPreset(m_dataPath + presetFileName);
......
......@@ -51,8 +51,13 @@ private:
QVector<QPointF> m_startPoints;
QVector<QPointF> m_endPoints;
QVector<QPoint> m_rectangleLeftLowerCorners;
QVector<QPoint> m_rectangleRightUpperCorners;
void initCurvePoints(int width, int height);
void initLines(int width, int height);
void initRectangles(int width, int height);
QString m_dataPath;
QString m_outputPath;
......@@ -62,6 +67,7 @@ private:
inline void benchmarkStroke(QString presetFileName);
inline void benchmarkLine(QString presetFileName);
inline void benchmarkCircle(QString presetFileName);
inline void benchmarkRectangle(QString presetFileName);
private Q_SLOTS:
void initTestCase();
......@@ -120,6 +126,15 @@ private Q_SLOTS:
void colorsmudge();
void colorsmudgeRL();
void roundMarker();
void roundMarkerRandomLines();
void roundMarkerRectangle();
void roundMarkerHalfPixel();
void roundMarkerRandomLinesHalfPixel();
void roundMarkerRectangleHalfPixel();
/*
void predefinedBrush();
void predefinedBrushRL();
......
......@@ -350,12 +350,13 @@ int quadraticEquation(qreal a, qreal b, qreal c, qreal *x1, qreal *x2)
int numSolutions = 0;
const qreal D = pow2(b) - 4 * a * c;
const qreal eps = 1e-14;
if (D < 0) {
return 0;
} else if (qFuzzyCompare(D, 0)) {
if (qAbs(D) <= eps) {
*x1 = -b / (2 * a);
numSolutions = 1;
} else if (D < 0) {
return 0;
} else {
const qreal sqrt_D = std::sqrt(D);
......
......@@ -44,6 +44,23 @@ KisMarkerPainter::~KisMarkerPainter()
{
}
bool KisMarkerPainter::isNumberInValidRange(qint32 number)
{
if (number < -ValidNumberRangeValue || number > ValidNumberRangeValue)
return false;
return true;
}
bool KisMarkerPainter::isRectInValidRange(const QRect &rect)
{
return isNumberInValidRange(rect.x())
&& isNumberInValidRange(rect.y())
&& isNumberInValidRange(rect.width())
&& isNumberInValidRange(rect.height());
}
void KisMarkerPainter::fillHalfBrushDiff(const QPointF &p1, const QPointF &p2, const QPointF &p3,
const QPointF &center, qreal radius)
{
......@@ -63,7 +80,12 @@ void KisMarkerPainter::fillHalfBrushDiff(const QPointF &p1, const QPointF &p2, c
boundRect = KisAlgebra2D::cutOffRect(boundRect, plane1);
boundRect = KisAlgebra2D::cutOffRect(boundRect, plane2);
KisSequentialIterator it(m_d->device, boundRect.toAlignedRect());
QRect alignedRect = boundRect.toAlignedRect();
KIS_SAFE_ASSERT_RECOVER_RETURN(isRectInValidRange(alignedRect));
KisSequentialIterator it(m_d->device, alignedRect);
while (it.nextPixel()) {
QPoint pt(it.x(), it.y());
......@@ -104,7 +126,11 @@ void KisMarkerPainter::fillFullCircle(const QPointF &center, qreal radius)
KisAlgebra2D::OuterCircle outer(center, radius);
KisSequentialIterator it(m_d->device, boundRect.toAlignedRect());
QRect alignedRect = boundRect.toAlignedRect();
KIS_SAFE_ASSERT_RECOVER_RETURN(isRectInValidRange(alignedRect));
KisSequentialIterator it(m_d->device, alignedRect);
while (it.nextPixel()) {
QPoint pt(it.x(), it.y());
......
......@@ -30,6 +30,10 @@ class KoColor;
class KRITAIMAGE_EXPORT KisMarkerPainter
{
public:
/// Any number bigger than this or lower than -this is considered invalid
static const qint32 ValidNumberRangeValue = 2140000000; // bit less than max value of int
KisMarkerPainter(KisPaintDeviceSP device, const KoColor &color);
~KisMarkerPainter();
......@@ -43,6 +47,22 @@ public:
private:
struct Private;
const QScopedPointer<Private> m_d;
/// This method is to check whether the number is not infinite
/// or negative infinite with some epsilon
/// (@see ValidNumberRangeValue)
/// @param number value entered by the user
/// @return true if number is in range, false otherwise
bool isNumberInValidRange(qint32 number);
/// This method is to check whether the rectangle has only valid numbers
/// as values for x, y, height and width.
/// If values are not valid, Sequential Iterator can give incorrect values.
/// (@see isNumberInValidRange, ValidNumberRangeValue)
/// @param number value entered by the user
/// @return true if rect's values is in range, false otherwise
bool isRectInValidRange(const QRect &rect);
};
#endif /* __KIS_MARKER_PAINTER_H */
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