Reload element drawings: keep old drawing on failure, skip elements whose geometry changed (#802)

- Element::reloadPicture() now returns a ReloadPictureResult and never
  clears the current drawing before a successful rebuild: a missing or
  unreadable definition leaves the element as it was instead of blank.
- Elements whose size, hotspot or terminals (added, removed or moved)
  differ from the new definition are not redrawn: the new drawing would
  no longer match their bounding rect and live terminals.
- The action lists those elements and warns that they must be removed
  and re-inserted, which deletes the conductors already connected to
  them.
- Status tip states the action is not undoable.

sources/qetdiagrameditor.cpp

@@ -555,7 +555,7 @@ void QETDiagramEditor::setUpActions()
 	m_reload_element_drawings = new QAction(QET::Icons::ViewRefresh, tr("Recharger les dessins des éléments"), this);
 	m_reload_element_drawings->setStatusTip(
 		tr("Redessine chaque élément placé d'après sa définition actuelle,"
-		   " sans avoir à fermer et rouvrir le projet"));
+		   " sans avoir à fermer et rouvrir le projet (action non annulable)"));
 	connect(m_reload_element_drawings, &QAction::triggered, this, &QETDiagramEditor::slot_reloadElementDrawings);
 
 	#ifdef QET_EXPORT_PROJECT_DB
