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.

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

Rotate shortcut not working correctly. #821

Closed
dsm opened this issue Jul 7, 2024 · 7 comments · Fixed by #840
Closed

Rotate shortcut not working correctly. #821

dsm opened this issue Jul 7, 2024 · 7 comments · Fixed by #840
Labels
Milestone

Comments

@dsm
Copy link
Contributor

dsm commented Jul 7, 2024

Rotate shortcut ctrl+r or cmd+r doesn't work correctly. Clicking a component ex. Ground symbol and pressing ctrl+r multiple times, component moves around on schematics. And also simulation box not rotating but moving around also. Also rotating component with shortcut makes component off the grid. Some components ex. Resistor it doesn't happen.

rotation.webm
@ra3xdh
Copy link
Owner

ra3xdh commented Jul 8, 2024

Clicking a component ex. Ground symbol and pressing ctrl+r multiple times, component moves around on schematics.

I believe the problem with rotating of the 1-port devices was always present in Qucs-S, but remain undiscovered. It is surely not introduced by recent refactoring. I propose to keep the things as is.

And also simulation box not rotating but moving around also.

The rotation of the simulation boxes and equations should be forbidden. It's need to add a check in rotate handler.

Also rotating component with shortcut makes component off the grid.

Also there is an old problem. Should be fixed in v2.1.0

@ra3xdh ra3xdh added the gui label Jul 8, 2024
@ra3xdh
Copy link
Owner

ra3xdh commented Jul 8, 2024

Device falling out of out grid after rotation should be fixed by #399

@dsm
Copy link
Contributor Author

dsm commented Jul 8, 2024

there is a difference between select and press ctrl+r and press ctrl+r and press component.

rotation.webm

@gslg-1
Copy link
Contributor

gslg-1 commented Jul 13, 2024

I have code that could solve the issue. But since I am very new to this project, please check carefully.
Screencast from 13.07.2024 13:34:04.webm
Since I am not able to push (I guess I need to prove myself ) I will just add the code snippet here.

Update Schematic.ccp:

void Schematic::relativeRotation(int &newX, int &newY, int comX, int comY, int oldX, int oldY)
{
    // Shift coordinate system to center of mass
    // Rotate
    // Shift coordinate system back to origin
    newX = (oldY-comY)+comX;
    newY = -(oldX-comX)+comY;
}
bool Schematic::rotateElements()
{
    Wires->setAutoDelete(false);
    Components->setAutoDelete(false);

    // To rotate a selected area its necessary to work with half steps
    int x1 = INT_MAX, y1 = INT_MAX;
    int x2 = INT_MIN, y2 = INT_MIN;
    QList<Element *> ElementCache;
    copyLabels(x1, y1, x2, y2, &ElementCache); // must be first of all !
    copyComponents(x1, y1, x2, y2, &ElementCache);
    copyWires(x1, y1, x2, y2, &ElementCache);
    copyPaintings(x1, y1, x2, y2, &ElementCache);
    if (y1 == INT_MAX)
    {
        return false; // no element selected
    }
    int comX = (x1 + ((x2-x1) / 2)); // center of mass
    int comY = (y1 + ((y2-y1) / 2)); 
    int newPosX = INT_MIN; 
    int newPosY = INT_MIN;

    Wires->setAutoDelete(true);
    Components->setAutoDelete(true);
    setOnGrid(comX, comY);

    Wire *pw;
    Painting *pp;
    Component *pc;
    WireLabel *pl;
    // re-insert elements
    for (Element *pe : ElementCache)
        switch (pe->Type) {
        case isComponent:
        case isAnalogComponent:
        case isDigitalComponent:
            pc = (Component *) pe;
            pc->rotate(); //rotate component !before! rotating its center
            relativeRotation(newPosX, newPosY, comX, comY, pc->cx, pc->cy);
            pc->setCenter(newPosX, newPosY);
            insertRawComponent(pc);
            break;

        case isWire:
            pw = (Wire *) pe;
            relativeRotation(newPosX, newPosY, comX, comY, pw->x1, pw->y1);
            pw->x1 = newPosX;
            pw->y1 = newPosY;
            relativeRotation(newPosX, newPosY, comX, comY, pw->x2, pw->y2);
            pw->x2 = newPosX;
            pw->y2 = newPosY;

            pl = pw->Label;
            if (pl) {
                x2 = pl->cx;
                relativeRotation(newPosX, newPosY, comX, comY, pl->cx, pl->cy);
                pl->cx = newPosX;
                pl->cy = newPosY;
                if (pl->Type == isHWireLabel)
                    pl->Type = isVWireLabel;
                else
                    pl->Type = isHWireLabel;
            }
            insertWire(pw);
            break;

        case isHWireLabel:
        case isVWireLabel:
            pl = (WireLabel *) pe;
            relativeRotation(newPosX, newPosY, comX, comY, pl->x1, pl->y1);
            pl->x1 = newPosX;
            pl->y1 = newPosY;
            break;
        case isNodeLabel:
            pl = (WireLabel *) pe;
            if (pl->pOwner == 0) {
                relativeRotation(newPosX, newPosY, comX, comY, pl->x1, pl->y1);
                pl->x1 = newPosX;
                pl->y1 = newPosY;
            }
            relativeRotation(newPosX, newPosY, comX, comY, pl->cx, pl->cy);
            pl->cx = newPosX;
            pl->cy = newPosY;
            insertNodeLabel(pl);
            break;

        case isPainting:
            pp = (Painting *) pe;
            pp->rotate(x1, y1); // rotate around the center x1 y1
            Paintings->append(pp);
            break;
        default:;
        }

    ElementCache.clear();

    setChanged(true, true);
    return true;
}

