Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes dragging bendpoints of 4 or more wires (fixes #3006). #3942

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Fixes dragging bendpoints of 4 or more wires (fixes #3006). #3942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3e895db
Fixes movement of the wires when dragging bendpoints of 4 or more wir…
failiz Mar 24, 2022
580a8c4
Improvements to the connectors status color (red=unconnected, green=c…
failiz Mar 27, 2022
955c1d3
fixed problem in PCB view. There are two connectors on top of each ot…
failiz May 19, 2023
9f99984
fixes issue with previous commit. We only add one connector from the …
failiz May 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions src/connectors/connectoritem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,13 +523,17 @@ void ConnectorItem::restoreColor(QList<ConnectorItem *> & visited)

QList<ConnectorItem *> connectorItems;
connectorItems.append(this);
collectEqualPotential(connectorItems, true, getSkipFlags());
collectEqualPotential(connectorItems, true, getSkipFlags(), false);
visited.append(connectorItems);
QSet<ItemBase *> attachedTo;

QSet<ConnectorItem *> attachedTo;
foreach (ConnectorItem * connectorItem, connectorItems) {
if (connectorItem->isEverVisible()) {
if (connectorItem->attachedToItemType() != ModelPart::Wire) {
attachedTo.insert(connectorItem->attachedTo()->layerKinChief());
//For PCB pad stacks: Only add one connector to the attachedTo list (top or bottom copper)
if(!attachedTo.contains(connectorItem->getCrossLayerConnectorItem())) {
attachedTo.insert(connectorItem);
}
}
}
}
Expand All @@ -554,7 +558,24 @@ void ConnectorItem::restoreColor(QList<ConnectorItem *> & visited)
}
}
}

if (this->attachedToViewID() == 3) {
DebugDialog::debug(QString("FAI restore color PCB equalPotential: %1 attachedTo: %2")
.arg(connectorItems.count())
.arg(attachedTo.count())
);
for(ConnectorItem * c : connectorItems)
DebugDialog::debug(QString("equalPotential conn, title: %1 %2 %3")
.arg(c->attachedToTitle())
.arg(c->attachedTo()->label())
.arg(c->attachedTo()->viewLayerID())
);
for(ConnectorItem * c : attachedTo)
DebugDialog::debug(QString("attachedTo conn, title: %1 %2 %3")
.arg(c->attachedToTitle())
.arg(c->attachedTo()->label())
.arg(c->attachedTo()->viewLayerID())
);
}

