Never use a folio grid step below 1 from the settings

The folio grid steps, diagrameditor/Xgrid and Ygrid, are read from the
settings in six places with a plain toInt() and used as they come. The
preferences page cannot store anything below 1, but a hand-edited or
damaged settings file can hold 0, a negative number or text:

- Diagram::snapToGrid() divides by the step, so 0 is a SIGFPE on the
  first click that places a symbol;
- Diagram::drawBackground() runs "while (g_x % xGrid)" and then loops
  "gx += xGrid", so 0 crashes every repaint and a negative step never
  ends;
- the paste, duplicate and align paths divide by it or multiply with it.

Add foliogrid.h, a header-only helper: FolioGrid::step() reads a
settings entry and returns the built-in step (Diagram::xGrid, 10) when
the entry is missing, not a number, below 1 or above what an int
holds (QVariant::toInt() wraps such a value around). Every reader of
the two keys goes through it; the settings page, which only writes the
spin box values, is unchanged.

No file-format change, and no change for any step the preferences page
can produce.

Tests: tst_foliogrid covers the helper: 1, 10, "7", int max are kept;
missing, 0, -5, "ten", "", "nan", 99999999999 (as text and as a
number) and int max + 1 fall back; 7.9 rounds to 8 as before; and the
two settings keys through a QSettings scope of the test's own. The
helper is new, so the test cannot fail on master; the six readers are
the replacements in the diff.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Beat Hangartner <beat@hangartners.ch>
This commit is contained in:
Beat Hangartner
2026-10-08 06:04:35 +02:00
parent 51675c8a65
commit ede7bd99e1
7 changed files with 178 additions and 26 deletions
+7 -12
View File
@@ -45,6 +45,7 @@
#include "diagramsortkeys.h"
#include "itemgroups.h"
#include "textgrid.h"
#include "foliogrid.h"
#include <QGraphicsView>
#include <QTextStream>
#include <algorithm>
@@ -339,10 +340,8 @@ void Diagram::drawBackground(QPainter *p, const QRectF &r) {
p -> setBrush(Qt::NoBrush);
int xGrid = settings.value(QStringLiteral("diagrameditor/Xgrid"),
Diagram::xGrid).toInt();
int yGrid = settings.value(QStringLiteral("diagrameditor/Ygrid"),
Diagram::yGrid).toInt();
const int xGrid = FolioGrid::step(settings.value(FolioGrid::x_key), Diagram::xGrid);
const int yGrid = FolioGrid::step(settings.value(FolioGrid::y_key), Diagram::yGrid);
qreal limit_x = rect.x() + rect.width();
qreal limit_y = rect.y() + rect.height();
@@ -2946,10 +2945,8 @@ DiagramPosition Diagram::convertPosition(const QPointF &pos) {
QPointF Diagram::snapToGrid(const QPointF &p)
{
QSettings settings;
int xGrid = settings.value(QStringLiteral("diagrameditor/Xgrid"),
Diagram::xGrid).toInt();
int yGrid = settings.value(QStringLiteral("diagrameditor/Ygrid"),
Diagram::yGrid).toInt();
const int xGrid = FolioGrid::step(settings.value(FolioGrid::x_key), Diagram::xGrid);
const int yGrid = FolioGrid::step(settings.value(FolioGrid::y_key), Diagram::yGrid);
//Return a point rounded to the nearest pixel
if (QApplication::keyboardModifiers().testFlag(Qt::ControlModifier))
@@ -2981,10 +2978,8 @@ QPointF Diagram::snapToTextGrid(const QPointF &p)
: settings.value(TextGrid::settings_key, 1).toReal();
return TextGrid::snap(p,
settings.value(QStringLiteral("diagrameditor/Xgrid"),
Diagram::xGrid).toInt(),
settings.value(QStringLiteral("diagrameditor/Ygrid"),
Diagram::yGrid).toInt(),
FolioGrid::step(settings.value(FolioGrid::x_key), Diagram::xGrid),
FolioGrid::step(settings.value(FolioGrid::y_key), Diagram::yGrid),
divisor);
}
@@ -19,6 +19,7 @@
#include "../autoNum/ui/pastenumberingimport.h"
#include "../diagram.h"
#include "../foliogrid.h"
#include "../diagramcommands.h"
#include "../qetapp.h"
#include "../qetdiagrameditor.h"
@@ -108,10 +109,8 @@
}
const QPointF top_left = items_rect.topLeft();
QSettings settings;
const int xGrid = settings.value(QStringLiteral("diagrameditor/Xgrid"),
Diagram::xGrid).toInt();
const int yGrid = settings.value(QStringLiteral("diagrameditor/Ygrid"),
Diagram::yGrid).toInt();
const int xGrid = FolioGrid::step(settings.value(FolioGrid::x_key), Diagram::xGrid);
const int yGrid = FolioGrid::step(settings.value(FolioGrid::y_key), Diagram::yGrid);
const auto snapGrid = [xGrid, yGrid](const QPointF &p) -> QPointF {
return QPointF(
qRound(p.x() / xGrid) * xGrid,
@@ -287,10 +286,8 @@ void DiagramEventAddPaste::showHint()
void DiagramEventAddPaste::moveTo(const QPointF &scene_pos)
{
QSettings settings;
const int xGrid = settings.value(QStringLiteral("diagrameditor/Xgrid"),
Diagram::xGrid).toInt();
const int yGrid = settings.value(QStringLiteral("diagrameditor/Ygrid"),
Diagram::yGrid).toInt();
const int xGrid = FolioGrid::step(settings.value(FolioGrid::x_key), Diagram::xGrid);
const int yGrid = FolioGrid::step(settings.value(FolioGrid::y_key), Diagram::yGrid);
const auto snapGrid = [xGrid, yGrid](const QPointF &p) -> QPointF {
return QPointF(
+3 -4
View File
@@ -40,6 +40,7 @@
#include "utils/conductorcreator.h"
#include "undocommand/addgraphicsobjectcommand.h"
#include "diagram.h"
#include "foliogrid.h"
#include "diagramcontexttoolbar.h"
#include "diagramgestureoverlay.h"
#include "gesturesettings.h"
@@ -643,10 +644,8 @@ void DiagramView::duplicate(const QPoint &stepOffset)
if (selection.isEmpty()) return;
QSettings settings;
const int x_grid = settings.value(QStringLiteral("diagrameditor/Xgrid"),
Diagram::xGrid).toInt();
const int y_grid = settings.value(QStringLiteral("diagrameditor/Ygrid"),
Diagram::yGrid).toInt();
const int x_grid = FolioGrid::step(settings.value(FolioGrid::x_key), Diagram::xGrid);
const int y_grid = FolioGrid::step(settings.value(FolioGrid::y_key), Diagram::yGrid);
const QPointF offset(stepOffset.x() * x_grid, stepOffset.y() * y_grid);
// Mirrors copy(), but does not touch the system clipboard: Ctrl+D
+54
View File
@@ -0,0 +1,54 @@
/*
Copyright 2006-2026 The QElectroTech Team
This file is part of QElectroTech.
QElectroTech is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 2 of the License, or
(at your option) any later version.
QElectroTech is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with QElectroTech. If not, see <http://www.gnu.org/licenses/>.
*/
#ifndef FOLIOGRID_H
#define FOLIOGRID_H
#include <QString>
#include <QVariant>
#include <limits>
/**
The folio grid: the step, in pixels, that symbols snap to. Both steps
are kept in the settings; the preferences page only offers 1 and up,
but a hand-edited or damaged settings file can hold 0, a negative
number or text, and the code that divides by the step or loops over it
must never see such a value.
*/
namespace FolioGrid
{
/// QSettings keys holding the two steps.
inline const QString x_key{QStringLiteral("diagrameditor/Xgrid")};
inline const QString y_key{QStringLiteral("diagrameditor/Ygrid")};
/**
@return the step held in @p value, a settings entry; or @p fallback,
the built-in step, when the entry is missing, not a number, less
than 1, or more than an int holds (QVariant::toInt() wraps such a
number around instead of failing).
*/
inline int step(const QVariant &value, int fallback)
{
bool ok = false;
const qlonglong s = value.toLongLong(&ok);
if (!ok || s < 1 || s > std::numeric_limits<int>::max())
return fallback;
return int(s);
}
}
#endif // FOLIOGRID_H
@@ -20,6 +20,7 @@
#include "../QPropertyUndoCommand/qpropertyundocommand.h"
#include "../alignment.h"
#include "../diagram.h"
#include "../foliogrid.h"
#include "../diagramcontent.h"
#include "../itemgroups.h"
#include "../qetgraphicsitem/diagramimageitem.h"
@@ -121,8 +122,8 @@ AlignSelectionCommand::AlignSelectionCommand(Diagram *diagram, Mode mode, QUndoC
m_locked_count = dc.removeNonMovableItems();
QSettings settings;
const int x_grid = settings.value(QStringLiteral("diagrameditor/Xgrid"), Diagram::xGrid).toInt();
const int y_grid = settings.value(QStringLiteral("diagrameditor/Ygrid"), Diagram::yGrid).toInt();
const int x_grid = FolioGrid::step(settings.value(FolioGrid::x_key), Diagram::xGrid);
const int y_grid = FolioGrid::step(settings.value(FolioGrid::y_key), Diagram::yGrid);
const qreal text_divisor = settings.value(TextGrid::settings_key, 1).toReal();
//Each kind goes where dragging it would have left it: symbols,
+7
View File
@@ -128,6 +128,13 @@ add_test(NAME tst_textgrid COMMAND tst_textgrid)
target_include_directories(tst_textgrid PRIVATE ${QET_DIR}/sources)
target_link_libraries(tst_textgrid PRIVATE Qt::Test)
# foliogrid.h is header-only: the folio grid step as read from the
# settings, never 0 or negative, so no reader can divide by it or loop on it.
add_executable(tst_foliogrid tst_foliogrid.cpp)
target_include_directories(tst_foliogrid PRIVATE ${QET_DIR}/sources)
target_link_libraries(tst_foliogrid PRIVATE Qt::Test Qt::Core)
add_test(NAME tst_foliogrid COMMAND tst_foliogrid)
# textlines.h is header-only: the lines of a text as they are laid out,
# for the exports that write a text line by line.
add_executable(tst_textlines tst_textlines.cpp)
+99
View File
@@ -0,0 +1,99 @@
/*
Copyright 2006-2026 The QElectroTech Team
This file is part of QElectroTech.
QElectroTech is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 2 of the License, or
(at your option) any later version.
QElectroTech is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with QElectroTech. If not, see <http://www.gnu.org/licenses/>.
*/
#include "foliogrid.h"
#include <QtTest>
#include <QSettings>
/**
FolioGrid::step() -- the folio grid step as read from the settings.
Every reader of diagrameditor/Xgrid and Ygrid goes through it, so a
settings file holding 0 (a division by zero in Diagram::snapToGrid()
and an endless "% 0" in drawBackground()), a negative number (an
endless loop) or text can no longer reach the code that uses the step.
*/
class tst_foliogrid : public QObject
{
Q_OBJECT
private slots:
void usableValuesAreKept_data()
{
QTest::addColumn<QVariant>("value");
QTest::addColumn<int>("expected");
QTest::newRow("one") << QVariant(1) << 1;
QTest::newRow("default") << QVariant(10) << 10;
QTest::newRow("text number") << QVariant(QStringLiteral("7")) << 7;
QTest::newRow("large") << QVariant(100000) << 100000;
QTest::newRow("int max") << QVariant(2147483647) << 2147483647;
}
void usableValuesAreKept()
{
QFETCH(QVariant, value);
QFETCH(int, expected);
QCOMPARE(FolioGrid::step(value, 10), expected);
}
void unusableValuesFallBack_data()
{
QTest::addColumn<QVariant>("value");
QTest::newRow("missing") << QVariant();
QTest::newRow("zero") << QVariant(0);
QTest::newRow("negative") << QVariant(-5);
QTest::newRow("text") << QVariant(QStringLiteral("ten"));
QTest::newRow("empty") << QVariant(QString());
QTest::newRow("nan") << QVariant(QStringLiteral("nan"));
QTest::newRow("too large") << QVariant(QStringLiteral("99999999999"));
QTest::newRow("too large number") << QVariant(qlonglong(99999999999));
QTest::newRow("int max + 1") << QVariant(qlonglong(2147483648));
}
void unusableValuesFallBack()
{
QFETCH(QVariant, value);
QCOMPARE(FolioGrid::step(value, 10), 10);
QCOMPARE(FolioGrid::step(value, 20), 20);
}
// A decimal in the file is rounded to a whole number, as the plain
// QVariant::toInt() read did before.
void decimalIsRoundedLikeBefore()
{
QCOMPARE(FolioGrid::step(QVariant(7.9), 10), 8);
QCOMPARE(FolioGrid::step(QVariant(0.4), 10), 10);
}
// Through a real settings file, in a scope of this test's own.
void readsTheSettingsKeys()
{
QCoreApplication::setOrganizationName(QStringLiteral("QElectroTech-tst_foliogrid"));
QCoreApplication::setApplicationName(QStringLiteral("tst_foliogrid"));
QSettings settings;
settings.clear();
QCOMPARE(FolioGrid::step(settings.value(FolioGrid::x_key), 10), 10);
settings.setValue(FolioGrid::x_key, 0);
settings.setValue(FolioGrid::y_key, 15);
QCOMPARE(FolioGrid::step(settings.value(FolioGrid::x_key), 10), 10);
QCOMPARE(FolioGrid::step(settings.value(FolioGrid::y_key), 10), 15);
settings.clear();
}
};
QTEST_GUILESS_MAIN(tst_foliogrid)
#include "tst_foliogrid.moc"