@@ -2990,9 +2990,10 @@ void QETDiagramEditor::slot_terminalNumbering() {
 	is closed and reopened.
 
 	Purely visual and not undoable, the way pressing a "refresh" button
-	would be: it does not touch position, rotation, links, elementInformations,
+	would be: nothing is pushed on the undo stack and the project is not
-	labels or dynamic texts, and does not detect or handle a definition whose
+	marked as modified. Elements whose size, hotspot or terminals changed
-	terminals moved -- those still need the usual remove-and-reinsert.
+	are skipped and listed: they must be removed and re-inserted, which
+	deletes the conductors already connected to them.
 */
 void QETDiagramEditor::slot_reloadElementDrawings() {
 	QETProject *project = currentProject();
@@ -3015,12 +3016,62 @@ void QETDiagramEditor::slot_reloadElementDrawings() {
 		}
 	}
 
-	for (Element *elmt : elements) {
+	int reloaded = 0;
-		elmt->reloadPicture();
+	int unavailable = 0;
+	QStringList geometry_changed;
+	for (Element *elmt : elements)
+	{
+		switch (elmt->reloadPicture())
+		{
+			case Element::ReloadPictureResult::Reloaded:
+				++reloaded;
+				break;
+			case Element::ReloadPictureResult::Unavailable:
+				++unavailable;
+				break;
+			case Element::ReloadPictureResult::GeometryChanged:
+			{
+				const Diagram *diagram = elmt->diagram();
+				const QString folio = diagram
+						? tr("folio %1").arg(project->folioIndex(diagram) + 1)
+						: QString();
+				geometry_changed << QStringLiteral("%1 (%2)").arg(elmt->name(), folio);
+				break;
+			}
+		}
 	}
 
-	QET::QetMessageBox::information(
+	QString message = tr("%n élément(s) redessiné(s).", "", reloaded);
-		this,
+
-		tr("Recharger les dessins des éléments"),
+	if (unavailable) {
-		tr("%n élément(s) redessiné(s).", "", elements.size()));
+		message += QStringLiteral("\n\n")
+				% tr("%n élément(s) dont la définition est introuvable ou illisible :"
+					  " leur dessin actuel a été conservé.", "", unavailable);
+	}
+
+	if (geometry_changed.isEmpty())
+	{
+		QET::QetMessageBox::information(
+			this, tr("Recharger les dessins des éléments"), message);
+		return;
+	}
+
+	message += QStringLiteral("\n\n")
+			% tr("%n élément(s) non redessiné(s) : leur taille, leur point de saisie"
+				  " ou leurs bornes ont changé (borne ajoutée, supprimée ou déplacée).",
+				  "", geometry_changed.size())
+			% QStringLiteral("\n\n")
+			% tr("Pour les mettre à jour, il faut les supprimer puis les réinsérer."
+				  " Attention : cette opération supprime les conducteurs déjà reliés"
+				  " à ces éléments, qu'il faudra retracer.");
+
+		//The full list goes in the expandable, scrollable details area
+		//so the dialog stays readable on large projects.
+	QMessageBox box(QMessageBox::Warning,
+					tr("Recharger les dessins des éléments"),
+					message,
+					QMessageBox::Ok,
+					this);
+	box.setDetailedText(geometry_changed.join(QLatin1Char('\n')));
+	box.exec();
 }

sources/qetgraphicsitem/element.cpp

@@ -1792,24 +1792,129 @@ ElementsLocation Element::location() const
 	(buildFromXml()), and nothing afterwards ever makes it look again --
 	editing and saving the definition leaves every already-placed instance
 	showing the old drawing until the project is closed and reopened
-	(bugtracker #802). This is the per-instance half of the fix: the caller
+	(bugtracker #802). This is the per-instance half of the fix.
-	is expected to have already dropped the shared ElementPictureFactory
-	cache for this location, or the fresh fetch below just returns the same
-	cached picture unchanged.
 
-	Deliberately limited to the drawing: terminals and dynamic texts are
+	Deliberately limited to the drawing. Terminals are what conductors are
-	live objects carrying state a reload cannot safely fabricate --
+	attached to: if the new definition adds, removes or moves a terminal,
-	terminal positions are what conductors are attached to, and dynamic
+	or changes the element size or hotspot, the new drawing would no longer
-	texts carry per-instance overrides (position, visibility) a rebuild
+	match the live terminals and bounding rect. Such an element is left
-	would have to invent a default for. A definition whose terminals moved
+	untouched and GeometryChanged is returned; it has to be removed and
-	still needs the element removed and re-inserted, same as today.
+	re-inserted, which deletes the conductors already connected to it.
+
+	If the definition cannot be found or read, the current drawing is kept
+	and Unavailable is returned, so the element never goes blank.
+
+	Purely visual: nothing is pushed on the undo stack and the project is
+	not marked as modified.
+	@return what happened to this element
 */
-void Element::reloadPicture()
+Element::ReloadPictureResult Element::reloadPicture()
 {
-	m_picture = QPicture();
+	if (!m_location.exist()) {
-	m_low_zoom_picture = QPicture();
+		return ReloadPictureResult::Unavailable;
-	ElementPictureFactory::instance()->getPictures(m_location, m_picture, m_low_zoom_picture);
+	}
+
+	const QDomElement definition = m_location.xml();
+	if (definition.isNull()) {
+		return ReloadPictureResult::Unavailable;
+	}
+
+	if (!definitionGeometryMatches(definition)) {
+		return ReloadPictureResult::GeometryChanged;
+	}
+
+	QPicture picture;
+	QPicture low_zoom_picture;
+	ElementPictureFactory::instance()->getPictures(m_location,
+												   picture,
+												   low_zoom_picture);
+	if (picture.isNull()) {
+		return ReloadPictureResult::Unavailable;
+	}
+
+	m_picture = picture;
+	m_low_zoom_picture = low_zoom_picture;
 	update();
+	return ReloadPictureResult::Reloaded;
+}
+
+/**
+	@brief Element::definitionGeometryMatches
+	Compare the geometry described by @p definition with this live element:
+	size and hotspot (normalized the same way setSize()/setHotspot() do it)
+	and the set of terminal positions (same parsing rules as
+	TerminalData::fromXml()).
+	@param definition : the <definition> root of the element
+	@return true if the new drawing can be applied without desynchronizing
+	the bounding rect or the terminals
+*/
+bool Element::definitionGeometryMatches(const QDomElement &definition) const
+{
+	int w = 0, h = 0, hot_x = 0, hot_y = 0;
+	if (!QET::attributeIsAnInteger(definition, QStringLiteral("width"), &w)         ||
+		!QET::attributeIsAnInteger(definition, QStringLiteral("height"), &h)        ||
+		!QET::attributeIsAnInteger(definition, QStringLiteral("hotspot_x"), &hot_x) ||
+		!QET::attributeIsAnInteger(definition, QStringLiteral("hotspot_y"), &hot_y)) {
+		return false;
+	}
+
+		//Same rounding as setSize()
+	while (w % 10) ++w;
+	while (h % 10) ++h;
+	if (QSize(w, h) != dimensions) {
+		return false;
+	}
+
+		//Same clamping as setHotspot()
+	const QPoint new_hotspot = dimensions.isNull()
+			? QPoint(0, 0)
+			: QPoint(qMin(hot_x, w), qMin(hot_y, h));
+	if (new_hotspot != hotspot_coord) {
+		return false;
+	}
+
+		//Terminal positions described by the new definition
+	QList<QPointF> new_terminals;
+	for (QDomElement description = definition.firstChildElement(QStringLiteral("description")) ;
+		 !description.isNull() ;
+		 description = description.nextSiblingElement(QStringLiteral("description")))
+	{
+		for (QDomElement terminal = description.firstChildElement(QStringLiteral("terminal")) ;
+			 !terminal.isNull() ;
+			 terminal = terminal.nextSiblingElement(QStringLiteral("terminal")))
+		{
+			qreal x = 0.0, y = 0.0;
+			if (QET::attributeIsAReal(terminal, QStringLiteral("x"), &x) &&
+				QET::attributeIsAReal(terminal, QStringLiteral("y"), &y)) {
+				new_terminals << QPointF(x, y);
+			}
+		}
+	}
+
+	if (new_terminals.size() != m_terminals.size()) {
+		return false;
+	}
+
+		//Every live terminal must still exist at the same place
+	for (const Terminal *terminal : m_terminals)
+	{
+		const QPointF live_pos = mapFromScene(terminal->dockConductor());
+		bool found = false;
+		for (int i = 0 ; i < new_terminals.size() ; ++i)
+		{
+			const QPointF delta = new_terminals.at(i) - live_pos;
+			if (qAbs(delta.x()) < 0.01 && qAbs(delta.y()) < 0.01) {
+				new_terminals.removeAt(i);
+				found = true;
+				break;
+			}
+		}
+		if (!found) {
+			return false;
+		}
+	}
+
+	return true;
 }
 
 /**

sources/qetgraphicsitem/element.h

@@ -83,6 +83,7 @@ class Element : public QetGraphicsItem
 			Element::kind link_type = Element::Simple);
 		~Element() override;
 	private:
+		bool definitionGeometryMatches(const QDomElement &definition) const;
 		Element(const Element &);
 
 		// attributes
@@ -153,7 +154,13 @@ class Element : public QetGraphicsItem
 
 		QString name() const override;
 		ElementsLocation location() const;
-		void reloadPicture();
+		/// Result of Element::reloadPicture()
+		enum class ReloadPictureResult {
+			Reloaded,          ///< drawing replaced by the current definition
+			Unavailable,       ///< definition missing or unreadable, old drawing kept
+			GeometryChanged    ///< size, hotspot or terminals changed, old drawing kept
+		};
+		ReloadPictureResult reloadPicture();
 		virtual void setHighlighted(bool);
 		void displayHelpLine(bool b = true);
 		QSize size() const;