Raised by plc-user in discussion #618: the diagram editor allows moving
elements by as little as 1px, and asked that rotation not undershoot
that. Checking the actual constraint (Settings -> DiagramEditor_xGrid_sb
/ _yGrid_sb, both independently configurable, minimum 1, maximum 30)
turned up a real, verified gap this PR's existing fractional-pivot fix
doesn't cover: an ASYMMETRIC grid (xGrid != yGrid).
Swapping X/Y deltas for a 90-degree turn -- the exact-arithmetic path
already in this file -- only stays on the configured grid if
xGrid == yGrid. With an asymmetric grid, a delta that was a clean
multiple of xGrid lands on the Y axis after the swap, where the grid
unit is yGrid, and one is not generally a multiple of the other.
Verified on a real build (not just derived): two elements at (100,210)
and (150,420), both on-grid under xGrid=10/yGrid=7, selected and
group-rotated 90 degrees via a temporary local CLI harness.
before this change: (242,292) and (32,342) -- off-grid on both axes
after this change: (240,294) and ( 30,343) -- exactly on-grid
Confirmed the same drift is present without this change too (i.e. not
something introduced elsewhere) and that xGrid==yGrid, the common case,
is unaffected: snapping an already-on-grid point is a no-op.
Fix: re-snap the final computed position to Diagram::snapToGrid(), not
just the shared pivot, for the exact-90-degree path. Left the
arbitrary-angle trig fallback alone -- it has no caller today (the
diagram editor only ever passes multiples of 90) and "on-grid" doesn't
have a clean meaning for an arbitrary angle regardless of grid shape.
Does not attempt to fix a separate, pre-existing property surfaced
while testing this: four consecutive 90-degree turns do not reliably
return a selection to its exact starting position, even on a symmetric
grid, because each RotateSelectionCommand recomputes the pivot fresh
from the selection's current sceneBoundingRect(), and an item whose
bounding box isn't rotationally symmetric reports a different box (and
therefore a different centre) at 0 and 90 degrees. Verified this drift
is identical with and without this change, so it is not a regression --
just a different, harder guarantee this change does not attempt.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
NewDiagramPage::applyConf() writes every other default to QSettings —
border, title block, conductors, folio reports and the guides — but the
cross-reference branch only fetched the properties into a local hash and
then dropped it on the floor. hash_xrp was never used.
The result: changing the cross-reference defaults under Settings > New
project has no effect. Nothing is written, no defaultxref* key ever
appears in the configuration file, and XRefProperties::defaultProperties()
keeps handing out the hardcoded fallbacks for every new project.
Write each of the four types (coil, protection, commutator, plc) with the
"diagrameditor/defaultxref" + key prefix that defaultProperties() already
reads back.
Following up on the earlier division-by-zero fix: return -1 from the bad denominator branch of minimumWidth(), matching the "no
constraint" convention maximumWidth() already uses, instead of std::numeric_limits<int>::max() or 0. Update
TitleBlockTemplateView::updateDisplayedMinMaxWidth() to skip the "Longueur minimale" line when minimumWidth() reports -1, mirroring
its existing handling of maximumWidth() == -1.
Building QET is dominated by re-parsing Qt's headers. A 214-line source
file expands to roughly 198,000 preprocessed lines, and compiling one
translation unit costs ~4.1 s, of which only ~0.35 s is optimisation --
switching -O3 to -O0 saves just 8%, so the usual "build Debug for faster
compiles" advice does not help here. A precompiled header caches the
parsed header state, which is the part that actually costs.
Measured on a 24-thread Xeon E5-2650 v4 with Qt 5.15.18 and GCC 15.2,
same build tree, only the option differing:
compile one translation unit 4.12 s -> 1.21 s
edit one .cpp -> linked binary 5.22 s -> 1.65 s
Deliberately OFF by default. A PCH satisfies includes that a source file
neglected to make for itself, so code written with it enabled can fail to
compile for everyone else. Leaving the default off keeps CI and
contributors on the strict behaviour; only developers who opt in trade
that away for the speed.
Two details in the implementation are load-bearing:
- The generator expressions are not decoration. This target also compiles
the 18 C files of the bundled LZMA decoder, and an unguarded header list
applies to every language in the target, so the Qt headers would be fed
to the C compiler and fail with "unknown type name 'namespace'".
$<ANGLE-R> is needed because a literal '>' would end the generator
expression.
- target_precompile_headers() requires CMake 3.16 while the project still
declares a 3.5 minimum, so the block warns and skips rather than raising
the project-wide requirement for an opt-in developer feature.
Verified both ways: with the option off no PCH artefacts are generated and
the build is byte-for-byte the previous behaviour; with it on, all 18 C
files still compile, the generated PCH is C++-only (cmake_pch.hxx, with no
cmake_pch.h), the C compile commands carry no PCH, and the resulting
binary runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both actions already exist and work; they were simply only reachable from
a toolbar, and those toolbars are user-hideable via Configuration >
Afficher, so hiding one made the feature unreachable entirely.
- "Afficher les guides" (m_draw_guides) goes into the Affichage menu next
to "Afficher la grille". The two are adjacent lines in the view toolbar
and do the same kind of thing, but only the grid had a menu entry.
- "Creation automatique de conducteur(s)" (m_auto_conductor) goes into the
Projet menu. It writes a project setting via
QETProject::setAutoConductor(), so the Projet menu is where a user would
look for it; it is placed with the project properties, above a separator
that keeps the folio operations grouped as before.
Also removes conductor_default and m_project_folio_list from the header.
Both are declared but never allocated and never referenced anywhere in the
tree -- that the build still links is the proof they were dead.
No new strings: both actions already carry translated text.
Found while auditing every QAction against every menu, discussion #677.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Rotating a selection around the raw bounding-box centre moved
grid-aligned elements off the grid permanently. sceneBoundingRect()
comes from font metrics and pen widths, so the centre is almost never
a round number: with a pivot of (132.67, 101.11), an element sitting
at x=100 landed at x=133.78, and no further rotation brought it back.
Positions are written with QString::number() (%.6g), which hides the
floating-point noise but keeps the offset, so the diagram ends up
subtly misaligned with no way to repair it from the UI.
Snap the pivot with Diagram::snapToGrid(), which also follows the
user's configured X/Y grid rather than assuming the 10 px default.
Also compute the rotated offset exactly for multiples of 90 degrees
instead of going through qCos()/qSin(). The rotate actions only ever
pass right angles, and qCos(90 deg) is 6.12e-17 rather than 0, so the
trig path added error for no benefit -- four 90 degree steps did not
return a point to where it started. A quadrant is an axis swap, which
is exact; trig is kept as the fallback for any other angle.
With both, four 90 degree rotations of a grid-aligned element return
it exactly to its original position and every intermediate step stays
on the grid.
Reported by plc-user, who hit the same problem rotating graphical
primitives in the Element Editor -- discussion #618.
QetLogger (discussion #644, steps 1-3) captures whatever an explicit
qDebug()/qInfo()/qWarning() call already decided to report. Most of a
session -- painting, dragging, a slow synchronous operation -- produces
no log output at all, so a silent multi-second gap in the log is
indistinguishable from the user simply not doing anything. That gap
came up directly: investigating a user-reported "the program lagged"
required inferring stalls from timestamp gaps between unrelated log
lines, which can't tell a real freeze apart from normal idle time.
EventLoopWatchdog closes that gap directly instead of inferring it. A
QTimer::PreciseTimer repeating tick (every 50ms) measures the *actual*
elapsed time since the previous tick via QElapsedTimer (monotonic,
unaffected by system clock/NTP adjustments). Qt does not queue up
missed fires for a normal repeating timer, so if the main thread is
blocked for 600ms, the timer fires once as soon as the loop frees up,
with ~600ms measured since the last tick -- that gap is the stall,
measured at its source. Only logs (via the existing qWarning() path,
so it reuses QetLogger's file/ring/rotation with no new plumbing) when
a tick is late by more than 200ms, so a healthy session produces zero
output from this class, in keeping with QetLogger's bounded-log design.
Same QET_WATCHDOG_DISABLE=1 escape-hatch convention as QetLogger's own
QET_LOG_DISABLE=1.
Deliberately not included: attributing a stall to what caused it. This
tells you a stall happened and how long -- pairing that timestamp with
gdb attached to a running session (as used for the CLI hang, PR #661)
is still how you get from "it stalled" to a root cause.
Stacked on #647 (feature-diagnostic-logging-crash) for QetLogger/
qWarning() plumbing this depends on -- diff includes its commits until
that merges.
Verified against the compiled binary, not just read: temporarily
injected a QThread::msleep(600) via a one-shot QTimer 2s after
startup, confirmed the exact expected warning
("EventLoopWatchdog: main thread stalled for 620 ms") at the right
severity through the real qWarning()/QetLogger path, then removed the
test hook and reconfirmed a normal run produces no output from this
class at all.
Stacked on the steps 1-3 branch (feature-diagnostic-logging, PR #646).
Kept as its own PR rather than folded into that one, matching the
discussion's own framing: step 4 is explicitly "the highest-risk piece
... lands last, behind its own switch."
## Step 4 -- crash-time ring flush (CrashHandler)
Installs a handler for SIGSEGV/SIGABRT/SIGBUS/SIGFPE/SIGILL (POSIX) /
SetUnhandledExceptionFilter (Windows) that flushes the in-memory ring to
a fixed crash_dump.log before the process dies.
This required reworking LogRing (step 3) to be genuinely lock-free, not
just mutex-protected: a signal handler that blocks on a lock the
crashing thread (or another thread) already holds turns a clean crash
into a hang -- no ring dump *and* no core dump, worse than doing
nothing. append() now claims a slot with a single atomic fetch-add;
dumpToFd() reads the preallocated entries directly and writes them with
write(2) only, looping on EINTR/short writes. Accepted tradeoff: at most
one entry can be read torn if a crash lands mid-append into that exact
slot -- documented in logring.h, and the alternative (a seqlock to
detect and retry) wasn't judged worth the complexity for that window.
Other invariants implemented per the discussion:
- sigaltstack with a static 64 KiB buffer, SA_ONSTACK -- a stack-
overflow SIGSEGV has no usable stack for a handler without one.
- Nothing under the actual handler touches Qt, QString or the
allocator: the dump path and a small header (version/git/OS/Qt) are
precomputed into fixed char buffers by install(), which runs once at
startup in normal context.
- Atomic test-and-set so only the first crash writes a dump; a second
concurrent/nested fault goes straight to restore-and-re-raise.
- After writing, the handler restores SIG_DFL and re-raises (POSIX) /
returns EXCEPTION_CONTINUE_SEARCH (Windows) so the OS's own crash
path -- core dump, Windows Error Reporting -- still runs. A handler
that "fixed" the crash by swallowing the signal would destroy exactly
the post-mortem evidence this whole design exists to preserve.
Tested in this environment: POSIX/Linux only, all five signals. Sent
each directly to a running process and confirmed (a) crash_dump.log is
written with the correct header and ring contents, mode 0600, and (b)
the process still terminates via the signal with the kernel's own
"core dumped" flag set (exit code 128+signal, confirmed for all five).
The Windows path is implemented per the discussion's guidance but is
untested -- no Windows build available in this sandbox.
## Step 5 -- getting the data back out
- QETApp::checkCrashDump(), called from checkBackupFiles() only when
there's no stale project file to recover this run (so the two
prompts never both show, per the discussion), offers an unretrieved
crash dump via DiagnosticsReportDialog and then deletes it regardless
of the user's choice -- offered exactly once.
- A new "Aide > Enregistrer un rapport de diagnostic..." action
(QETMainWindow) builds the same kind of report from the *current*
session (QetLogger::buildDiagnosticsReport(): header + this session's
log file) for a manual "attach this to a bug report" flow, not tied
to a crash.
- Both go through QetLogger::redact() before ever reaching the user:
the one redaction implemented is a literal replace of the home
directory with "~", since an absolute path under it leaks the
account name. The discussion's fancier "optionally redact project
filenames too" isn't attempted -- reliably telling a project path
apart from arbitrary log text is a much fuzzier problem than a
literal prefix match.
- DiagnosticsReportDialog shows the full (already-redacted) content
before saving, per the discussion: "the user is about to attach this
to a public tracker."
Verified in a real GUI session (Xvfb): triggered a SIGSEGV, relaunched,
confirmed the crash-report dialog appears with the right header/content,
confirmed it does not reappear on a second relaunch, and confirmed the
manual "Save report" action produces a correctly-formatted report and
saves it to a chosen path.
Built clean, no new warnings.
## Build systems
Registered in both: cmake/qet_compilation_vars.cmake, and
qelectrotech.pro. The .pro needed explicit globs for the new
sources/logging/ui/ subfolder -- sources/logging/*.{h,cpp} was already
globbed, but unlike the other ui/ subfolders that one had no entry of
its own, so diagnosticsreportdialog.{h,cpp} would not have been built
under qmake.
The job trigger was narrowed to push:tags a while back, but the
job-level 'if: github.ref == refs/heads/master' was left in place.
A tag push never has github.ref == refs/heads/master, so the two
conditions are mutually exclusive: the job trigger fires only on
tag pushes, while the guard only allows master-branch refs, meaning
the job has been silently skipped on every run since the trigger
was narrowed.
This is also the likely source of the recent Git LFS bandwidth/
storage overage: while the trigger was still push-to-master (pre-
narrowing), this job ran on nearly every commit and committed a
new QElectroTech.qch (LFS-tracked) each time, via the auto-generated
update-qch PR -- accumulating one LFS object version per run."
ProjectView::initWidgets() called insertSpacing(1, 10) on a QHBoxLayout
that was still empty, inserting past the end of the item list. The
corrupted layout crashed later in QWidget::setLayout() via
QLayoutPrivate::reparentChildWidgets() and QBoxLayout::itemAt().
Use addSpacing(10) instead, which is equivalent for an empty layout.
BorderTitleBlock::slot_setAutoPageNum was removed in 471f876 ("Remove unused signal", 2023-10-17) without noticing autonumberingdockwidget.cpp still referenced it via old-style
SIGNAL()/SLOT() macros, which fail silently at runtime instead of producing a compile error. This has remained broken on master ever since; a fix (57572a2, "Fix two broken signal
connections") exists on the unmerged qt6-cmake-elevatormind-merged branch but that one only commented the lines out without solving the underlying issue.
Removed the dead code entirely and call BorderTitleBlock::importTitleBlock() directly on the active diagram in on_m_folio_cb_activated() instead. The same mechanism is already
used elsewhere in the codebase (undo command, new-diagram creation, XML loading) to push TitleBlockProperties into a diagram and trigger a folio numbering recompute via needFolioData().
27dcd5e renamed BorderTitleBlock::diagramTitleChanged to informationChanged and updated diagram.cpp accordingly, but diagramview.cpp still used the old string-based SIGNAL()/SLOT()
macro referencing the removed signal name, which failed silently at runtime instead of at compile time. The diagram/window title never refreshed after title block changes.
Observed when going through the warnings.
Updated the connect to use informationChanged with modern pointer-to-member syntax, matching diagram.cpp's own connect to the same signal.
removed in Qt6, replaced by mappedInt/mappedWidget/mappedString.
What was broken:
* the logo-conflict rename dialog
* the system tray show/hide toggle
* the Window menu
* export dialog's per-diagram preview controls
Switched to the modern mappedInt/mappedWidget signals with pointer-to-member connect(), guarded for Qt < 5.15 until Qt5 can be dropped.
Laurent Trinques