My git diff print out:

diff --git a/qucs/schematic.cpp b/qucs/schematic.cpp
index 04eeae6d..7e975c4f 100644
--- a/qucs/schematic.cpp
+++ b/qucs/schematic.cpp
@@ -1144,14 +1144,6 @@ void Schematic::drawGrid(QPainter* painter) {
     painter->restore();
 }
 
-void Schematic::relativeRotation(int &newX, int &newY, int comX, int comY, int oldX, int oldY)
-{
-    // Shift coordinate system to center of mass
-    // Rotate
-    // Shift coordinate system back to origin
-    newX = (oldY-comY)+comX;
-    newY = -(oldX-comX)+comY;
-}
 // ---------------------------------------------------
 // Correction factor for unproportional font scaling.
 float Schematic::textCorr()
@@ -1386,7 +1378,6 @@ bool Schematic::rotateElements()
     Wires->setAutoDelete(false);
     Components->setAutoDelete(false);
 
-    // To rotate a selected area its necessary to work with half steps
     int x1 = INT_MAX, y1 = INT_MAX;
     int x2 = INT_MIN, y2 = INT_MIN;
     QList<Element *> ElementCache;
@@ -1395,17 +1386,14 @@ bool Schematic::rotateElements()
     copyWires(x1, y1, x2, y2, &ElementCache);
     copyPaintings(x1, y1, x2, y2, &ElementCache);
     if (y1 == INT_MAX)
-    {
         return false; // no element selected
-    }
-    int comX = (x1 + ((x2-x1) / 2)); // center of mass
-    int comY = (y1 + ((y2-y1) / 2)); 
-    int newPosX = INT_MIN; 
-    int newPosY = INT_MIN;
 
     Wires->setAutoDelete(true);
     Components->setAutoDelete(true);
-    setOnGrid(comX, comY);
+
+    x1 = (x1 + x2) >> 1; // center for rotation
+    y1 = (y1 + y2) >> 1;
+    //setOnGrid(x1, y1);
 
     Wire *pw;
     Painting *pp;
@@ -1419,26 +1407,23 @@ bool Schematic::rotateElements()
         case isDigitalComponent:
             pc = (Component *) pe;
             pc->rotate(); //rotate component !before! rotating its center
-            relativeRotation(newPosX, newPosY, comX, comY, pc->cx, pc->cy);
-            pc->setCenter(newPosX, newPosY);
+            pc->setCenter(pc->cy - y1 + x1, x1 - pc->cx + y1);
             insertRawComponent(pc);
             break;
 
         case isWire:
             pw = (Wire *) pe;