/*
DebugDialog::debug(QString("restore color dobus:%1 bccount:%2 docross:%3 cid:'%4' '%5' id:%6 '%7' vid:%8 vlid:%9 %10")
Expand Down Expand Up @@ -1315,7 +1336,7 @@ bool ConnectorItem::isConnectedToPart() {
* @param[in] skipFlags filter for the types of wires that are not to be included
*/
void ConnectorItem::collectEqualPotential(QList<ConnectorItem *> &connectorItems,
bool crossLayers, ViewGeometry::WireFlags skipFlags)
bool crossLayers, ViewGeometry::WireFlags skipFlags, bool followBuses)
{
// take a local (temporary working) copy of the supplied list, and wipe the original
QList<ConnectorItem *> tempItems = connectorItems;
Expand Down Expand Up @@ -1365,7 +1386,7 @@ void ConnectorItem::collectEqualPotential(QList<ConnectorItem *> &connectorItems
// When the kept connector item is part of a bus, include all of the other
// connectors on the bus in the list being processed
Bus *bus = connectorItem->bus();
if (bus) {
if (bus && (followBuses||fromWire)) {
QList<ConnectorItem *> busConnectedItems;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the "skipBuses" param was broken, or maybe never implemented.
For the IPC-D 356 export we also needed this and I modified this to

@@ -1381,7 +1384,7 @@ void ConnectorItem::collectEqualPotential(QList<ConnectorItem *> &connectorItems
                // When the kept connector item is part of a bus, include all of the other
                // connectors on the bus in the list being processed
                Bus *bus = connectorItem->bus();
-               if (bus) {
+               if (!skipBuses && bus) {
                        QList<ConnectorItem *> busConnectedItems;

The renaming is not an issue, but I am a bit paranoid about the "fromWire". The collectEqualPotential method is quite critical. Do you think by adding the skipBuses like in our current version, we are excluding to many items?

Is the patch about more than just highlighting the connections? Or do we need it to send proper netlists to ngspice?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think skipBuses was never implemented and you implemented it recently. At least, I cannot find it in the source code. I guess skipBuses and followBuses are basically the same.

The fromWire is because I want to avoid following buses of the parts (e.g., breadboard or Arduino pins), but I still want to follow the wires. And the wires are implemented as buses. Removing the fromWire from the code breaks the connector highlighting.

The patch is to fix issue #3827 and is just about improving the highlighting of the connector status (not related to the SPICE netlist). That issue is caused because the current algorithm checks if a part is connected to another part (ItemBase). In my patch, I changed it to check if a connector is connected to other connectors (from the same part or from a different one). No following the buses makes that ony the hole of the breadboard that is connected is highlighted.

In my code, the only point where buses are not followed is in the restoreColor function. The code for generating the spice netlists uses the default argument of followBuses (as it is necessary to follow the buses). Not sure how the exporting of the IPC-D 356 netlists works.

connectorItem->attachedTo()->busConnectorItems(bus, connectorItem, busConnectedItems);
#ifndef QT_NO_DEBUG
Expand Down
2 changes: 1 addition & 1 deletion src/connectors/connectoritem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ class ConnectorItem : public NonConnectorItem, public CursorKeyListener
static void collectPart(ConnectorItem * connectorItem, QList<ConnectorItem *> & partsConnectors, ViewLayer::ViewLayerPlacement);

public:
static void collectEqualPotential(QList<ConnectorItem *> & connectorItems, bool crossLayers, ViewGeometry::WireFlags skipFlags);
static void collectEqualPotential(QList<ConnectorItem *> & connectorItems, bool crossLayers, ViewGeometry::WireFlags skipFlags, bool followBuses = true);
static void collectParts(QList<ConnectorItem *> & connectorItems, QList<ConnectorItem *> & partsConnectors, bool includeSymbols, ViewLayer::ViewLayerPlacement);
static void clearEqualPotentialDisplay();
static bool isGrounded(ConnectorItem * c1, ConnectorItem * c2);
Expand Down
28 changes: 20 additions & 8 deletions src/items/wire.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,22 +600,31 @@ void Wire::mouseMoveEventAux(QPointF eventPos, Qt::KeyboardModifiers modifiers)
}
setConnector1Rect();


QSet<ConnectorItem *> allTo;
allTo.insert(whichConnectorItem);
QList<ConnectorItem *> allToList;
foreach (ConnectorItem * toConnectorItem, whichConnectorItem->connectedToItems()) {
Wire * chainedWire = qobject_cast<Wire *>(toConnectorItem->attachedTo());
if (chainedWire == NULL) continue;

allTo.insert(toConnectorItem);
foreach (ConnectorItem * subTo, toConnectorItem->connectedToItems()) {
allTo.insert(subTo);
if(!allToList.contains(toConnectorItem)) {
allToList.append(toConnectorItem);
}
}
allTo.remove(whichConnectorItem);

// TODO: this could all be determined once at mouse press time
for (int i = 0; i < allToList.size(); i++) {
ConnectorItem * conItem = allToList.at(i);
foreach(ConnectorItem * toConnectorItem, conItem->connectedToItems()) {
Wire * chainedWire = qobject_cast<Wire *>(toConnectorItem->attachedTo());
if (chainedWire == NULL) continue;

if(!allToList.contains(toConnectorItem)) {
allToList.append(toConnectorItem);
}
}
}

QSet<ConnectorItem *> allTo(allToList.begin(), allToList.end());

// TODO: this could all be determined once at mouse press time
if (allTo.count() == 0) {
// dragging one end of the wire

Expand Down Expand Up @@ -684,9 +693,12 @@ void Wire::mouseMoveEventAux(QPointF eventPos, Qt::KeyboardModifiers modifiers)
}
else {
// dragging a bendpoint
DebugDialog::debug("dragging a bendpoint");
DebugDialog::debug(QString("CON0 %1 CON1 %2").arg(this->connector0()->connectedToItems().count()).arg(this->connector0()->connectedToItems().count()));
foreach (ConnectorItem * toConnectorItem, allTo) {
Wire * chained = qobject_cast<Wire *>(toConnectorItem->attachedTo());
if (chained) {
DebugDialog::debug("dragging a bendpoint123");
chained->simpleConnectedMoved(whichConnectorItem, toConnectorItem);
}
}
Expand Down