Saving an unmodified project produced a different byte stream on
every run: QGraphicsScene::items() returns items in stacking order,
and ties between same-Z items follow the scene's internal index --
not any content-derived order -- so it isn't reproducible across
process runs. The legacy terminal-id table inherits the same
instability, since ids are assigned sequentially in element order.
Sort list_elements and list_conductors into a deterministic order
before serializing, using a key built from data that's actually
stable across loads (position), not Element::uuid()/Conductor::uuid():
for an item with no persisted uuid attribute, fromXml() invents a
fresh random one on every load, so sorting by uuid would still be
non-deterministic across process runs for any legacy file -- which
this corpus has plenty of.
Also fixes a second, related source of byte-level non-determinism
found while verifying the above: Conductor::toXml() unconditionally
wrote m_uuid back out, including the synthetic value fromXml() just
invented for a conductor with no uuid attribute in the file. Every
conductor in every example project checked has no persisted uuid at
all, so this alone meant no project with conductors could ever
resave identically, regardless of ordering. Conductor gets a
m_persist_uuid flag, false only when the uuid it's holding was
synthesized rather than loaded, so toXml() stops writing a value
that was never meant to be permanent.
Deliberately NOT applying the same uuid-persistence fix to Element:
element uuids are cross-referenced by other elements' <links_uuids>
blocks for master/slave/report linking (element.cpp, tmp_uuids_link,
matched by elmt->uuid() == stored uuid on load). Making an element's
own uuid non-persistent would silently break that match for any
linked element without one already -- a real regression, not a
theoretical one. Left as a smaller, separate residual: 1-6 elements
per project across the corpus (a few tenths of a percent) still get
a fresh uuid on each load, same class of bug, needs the link-aware
version of this fix instead of this one.
Verified against 8 example projects (the ones with conductors, plus
the two zero-conductor control cases from FINDINGS.md F002), 5
resaves each in isolated HOME/XDG environments:
- Element and conductor ORDER: 0 churning sections across the whole
corpus (previously the majority of diagrams in industrial.qet,
m_000.qet and tremie_vibrante.qet churned on every run).
- Conductor uuid VALUES: 0 churn (previously every conductor in
every project, since none have a persisted uuid).
- 6 of 8 projects are now byte-for-byte identical (md5) across all 5
runs. The remaining 2 (industrial.qet, m_000.qet) differ only in
the handful of element uuids covered by the known Element residual
above -- confirmed by checking those uuids specifically, not
inferred.
- Element/conductor counts before and after resave match exactly on
every project (no data loss from the sort).
Fixes#754.
redo() has a direct-write path guarded by m_first_time: on the very
first call it writes the property immediately, and only animates on
calls after that (per setAnimated()'s documented contract). undo()
had no equivalent -- it always animated, so undo() both returned
before the property was restored (stale state visible to anything
sharing the call stack) and, with no running event loop, never
restored it at all.
The obvious fix -- reuse m_first_time in undo()'s guard too -- turns
out not to work, and I verified this with a standalone build before
picking an approach: QUndoStack::push() always calls redo() once
before any undo() can run, and redo()'s direct-write branch sets
m_first_time = true as it completes. So by the time undo() is ever
called, m_first_time has already flipped, and reusing it would make
undo() take the animate branch on every call, unconditionally --
syntactically symmetric with redo(), but behaviourally unchanged for
the exact scenario reported.
Instead, undo() gets its own m_undo_first_time flag, seeded from the
same first_time argument setAnimated() already takes, and set true by
undo()'s own direct-write branch the same way m_first_time is set by
redo()'s. That gives undo() a real, reachable direct-write path on its
own first call, independent of how many times redo() has already run.
Verified against a standalone build of just this class (as the issue's
own repro does): first redo and first undo are both now synchronous
with no event loop running; with an event loop present, both settle to
the correct value once "broken in"; behaviour for every other caller
of QPropertyUndoCommand -- everywhere that calls plain enableAnimation()
or the bare setAnimated() (first_time defaulting true) -- is provably
unchanged, since m_undo_first_time starts true either way and the
animate branch never modifies it.
Fixes#755.
- Remove dead #if QT_VERSION conditionals (both branches were identical)
- Add settings.remove() guard on restoreState() failure consistently
across all three editors (now safe since all run after show())
The Windows Qt5 build is going to be removed from CI/CD soon (Qt6/KF6
is becoming the sole supported track). Replace the 'experimental'
warning wording on the nightly download page and in the nightly
release body with a call to action inviting users to switch to Qt6
now and report issues, ahead of the Qt5 removal.
QPdfDocument::pagePointSize() (used for PDF page import) requires
Qt >= 6.4, and the QtPdf module itself is missing entirely on some
Qt6 distributions (e.g. the Flatpak org.kde.Platform runtime), since
it ships from the qtwebengine source tree rather than Qt6 core.
CMake: probe Pdf with find_package(... QUIET) instead of REQUIRED,
mirroring the existing GuiPrivate pattern. Define QET_HAS_QTPDF
only when the module is found AND Qt >= 6.4.
Replace the ad-hoc QT_VERSION_CHECK(6, 0, 0) / (6, 4, 0) guards in
qetdiagrameditor.cpp, diagrameventaddpdf.{h,cpp} and
pdfpagesdialog.{h,cpp} with #ifdef QET_HAS_QTPDF, so version and
module-availability checks live in one place.
Fixes the Flatpak build (missing Qt6Pdf) and the Windows/Debian CI
failures (QPdfDocument::pagePointSize undeclared on Qt < 6.4). The
PDF import toolbar action is now silently unavailable wherever
QtPdf isn't usable, instead of breaking the whole build.
Compiled into the binary but never instantiated -- searching the tree
for any reference outside its own three files finds nothing, and this
still holds on current master. Contains a latent bug that would be
user-visible if the widget were ever reachable
(on_tableDiagram_customContextMenuRequested compares QMenu::exec()'s
return value against one action but falls through to "select all" on
both the other action and on a plain dismiss, since exec() returns
nullptr on Escape/click-away and that's not equal to either QAction*),
which supports genuine disuse rather than temporary disconnection.
Removed the three files and their three explicit entries in
cmake/qet_compilation_vars.cmake (qelectrotech.pro globs sources/ui/*
so needs no change). Builds clean; no other file references
diagramselection.
Fixes#756.
checkConflicts() compared key sequences across the whole registry, but
QElectroTech deliberately registers one key per editor window: Undo,
Redo, New, Open, Save and Ctrl+Shift+S each exist three times, once for
the diagram, element and titleblock editors. Those are not collisions --
they act on different windows.
The result was that 60 of the 95 shipped default bindings displayed as
conflicts, so the indicator carried no information and the page looked
broken on first open.
Conflicts are now keyed on (category, sequence). The category is the
registry's existing per-window grouping, so no new concept and no
ShortcutManager API change is needed.
Fixes#757
m_jump_to_element was the only action in the tree that set its
QKeySequence directly instead of going through
ShortcutManager::registerAction() -- of 98 actions carrying a runtime
shortcut, 95 matched a registerAction() call, 2 were Qt built-ins, and
this was the sole exception (verified by dumping every QAction from a
running instance and cross-checking against a static scan of the
source; the only other setShortcut() call in the tree clears a
shortcut rather than setting one).
Bypassing the registry meant the binding didn't appear on the
Shortcuts preferences page (so it couldn't be discovered or rebound),
and checkConflicts() couldn't see it either, so assigning Ctrl+G to
another action there would silently collide at runtime instead of
being flagged.
Fixes#758.
Apply the same split readSettings()/readSettingsState() pattern from
QETDiagramEditor to the other two main windows:
- QETElementEditor: split in constructor, call readSettingsState() after show()
- QETTitleBlockTemplateEditor: split readSettings(), callers call
readSettingsState() after show() (newTemplate + 2x openTitleBlockTemplate)
- Remove destructive settings.remove() guards that would delete saved
state on every Qt6 launch when restoreState() fails before show()
Co-authored-by: ispyisail
- Add DPI selection (150/300/600) to the page selection dialog
- Add live page preview in the selection dialog
- Conditionally compile PDF import only for Qt6 (#if QT_VERSION)
- Add custom pdf-import icon (PDF document with + symbol)
- Register new icon in qelectrotech.qrc
- Qt5 builds: PDF import action is hidden, everything else works as before
setTabVisible(1, ...) only allowed Simple and Master, but the tree
(updateTree()) and the actual write path (ElementScene::toXml()) both
already support elementInformations for Terminal and Thumbnail too --
three independent "is this type allowed" checks that were never
reconciled, leaving already-working support unreachable through the
UI for those two types. Slave stays excluded here, consistent with
having no elementInformations support at either of the other two
points either (separate, larger gap, not addressed here).
"false" writes, delete key instead
Unchecking a checkbox previously wrote the key with value "false"
rather than omitting it -- behaviorally identical to every consumer
(all three do a case-sensitive == "true" comparison), but left dead
entries cluttering the .elmt file, inconsistent with how other
elementInformation fields (manufacturer, designation) are only present
when actually set. Now removes the key entirely when unchecked.
numericInfoPattern()'s integer branch no longer allows an optional
trailing ".", so "12." is now Intermediate rather than Acceptable --
already-built hasAcceptableInput() guard then keeps it from being
stored, same as a lone ".". Previously it validated fine and got
saved verbatim, silently freezing in that form since nothing ever
normalized it afterward.
- ElementInfoWidget::currentInfo() now skips a field whose validator
hasn't accepted its text (e.g. a lone "." mid-typing), which
previously stored and later parsed to 0.
- New QETInformation::NumericInfoValidator rewrites "," to "." before
validating, so 80,5 on a German/French keyboard no longer silently
becomes 805. Used at both existing call sites.
- Restored the header's #1/#2/#3 doc comment (was reflowed into a
run-on paragraph by a previous edit).
excludedConductorCount() counted conductors whose terminals had no uuid,
which was the right rule when that was the reason they were dropped. It no
longer is: Terminal::stableUuid() derives an identity from the terminal's
geometry, so those conductors are in the table.
Left unchanged, the dialog would have told the user "671 conductors excluded"
on industrial.qet while listing all 671 of them -- a worse failure than the
one the count exists to prevent, because it undermines a list that is now
correct.
The count and the dialog's explanation both now describe the case that
actually remains: an endpoint attached to no element at all, which has no
identity to key on under any scheme.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The summary line exists so that an empty wiring list is distinguishable
from one where every conductor was excluded, and it was reporting the wrong
number to do it. QSqlQueryModel fetches lazily, so rowCount() straight after
setQuery() returns the rows fetched so far -- 256 -- not the size of the
query. Measured with Qt's own QSQLITE driver: a 1000-row view reports 256
until the model is drained, then 1000. The test project quoted in slice 2
has 280 conductors, so this was already displaying 256 on our own data,
plausibly enough that nobody looked twice.
Drain the model before reading the count.
Also refresh the database before building the model. The dialog queries the
database rather than the diagrams, so anything not yet written through was
invisible here; with conductor text now updated on change that gap is
smaller, but a project loaded before this dialog was ever opened still
relies on the repopulate.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The element and element_info tables had two independent insert paths --
addElement() for an element added to a live diagram, and
populateElementTable()/populateElementInfoTable() for a full rebuild --
which bound the same row differently. The incremental path wrote
kindInformations()["type"] into element.sub_type; the bulk path wrote
elementData().masterTypeToString(). So the table held different values
depending on whether the project had been reloaded since the element was
placed, and element_nomenclature_view exposes that column as
element_sub_type, which ElementQueryWidget filters on for the Coil,
Protection, Commutator and PLC nomenclature options.
That divergence is the same shape as the type-filter one fixed in the
previous commit, and it is the reason this stack kept finding bugs that
were invisible while editing and only appeared after a reload. Rather
than correct a second instance of it, both paths now go through
bindElementValues() and bindElementInfoValues(), following the
bindDiagramInfoValues() helper this class already had. Live and reloaded
now agree by construction instead of by coincidence.
The bulk path's values are the ones kept, because they are what every
already saved project contains: nothing a reload produces changes, and
the previous commit's 19-project BOM regression stays valid. It is the
live path that moves, onto the values a reload would have given it
anyway.
Measured, placing one element into a new project and then saving and
reopening it:
live element table: slave/ x1
reloaded element table: slave/ x1
and for the same element, what the two paths would have stored:
bulk (now shared): "" incremental (before this commit): "simple"
Re-ran the BOM regression over the same 19 projects after this change:
content identical to the pre-change baseline on all 19, and identical
line-for-line on 18, the exception being the three byte-identical
photovoltaique rows already described in the previous commit.
Note for anyone reading masterTypeToString(): the const no-argument
overload returns an empty string for anything that is not a Master, so
the "coil" fallback in the static overload is only reached for real
master elements. Non-master elements get an empty sub_type, not a
spurious "coil".
Closes the gap left open by the previous commit, at the root rather than
around it.
populateElementTable()/populateElementInfoTable() only inserted elements
matching Simple|Terminal|Master|Thumbnail. That quietly made the element
table mean "the elements a nomenclature cares about" rather than "the
elements of the project": slave elements (relay contacts) and report
elements -- ordinary conductor endpoints -- had no row at all after a
project load, so the wiring list could not name either end of a wire
that terminated on one.
Both tables are now populated with every ElementData::Type, and the type
restriction moves into element_nomenclature_view, which is where a
"what belongs in a bill of materials" decision belongs. The mask in the
view is character-for-character the one the population used to apply, so
a relay contact is still not a BOM line item.
This is safe to do in one place because every consumer of the project
database goes through a view: element_nomenclature_view (the on-diagram
nomenclature table via ElementQueryWidget, the BOM dialog, and the
--export-bom CLI) or project_summary_view (which does not reference
element at all). Nothing queries the element or element_info tables
directly -- checked across the whole tree.
Regression evidence. --export-bom runs updateDB() and then queries
element_nomenclature_view, so it is an exact harness for what the GUI
BOM shows. Captured for 19 projects (all 17 usable examples/ plus two
slave-element fixtures) before and after:
- BOM content byte-identical on all 19, compared as a multiset.
- 18 of 19 are also identical line-for-line in order.
- photovoltaique differs only in the position of three byte-identical
rows among themselves. Its query is ORDER BY label and those rows
share an empty label, so their relative order was never defined;
they are indistinguishable in the output. The on-diagram
nomenclature orders by every displayed column, so a tie there means
the rows are identical on screen too.
Effect on the wiring list, same project and same reload path: element_info
rows 0 -> 2, and the two component columns go from blank to K2 -> K1.
Cost: the database phase of loading examples/industrial.qet (150 folios,
1794 terminals) moves from 0.210 s to 0.233 s.
Slice 4 of discussion #503, on top of slice 3 (#629): the smallest
surface that makes wiring_list_view visible, plus the diagnostic the
view needs to be honest about what it is missing.
Projet > "Liste de câblage (base de données)" opens a read-only table of
wiring_list_view, headed by a line stating how many conductors are
listed and, when non-zero, how many were excluded and why.
Deliberately not another exporter. QET already ships a wiring-list CSV
export (Projet > Exporter le plan de câblage, and --export-cables) which
walks the project XML; measured on the same projects it produces a row
per conductor and resolves labels correctly when the project has them.
Adding a second, competing CSV would be worse, not better -- the
database path's value is what it unlocks (terminal plans, BOM joins),
not replacing that export.
projectDataBase::excludedConductorCount() counts, from the live scene,
the conductors deliberately absent from the conductor table because a
terminal has no uuid. Counted from the scene precisely because the
database is where those conductors are not. Verified: 671 on
examples/industrial.qet (which has 1794 terminals and no terminal uuids
at all, so its list is empty and now says so), 0 on a project whose
elements do carry terminal uuids.
KNOWN GAP, not fixed here and the reason this is opened for discussion
rather than merge: after a save/reload the component columns are blank
for slave elements. populateElementTable()/populateElementInfoTable()
only insert Simple|Terminal|Master|Thumbnail, so slave elements -- relay
contacts, i.e. a large share of real wire endpoints -- have no row in
element_info for the view to read a label from. Measured on a two-slave-
contact project after reload: element rows 0, element_info rows 0,
terminal rows 2, conductor rows 1; the wire is listed (slice 3's LEFT
JOIN keeps it) but both component names are empty, where the existing
CSV export shows K1 -> K2 for the same file.
Closing that gap means widening a filter shared with the nomenclature
and summary views, which would change what those existing, shipped
features contain. That is a maintainer decision, not one to take
unilaterally inside an additive slice.
The wiring_list_view added by this slice was only reachable through the GUI,
which meant the one thing worth proving about it -- that it still describes
the project -- could not be checked without a person clicking. This is the
same shape as the existing --export-bom, which reads
element_nomenclature_view, and it makes the view verifiable in CI.
It also makes this slice useful on its own: a from-to wiring list is a thing
people want as a CSV, and it no longer waits on the dialog in the next slice.
There is deliberately an overlap with --export-cables, which produces the same
logical list from the document XML rather than the database. Keeping both is
the point: running them and diffing them is a direct check that the cache and
the document still agree, which nothing else in the codebase can do.
Measured on the example corpus, the two also differ in what they can actually
fill in. Rows carrying any endpoint data:
--export-cables --export-wiring
industrial.qet 0 / 671 541 / 671
m_000.qet 0 / 457 362 / 457
affuteuse_250h.qet 0 / 263 197 / 263
tremie_vibrante.qet 0 / 77 61 / 77
tableau_domestique.qet 58 / 130 104 / 130
Both return a row per conductor; the XML-derived one leaves the component and
terminal columns empty on the older projects, and emits an unresolved "%id"
in its folio column. That is not an argument for removing it -- it carries
columns the view does not, and it is the independent second opinion -- but it
does mean the database path is the one with the data on the projects people
actually have.
The terminal-name columns come back empty on most projects. That is absent
source data, not a loss in transit: tableau_domestique.qet has no terminal
name on 457 of 457 terminals, and industrial.qet stores the "_" placeholder
on 1421 of 1790.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ispyisail