-            relativeRotation(newPosX, newPosY, comX, comY, pw->x1, pw->y1);
-            pw->x1 = newPosX;
-            pw->y1 = newPosY;
-            relativeRotation(newPosX, newPosY, comX, comY, pw->x2, pw->y2);
-            relativeRotation(newPosX, newPosY, comX, comY, pw->x2, pw->y2);
-            pw->x2 = newPosX;
-            pw->y2 = newPosY;
-
+            x2 = pw->x1;
+            pw->x1 = pw->y1 - y1 + x1;
+            pw->y1 = x1 - x2 + y1;
+            x2 = pw->x2;
+            pw->x2 = pw->y2 - y1 + x1;
+            pw->y2 = x1 - x2 + y1;
             pl = pw->Label;
             if (pl) {
                 x2 = pl->cx;
-                relativeRotation(newPosX, newPosY, comX, comY, pl->cx, pl->cy);
-                pl->cx = newPosX;
-                pl->cy = newPosY;
+                pl->cx = pl->cy - y1 + x1;
+                pl->cy = x1 - x2 + y1;
                 if (pl->Type == isHWireLabel)
                     pl->Type = isVWireLabel;
                 else
@@ -1450,20 +1435,20 @@ bool Schematic::rotateElements()
         case isHWireLabel:
         case isVWireLabel:
             pl = (WireLabel *) pe;
-            relativeRotation(newPosX, newPosY, comX, comY, pl->x1, pl->y1);
-            pl->x1 = newPosX;
-            pl->y1 = newPosY;
+            x2 = pl->x1;
+            pl->x1 = pl->y1 - y1 + x1;
+            pl->y1 = x1 - x2 + y1;
             break;
         case isNodeLabel:
             pl = (WireLabel *) pe;
             if (pl->pOwner == 0) {
-                relativeRotation(newPosX, newPosY, comX, comY, pl->x1, pl->y1);
-                pl->x1 = newPosX;
-                pl->y1 = newPosY;
+                x2 = pl->x1;
+                pl->x1 = pl->y1 - y1 + x1;
+                pl->y1 = x1 - x2 + y1;
             }
-            relativeRotation(newPosX, newPosY, comX, comY, pl->cx, pl->cy);
-            pl->cx = newPosX;
-            pl->cy = newPosY;
+            x2 = pl->cx;
+            pl->cx = pl->cy - y1 + x1;
+            pl->cy = x1 - x2 + y1;
             insertNodeLabel(pl);
             break;
 
diff --git a/qucs/schematic.h b/qucs/schematic.h
index 491deec4..5f0e75f0 100644
--- a/qucs/schematic.h
+++ b/qucs/schematic.h
@@ -370,7 +370,6 @@ private:
   void drawPostPaintEvents(QPainter* painter);
   void paintFrame(QPainter* painter);
   void drawGrid(QPainter* painter);
-  void relativeRotation(int &x, int &y, int comX, int comY, int posX, int posY);
 
 /* ********************************************************************
    *****  The following methods are in the file                   *****
(END)

@ra3xdh
Copy link
Owner

ra3xdh commented Jul 13, 2024

@gslg-1 Thanks for providing a fix! I will test this in the next few days.

Since I am not able to push (I guess I need to prove myself )

The best way for contribution is to make a pool request (PR). You don't need to be repository member to make pull request. This feature is available for everyone having Github account. You may look at this guide how to make a pull request https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request

@gslg-1
Copy link
Contributor

gslg-1 commented Jul 13, 2024

I've provided a fork, with the changes and will create a pull request

gslg-1 added a commit to gslg-1/qucs_s_fork that referenced this issue Jul 13, 2024
gslg-1 added a commit to gslg-1/qucs_s_fork that referenced this issue Jul 13, 2024
@ra3xdh ra3xdh linked a pull request Jul 13, 2024 that will close this issue
@ra3xdh ra3xdh added this to the 24.3.0 milestone Jul 13, 2024
@ra3xdh
Copy link
Owner

ra3xdh commented Jul 13, 2024

Fixed by #840 Closing as completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants