From d59b7f456deed0b1651d49b1b4e094efdb8d9ae1 Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Thu, 20 Aug 2026 12:43:42 +0200 Subject: [PATCH] Clamp coordinates in QTriangulator We assume 21 bits for the absolute value of the POD point components and we assert for this deeper in the code. Up until now the contract has been that the user of QTriangulator clamps the values before passing them in, but to make this less error prone, we make sure they are clamped as part of the fixed point conversion. Fixes: QTBUG-149343 Pick-to: 6.5 5.15 Change-Id: Iff059306c5691cb751f44264458ee18a7abba0e2 Reviewed-by: Eirik Aavitsland (cherry picked from commit 4366caf9893c0fa225e95901f8930b88c52309e1) Reviewed-by: Qt Cherry-pick Bot (cherry picked from commit 7a78b19f853949011ee439bf3f96b501a28ac359) (cherry picked from commit d2129a7f0e1f2ebef3f4897276db9b5b05874d8f) --- diff --git a/src/gui/painting/qtriangulator.cpp b/src/gui/painting/qtriangulator.cpp index 79ef325..c45439e 100644 --- a/src/gui/painting/qtriangulator.cpp +++ b/src/gui/painting/qtriangulator.cpp @@ -20,7 +20,17 @@ //#define Q_TRIANGULATOR_DEBUG +// We assume 21 bits for the absolute value of vertices +#define QTRIANGULATOR_MAX (1 << 21) #define Q_FIXED_POINT_SCALE 32 +#define QTRIANGULATOR_FIXED_POINT_MAX ((qreal(QTRIANGULATOR_MAX - 1)) / qreal(Q_FIXED_POINT_SCALE)) + +static inline int qToClampedFixedPoint(qreal c) +{ + int ret = qRound(std::clamp(c, -QTRIANGULATOR_FIXED_POINT_MAX, QTRIANGULATOR_FIXED_POINT_MAX) * Q_FIXED_POINT_SCALE); + Q_ASSERT(qAbs(ret) < QTRIANGULATOR_MAX); + return ret; +} template struct QVertexSet @@ -742,8 +752,8 @@ QVertexSet QTriangulator::triangulate() { for (int i = 0; i < m_vertices.size(); ++i) { - Q_ASSERT(qAbs(m_vertices.at(i).x) < (1 << 21)); - Q_ASSERT(qAbs(m_vertices.at(i).y) < (1 << 21)); + Q_ASSERT(qAbs(m_vertices.at(i).x) < QTRIANGULATOR_MAX); + Q_ASSERT(qAbs(m_vertices.at(i).y) < QTRIANGULATOR_MAX); } if (!(m_hint & (QVectorPath::OddEvenFill | QVectorPath::WindingFill))) @@ -772,8 +782,8 @@ QVertexSet QTriangulator::polyline() { for (int i = 0; i < m_vertices.size(); ++i) { - Q_ASSERT(qAbs(m_vertices.at(i).x) < (1 << 21)); - Q_ASSERT(qAbs(m_vertices.at(i).y) < (1 << 21)); + Q_ASSERT(qAbs(m_vertices.at(i).x) < QTRIANGULATOR_MAX); + Q_ASSERT(qAbs(m_vertices.at(i).y) < QTRIANGULATOR_MAX); } if (!(m_hint & (QVectorPath::OddEvenFill | QVectorPath::WindingFill))) @@ -803,8 +813,8 @@ for (int i = 0; i < count; ++i) { qreal x, y; matrix.map(polygon[2 * i + 0], polygon[2 * i + 1], &x, &y); - m_vertices.at(i).x = qRound(x * Q_FIXED_POINT_SCALE); - m_vertices.at(i).y = qRound(y * Q_FIXED_POINT_SCALE); + m_vertices.at(i).x = qToClampedFixedPoint(x); + m_vertices.at(i).y = qToClampedFixedPoint(y); m_indices[i] = i; } m_indices[count] = T(-1); //Q_TRIANGULATE_END_OF_POLYGON @@ -831,8 +841,8 @@ m_vertices.resize(m_vertices.size() + 1); qreal x, y; matrix.map(p[0], p[1], &x, &y); - m_vertices.last().x = qRound(x * Q_FIXED_POINT_SCALE); - m_vertices.last().y = qRound(y * Q_FIXED_POINT_SCALE); + m_vertices.last().x = qToClampedFixedPoint(x); + m_vertices.last().y = qToClampedFixedPoint(y); break; case QPainterPath::CurveToElement: { @@ -847,8 +857,8 @@ for (int j = 1; j < poly.size(); ++j) { m_indices.push_back(T(m_vertices.size())); m_vertices.resize(m_vertices.size() + 1); - m_vertices.last().x = qRound(poly.at(j).x() * Q_FIXED_POINT_SCALE / lod); - m_vertices.last().y = qRound(poly.at(j).y() * Q_FIXED_POINT_SCALE / lod); + m_vertices.last().x = qToClampedFixedPoint(poly.at(j).x() / lod); + m_vertices.last().y = qToClampedFixedPoint(poly.at(j).y() / lod); } } i += 2; @@ -866,8 +876,8 @@ m_vertices.resize(m_vertices.size() + 1); qreal x, y; matrix.map(p[0], p[1], &x, &y); - m_vertices.last().x = qRound(x * Q_FIXED_POINT_SCALE); - m_vertices.last().y = qRound(y * Q_FIXED_POINT_SCALE); + m_vertices.last().x = qToClampedFixedPoint(x); + m_vertices.last().y = qToClampedFixedPoint(y); } } m_indices.push_back(T(-1)); // Q_TRIANGULATE_END_OF_POLYGON diff --git a/tests/auto/gui/painting/CMakeLists.txt b/tests/auto/gui/painting/CMakeLists.txt index b49a80b..44d2e00 100644 --- a/tests/auto/gui/painting/CMakeLists.txt +++ b/tests/auto/gui/painting/CMakeLists.txt @@ -17,6 +17,7 @@ add_subdirectory(qpaintengine) add_subdirectory(qtransform) add_subdirectory(qpolygon) +add_subdirectory(qtriangulator) # QTBUG-87669 if(NOT ANDROID) diff --git a/tests/auto/gui/painting/qtriangulator/CMakeLists.txt b/tests/auto/gui/painting/qtriangulator/CMakeLists.txt new file mode 100644 index 0000000..f0e942b --- /dev/null +++ b/tests/auto/gui/painting/qtriangulator/CMakeLists.txt @@ -0,0 +1,20 @@ +# Copyright (C) 2026 The Qt Company Ltd. +# SPDX-License-Identifier: BSD-3-Clause + +if(NOT QT_BUILD_STANDALONE_TESTS AND NOT QT_BUILDING_QT) + cmake_minimum_required(VERSION 3.16) + project(tst_qtriangulator LANGUAGES CXX) + find_package(Qt6BuildInternals REQUIRED COMPONENTS STANDALONE_TEST) +endif() + +##################################################################### +## tst_qtriangulator Test: +##################################################################### + +qt_internal_add_test(tst_qtriangulator + SOURCES + tst_qtriangulator.cpp + LIBRARIES + Qt::Gui + Qt::GuiPrivate +) diff --git a/tests/auto/gui/painting/qtriangulator/tst_qtriangulator.cpp b/tests/auto/gui/painting/qtriangulator/tst_qtriangulator.cpp new file mode 100644 index 0000000..d28726d --- /dev/null +++ b/tests/auto/gui/painting/qtriangulator/tst_qtriangulator.cpp @@ -0,0 +1,160 @@ +// Copyright (C) 2026 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only + +#include + +#include +#include +#include + +static const qreal maxCoordinate = qreal((1 << 21) - 1) / 32; + +class tst_QTriangulator : public QObject +{ + Q_OBJECT +private slots: + void square(); + void clampRect_data(); + void clampRect(); + void clampPolygon(); + void clampWithTransform(); + void clampCurve(); + void clampPolyline(); +}; + +static quint32 indexAt(const QVertexIndexVector &indices, int i) +{ + if (indices.type() == QVertexIndexVector::UnsignedInt) + return static_cast(indices.data())[i]; + return static_cast(indices.data())[i]; +} + +static bool verticesInRange(const QList &vertices) +{ + for (qreal v : vertices) { + if (!(qAbs(v) <= maxCoordinate)) // written this way to also catch NaN + return false; + } + return true; +} + +static bool indicesInRange(const QVertexIndexVector &indices, int vertexCount) +{ + for (int i = 0; i < indices.size(); ++i) { + quint32 index = indexAt(indices, i); + if (index >= quint32(vertexCount)) + return false; + } + return true; +} + +static qreal triangleSetArea(const QTriangleSet &set) +{ + qreal area = 0.0; + for (int i = 0; i + 2 < set.indices.size(); i += 3) { + const quint32 a = indexAt(set.indices, i); + const quint32 b = indexAt(set.indices, i + 1); + const quint32 c = indexAt(set.indices, i + 2); + const qreal ax = set.vertices.at(2 * a), ay = set.vertices.at(2 * a + 1); + const qreal bx = set.vertices.at(2 * b), by = set.vertices.at(2 * b + 1); + const qreal cx = set.vertices.at(2 * c), cy = set.vertices.at(2 * c + 1); + area += qAbs((bx - ax) * (cy - ay) - (cx - ax) * (by - ay)) / 2; + } + return area; +} + +void tst_QTriangulator::square() +{ + QPainterPath path; + path.addRect(QRectF(0, 0, 64, 64)); + + const QTriangleSet set = qTriangulate(path); + QVERIFY(!set.vertices.isEmpty()); + QCOMPARE(set.indices.size() % 3, 0); + QVERIFY(verticesInRange(set.vertices)); + QVERIFY(indicesInRange(set.indices, set.vertices.size() / 2)); + QCOMPARE(triangleSetArea(set), qreal(64 * 64)); +} + +void tst_QTriangulator::clampRect_data() +{ + QTest::addColumn("rect"); + + QTest::newRow("just out of range") << QRectF(-70000, -70000, 140000, 140000); + QTest::newRow("large") << QRectF(1e6, 1e6, 1e6, 1e6); + QTest::newRow("huge") << QRectF(-1e12, -1e12, 2e12, 2e12); + QTest::newRow("extreme") << QRectF(1e30, 1e30, 1e30, 1e30); + QTest::newRow("negative extreme") << QRectF(-2e30, -2e30, 1e30, 1e30); + QTest::newRow("partially in range") << QRectF(-100, -100, 1e8, 1e8); +} + +void tst_QTriangulator::clampRect() +{ + QFETCH(QRectF, rect); + + QPainterPath path; + path.addRect(rect); + + // Out-of-range coordinates must be clamped instead of triggering + // asserts or integer overflow in the fixed point representation. + const QTriangleSet set = qTriangulate(path); + QVERIFY(verticesInRange(set.vertices)); + QVERIFY(indicesInRange(set.indices, set.vertices.size() / 2)); +} + +void tst_QTriangulator::clampPolygon() +{ + // The raw polygon overload performs no validation of its input, so pass + // extreme values directly. + const qreal polygon[] = { + -1e100, 0.0, + 1e100, -1e100, + 1e100, 1e100, + 0.0, 65536.0, + }; + + const QTriangleSet set = qTriangulate(polygon, 4); + QVERIFY(verticesInRange(set.vertices)); + QVERIFY(indicesInRange(set.indices, set.vertices.size() / 2)); +} + +void tst_QTriangulator::clampWithTransform() +{ + // In-range path coordinates mapped out of range by the transform. + QPainterPath path; + path.addRect(QRectF(0, 0, 100, 100)); + + const QTriangleSet set = qTriangulate(path, QTransform::fromScale(1e7, 1e7)); + QVERIFY(verticesInRange(set.vertices)); + QVERIFY(indicesInRange(set.indices, set.vertices.size() / 2)); +} + +void tst_QTriangulator::clampCurve() +{ + // Exercises the bezier flattening code path. + QPainterPath path; + path.moveTo(0, 0); + path.cubicTo(1e8, 0, 1e8, 1e8, 0, 1e8); + path.closeSubpath(); + + const QTriangleSet set = qTriangulate(path); + QVERIFY(verticesInRange(set.vertices)); + QVERIFY(indicesInRange(set.indices, set.vertices.size() / 2)); +} + +void tst_QTriangulator::clampPolyline() +{ + QPainterPath path; + path.moveTo(-1e8, -1e8); + path.lineTo(1e8, -1e8); + path.lineTo(1e8, 1e8); + path.lineTo(-1e8, 1e8); + path.closeSubpath(); + + const QPolylineSet set = qPolyline(path); + QVERIFY(verticesInRange(set.vertices)); +} + +QTEST_MAIN(tst_QTriangulator) + +#include "tst_qtriangulator.moc"