Merge pull request #1273 from ispyisail/feature/wires-per-terminal-chains

Wire terminals in a chain, not a star, under a wire limit

sources/undocommand/deleteqgraphicsitemcommand.cpp

@@ -16,8 +16,10 @@
 	along with QElectroTech.  If not, see <http://www.gnu.org/licenses/>.
 */
 #include "deleteqgraphicsitemcommand.h"
+#include "../wiringrules.h"
 
 #include "../diagram.h"
+#include "../qetproject.h"
 #include "addgraphicsobjectcommand.h"
 #include "../qetdiagrameditor.h"
 #include "../qetgraphicsitem/ViewItem/qetgraphicstableitem.h"
@@ -207,17 +209,44 @@ void DeleteQGraphicsItemCommand::setPotentialsOfRemovedElements()
 			}
 
 			ConductorProperties properties = hub_terminal->conductors().first()->properties();
-			for (Terminal *t : terminals_to_connect_list)
+
+				//Every terminal to the hub (a star), or, when the project
+				//limits the wires per terminal (discussion #1158), one after
+				//another (a chain): a star gives the hub a wire per other
+				//terminal, which the limit forbids.
+			QList<std::pair<Terminal *, Terminal *>> pairs;
+			const QETProject *project = hub_terminal->diagram() ? hub_terminal->diagram()->project() : nullptr;
+			if (project && WiringRules::chainsWires(project->wiringRules(), WiringRules::masterEnabled()))
+			{
+				QList<Terminal *> chain {hub_terminal};
+				chain << terminals_to_connect_list;
+				QList<QPointF> points;
+				for (Terminal *ct : std::as_const(chain)) {
+					points << ct->scenePos();
+				}
+				const QList<int> order = WiringRules::chainOrder(points);
+				for (int i = 1 ; i < order.size() ; ++i) {
+					pairs.append(std::make_pair(chain.at(order.at(i - 1)), chain.at(order.at(i))));
+				}
+			}
+			else
+			{
+				for (Terminal *t : std::as_const(terminals_to_connect_list)) {
+					pairs.append(std::make_pair(hub_terminal, t));
+				}
+			}
+
+			for (const auto &new_pair : std::as_const(pairs))
 			{
 					//If a conductor was already created between these two terminals
 					//in this undo command, from another removed element, we do nothing
 				bool exist_ = false;
 				for (std::pair<Terminal *, Terminal *> pair : m_connected_terminals)
 				{
-					if  (pair.first == hub_terminal && pair.second == t) {
+					if  (pair.first == new_pair.first && pair.second == new_pair.second) {
 						exist_ = true;
 						continue;
-					} else if (pair.first == t && pair.second == hub_terminal) {
+					} else if (pair.first == new_pair.second && pair.second == new_pair.first) {
 						exist_ = true;
 						continue;
 					}
@@ -225,11 +254,11 @@ void DeleteQGraphicsItemCommand::setPotentialsOfRemovedElements()
 
 				if (exist_ == false)
 				{
-					m_connected_terminals.append(std::make_pair<Terminal *, Terminal *>((Terminal *)hub_terminal, (Terminal *)t));
+					m_connected_terminals.append(new_pair);
 					qInfo() << "m_connected_terminals" << m_connected_terminals;
-					Conductor *new_cond = new Conductor(hub_terminal, t);
+					Conductor *new_cond = new Conductor(new_pair.first, new_pair.second);
 					new_cond->setProperties(properties);
-					new AddGraphicsObjectCommand(new_cond, t->diagram(), QPointF(), this);
+					new AddGraphicsObjectCommand(new_cond, new_pair.second->diagram(), QPointF(), this);
 				}
 			}
 		}

sources/utils/conductorcreator.cpp

@@ -26,6 +26,7 @@
 #include "../qetgraphicsitem/element.h"
 #include "../qetgraphicsitem/terminal.h"
 #include "../ui/potentialselectordialog.h"
+#include "../wiringrules.h"
 #include "qgraphicsitem.h"
 
 #include <QPolygonF>
@@ -48,18 +49,19 @@ ConductorCreator::ConductorCreator(Diagram *d, QList<Terminal *> terminals_list)
 	if (!setUpPropertieToUse()) {
 		return;
 	}
-	Terminal *hub_terminal = hubTerminal();
-	
 	d->undoStack().beginMacro(QObject::tr("Création de conducteurs"));
 	
+	const bool chain = d->project()
+			&& WiringRules::chainsWires(d->project()->wiringRules(), WiringRules::masterEnabled());
 	QList<Conductor *> c_list;
-	for (Terminal *t : m_terminals_list)
+	for (const auto &pair : terminalPairs(chain))
 	{
-		if (t == hub_terminal) {
+			//Checked as the chain is built: the wire before this one may
+			//have just filled a terminal.
+		if (chain && !pair.first->canBeLinkedTo(pair.second)) {
 			continue;
 		}
-		
-		Conductor *cond = new Conductor(hub_terminal, t);
+		Conductor *cond = new Conductor(pair.first, pair.second);
 		cond->setProperties(m_properties);
 		cond->setSequenceNum(m_sequential_number);
 		d->undoStack().push(new AddGraphicsObjectCommand(cond, d));
@@ -224,6 +226,41 @@ QList<Conductor *> ConductorCreator::existingPotential(const QList<Terminal *> &
 	@brief ConductorCreator::hubTerminal
 	@return hub_terminal
 */
+/**
+	@brief ConductorCreator::terminalPairs
+	@param chain : the project limits the wires per terminal
+	(discussion #1158, WiringRules::chainsWires())
+	@return the pairs of terminals to wire: all to one hub terminal (a
+	star), or with \p chain one after another (WiringRules::chainOrder()),
+	since a star gives the hub a wire per other terminal.
+*/
+QList<QPair<Terminal *, Terminal *>> ConductorCreator::terminalPairs(bool chain)
+{
+	QList<QPair<Terminal *, Terminal *>> pairs;
+	if (chain)
+	{
+		QList<QPointF> points;
+		for (Terminal *t : std::as_const(m_terminals_list)) {
+			points << t->scenePos();
+		}
+		const QList<int> order = WiringRules::chainOrder(points);
+		for (int i = 1 ; i < order.size() ; ++i)
+		{
+			pairs << qMakePair(m_terminals_list.at(order.at(i - 1)),
+							   m_terminals_list.at(order.at(i)));
+		}
+		return pairs;
+	}
+
+	Terminal *hub_terminal = hubTerminal();
+	for (Terminal *t : std::as_const(m_terminals_list)) {
+		if (t != hub_terminal) {
+			pairs << qMakePair(hub_terminal, t);
+		}
+	}
+	return pairs;
+}
+
 Terminal *ConductorCreator::hubTerminal()
 {
 	Terminal *hub_terminal = m_terminals_list.first();

sources/utils/conductorcreator.h

@@ -44,6 +44,7 @@ class ConductorCreator
 		static QList<Conductor *> existingPotential(const QList<Terminal *> &terminals_list);
 		bool setUpPropertieToUse();
 		Terminal *hubTerminal();
+		QList<QPair<Terminal *, Terminal *>> terminalPairs(bool chain);
 		
 		
 		QList<Terminal *> m_terminals_list;

sources/wiringrules.cpp

@@ -174,6 +174,68 @@ bool WiringRules::hasRoom(int limit, int wires)
 	return limit <= 0 || wires < limit;
 }
 
+/**
+	@brief WiringRules::chainsWires
+	@return true if QElectroTech's own tools that wire several terminals at
+	once must wire them one after another (a chain) rather than all to one
+	of them (a star): a star gives that one terminal a wire per other
+	terminal, which the project's limit forbids.
+*/
+bool WiringRules::chainsWires(const Settings &settings, bool master_enabled)
+{
+	return master_enabled && settings.max_wires > 0;
+}
+
+/**
+	@brief WiringRules::chainOrder
+	The order in which to wire terminals at \a points one after another:
+	from the top left one (smallest x, then smallest y, the same terminal
+	the star used as its hub), each time to the nearest terminal not yet
+	wired. Distance is along the grid (|dx| + |dy|), since wires run
+	horizontally and vertically; a tie goes to the earlier point.
+	@return indexes into \a points, each once
+*/
+QList<int> WiringRules::chainOrder(const QList<QPointF> &points)
+{
+	QList<int> order;
+	if (points.isEmpty()) {
+		return order;
+	}
+
+	int current = 0;
+	for (int i = 1 ; i < points.size() ; ++i) {
+		const QPointF &p = points.at(i);
+		const QPointF &c = points.at(current);
+		if (p.x() < c.x() || (p.x() == c.x() && p.y() < c.y())) {
+			current = i;
+		}
+	}
+
+	QList<bool> done(points.size(), false);
+	order << current;
+	done[current] = true;
+	while (order.size() < points.size())
+	{
+		int nearest = -1;
+		qreal nearest_distance = 0;
+		for (int i = 0 ; i < points.size() ; ++i) {
+			if (done.at(i)) {
+				continue;
+			}
+			const QPointF d = points.at(i) - points.at(current);
+			const qreal distance = qAbs(d.x()) + qAbs(d.y());
+			if (nearest < 0 || distance < nearest_distance) {
+				nearest = i;
+				nearest_distance = distance;
+			}
+		}
+		order << nearest;
+		done[nearest] = true;
+		current = nearest;
+	}
+	return order;
+}
+
 /**
 	@brief WiringRules::turnsRuleOn
 	@param before, after : the rules in force before and after a change

sources/wiringrules.h

@@ -19,6 +19,9 @@
 #ifndef WIRINGRULES_H
 #define WIRINGRULES_H
 
+#include <QList>
+#include <QPointF>
+
 class QDomElement;
 
 /**
@@ -79,6 +82,9 @@ namespace WiringRules
 
 	int limit(const Settings &settings, bool master_enabled, bool is_report);
 	bool hasRoom(int limit, int wires);
+
+	bool chainsWires(const Settings &settings, bool master_enabled);
+	QList<int> chainOrder(const QList<QPointF> &points);
 }
 
 #endif // WIRINGRULES_H

tests/qttest/tst_wiringrules.cpp

@@ -137,6 +137,49 @@ class tst_wiringrules : public QObject
 		return {};
 	}
 
+		// On a fixture with @p rules: a symbol wired to four others is
+		// deleted, so QElectroTech rewires the four to keep the potential.
+		// Returns the most wires any one of them ends with.
+	QString mostWiresAfterDeletingTheHub(const QString &rules)
+	{
+		const QString project = fixtureWith(rules);
+		const QString script_path = m_dir.filePath(QStringLiteral("hub%1.js").arg(m_run));
+		QFile script(script_path);
+		if (project.isEmpty() || !script.open(QIODevice::WriteOnly))
+			return {};
+			// Placed on a diagonal, so no two terminals line up and nothing
+			// is auto-connected.
+		script.write(
+			"var p = 'embed://import/probe/v2_fuse.elmt';\n"
+			"var hub = qet.addElement(0, p, 400, 400);\n"
+			"var others = [];\n"
+			"for (var i = 1; i <= 4; ++i) others.push(qet.addElement(0, p, 400 + 70 * i, 400 + 90 * i));\n"
+			"others.forEach(function (o) { qet.addConductor(0, hub, 0, o, 0); });\n"
+			"qet.deleteElement(0, hub);\n"
+			"var count = {};\n"
+			"qet.conductorUuids(0).forEach(function (u) {\n"
+			"  qet.conductorEnds(0, u).forEach(function (e) { count[e] = (count[e] || 0) + 1; }); });\n"
+			"var most = 0;\n"
+			"others.forEach(function (o) { most = Math.max(most, count[o + ' terminal 0'] || 0); });\n"
+			"qet.log('PROBE ' + most);\n");
+		script.close();
+
+		QProcess proc;
+		proc.setProcessEnvironment(sandbox());
+		proc.start(QStringLiteral(QET_TEST_BINARY_PATH), {QStringLiteral("--run"), script_path, project});
+		if (!proc.waitForFinished(60000))
+			return {};
+		const QString out = QString::fromUtf8(proc.readAllStandardOutput()
+											  + proc.readAllStandardError());
+		const QString mark = QStringLiteral("PROBE ");
+		for (const QString &line : out.split(QLatin1Char('\n'))) {
+			const int i = line.indexOf(mark);
+			if (i >= 0)
+				return line.mid(i + mark.size()).trimmed();
+		}
+		return {};
+	}
+
 private slots:
 	void initTestCase()
 	{
@@ -173,6 +216,39 @@ private slots:
 		QVERIFY(!WiringRules::hasRoom(1, 1));
 	}
 
+	void chainsOnlyUnderALimit()
+	{
+		WiringRules::Settings rules;
+		QVERIFY(!WiringRules::chainsWires(rules, true));
+		rules.one_wire_per_report = true;
+		QVERIFY(!WiringRules::chainsWires(rules, true));
+		rules.max_wires = 2;
+		QVERIFY(WiringRules::chainsWires(rules, true));
+		QVERIFY(!WiringRules::chainsWires(rules, false));
+	}
+
+	void chainOrder()
+	{
+		QCOMPARE(WiringRules::chainOrder({}), QList<int>());
+		QCOMPARE(WiringRules::chainOrder({QPointF(5, 5)}), QList<int>({0}));
+
+			// Starts top left, then always the nearest along the grid
+		const QList<QPointF> row {QPointF(300, 0), QPointF(0, 0), QPointF(200, 0), QPointF(100, 0)};
+		QCOMPARE(WiringRules::chainOrder(row), QList<int>({1, 3, 2, 0}));
+
+			// Same x: the higher one starts
+		const QList<QPointF> column {QPointF(0, 100), QPointF(0, 0), QPointF(0, 50)};
+		QCOMPARE(WiringRules::chainOrder(column), QList<int>({1, 2, 0}));
+
+			// Each index exactly once
+		const QList<QPointF> scattered {QPointF(40, 90), QPointF(10, 300), QPointF(220, 10),
+										QPointF(10, 10), QPointF(220, 300)};
+		QList<int> order = WiringRules::chainOrder(scattered);
+		QCOMPARE(order.first(), 3);
+		std::sort(order.begin(), order.end());
+		QCOMPARE(order, QList<int>({0, 1, 2, 3, 4}));
+	}
+
 	void projectOverridesApplication()
 	{
 		WiringRules::Settings application;
@@ -293,6 +369,18 @@ private slots:
 			// ...and the master switch still turns it off
 		QCOMPARE(addWireToWiredTerminal(plain, true, 1), QStringLiteral("true"));
 	}
+
+	void deletingASymbolChainsTheWires()
+	{
+		SKIP_WITHOUT_SCRIPTING;
+			// No rule: the four are wired to one of them, as on master
+		QCOMPARE(mostWiresAfterDeletingTheHub(QString()), QStringLiteral("3"));
+
+			// A limit: one after another, none gets more than two
+		QCOMPARE(mostWiresAfterDeletingTheHub(
+					 QStringLiteral("<wiring_rules max_wires_per_terminal=\"4\"/>")),
+				 QStringLiteral("2"));
+	}
 };
 
 QTEST_GUILESS_MAIN(tst_wiringrules)