Merge branch 'bugfix-610-unable-to-pull-wire' into copilot/sub-pr-621

Author: highperformancecoder

model/group.cc

@@ -1045,26 +1045,21 @@ namespace minsky
 
   }
 
-  void Group::draw1edge(const vector<VariablePtr>& vars, cairo_t* cairo, 
-                        float x) const
+  // Helper to compute edge variable layout (world-space positions) radially
+  static void layoutEdgeVariables(const vector<VariablePtr>& vars, float xEdge,
+                                  const Item* groupItem, double groupRotation)
   {
     float top=0, bottom=0;
-    const double angle=rotation() * M_PI / 180.0;
     for (size_t i=0; i<vars.size(); ++i)
       {
-        const Rotate r(rotation(),0,0);
+        const Rotate r(groupRotation,0,0);
         auto& v=vars[i];
-        float y=0;
+        float yOff=0;
         auto z=v->zoomFactor();
         auto t=v->bb.top()*z, b=v->bb.bottom()*z;
-        if (i>0) y = i%2? top-b: bottom-t;
-        v->moveTo(r.x(x,y)+this->x(), r.y(x,y)+this->y());
-        const cairo::CairoSave cs(cairo);
-        cairo_translate(cairo,x,y);
-        // cairo context is already rotated, so antirotate
-        cairo_rotate(cairo,-angle);
-        v->rotation(rotation()); 
-        v->draw(cairo);
+        if (i>0) yOff = i%2? top-b: bottom-t;
+        v->moveTo(r.x(xEdge,yOff)+groupItem->x(), r.y(xEdge,yOff)+groupItem->y());
+        v->rotation(groupRotation);
         if (i==0)
           {
             bottom=b;
@@ -1077,6 +1072,44 @@ namespace minsky
       }
   }
 
+  void Group::draw1edge(const vector<VariablePtr>& vars, cairo_t* cairo, 
+                        float xEdge) const
+  {
+    layoutEdgeVariables(vars, xEdge, this, rotation());
+
+    // Iterate again solely to perform drawing steps.
+    // Group::draw has ALREADY called cairo_scale(cairo, z, z), so the `xEdge`
+    // parameter is properly matched to the visual coordinates.
+    // However, translating `yOff` locally during rendering requires tracking
+    // the variable bounding boxes WITHOUT pre-multiplying `z` onto them here.
+    const double angle=rotation() * M_PI / 180.0;
+    float top=0, bottom=0;
+    for (size_t i=0; i<vars.size(); ++i)
+      {
+        auto& v=vars[i];
+        
+        float yOff = 0;
+        auto t = v->bb.top(), b = v->bb.bottom();
+        if (i > 0) yOff = i % 2 ? top - b : bottom - t;
+        
+        const cairo::CairoSave cs(cairo);
+        cairo_translate(cairo,xEdge,yOff);
+        // cairo context is already rotated, so antirotate
+        cairo_rotate(cairo,-angle);
+        v->draw(cairo);
+
+        if (i == 0)
+          {
+            bottom = b;
+            top = t;
+          }
+        else if (i % 2)
+          top -= v->height() / v->zoomFactor(); // Remove zoom as Group::draw supplies it
+        else
+          bottom += v->height() / v->zoomFactor();
+      }
+  }
+
   void Group::drawEdgeVariables(cairo_t* cairo) const
   {
     float left, right; margins(left,right);    
@@ -1196,28 +1229,8 @@ namespace minsky
     margins(leftMargin, rightMargin);
     const float z = zoomFactor();
 
-    auto position1edge = [&](const std::vector<VariablePtr>& vars, float xEdge)
-    {
-      float top = 0, bottom = 0;
-      for (size_t i = 0; i < vars.size(); ++i)
-        {
-          const Rotate r(rotation(), 0, 0);
-          auto& v = vars[i];
-          float yOff = 0;
-          auto vz = v->zoomFactor();
-          auto t = v->bb.top() * vz, b = v->bb.bottom() * vz;
-          if (i > 0) yOff = (i % 2) ? top - b : bottom - t;
-          v->moveTo(r.x(xEdge, yOff) + this->x(),
-                    r.y(xEdge, yOff) + this->y());
-          v->rotation(rotation());
-          if (i == 0) { bottom = b; top = t; }
-          else if (i % 2) top -= v->height();
-          else            bottom += v->height();
-        }
-    };
-
-    position1edge(inVariables,  -0.5f * (iWidth() * z - leftMargin));
-    position1edge(outVariables,  0.5f * (iWidth() * z - rightMargin));
+    layoutEdgeVariables(inVariables, -0.5f * (iWidth() * z - leftMargin), this, rotation());
+    layoutEdgeVariables(outVariables, 0.5f * (iWidth() * z - rightMargin), this, rotation());
   }
 
   ItemPtr Group::select(float x, float y) const

test/testCanvas.cc

@@ -124,7 +124,47 @@ TEST(GroupIOTest, groupIOVarSelect)
     EXPECT_NE(nullptr, found)
         << "select() should find the output variable without a prior draw() call (issue #610)";
     if (found)
+      {
         EXPECT_EQ(grp->outVariables[0].get(), found.get())
             << "select() should return the output variable, not some other item";
+      }
+}
+
+/// Regression test to verify the zoomed wire-pull path works.
+TEST(GroupIOTest, groupIOVarSelectZoomed)
+{
+    // Build a standalone group with an output variable.
+    auto grp = make_shared<Group>();
+    grp->self = grp;
+    grp->moveTo(200, 200);
+    grp->iWidth(100);
+    grp->iHeight(100);
+
+    // Set a non-default zoom
+    grp->setZoom(2.5f);
+
+    grp->addOutputVar();
+    ASSERT_EQ(1u, grp->outVariables.size());
+
+    // Update bounding box for the output variable so margins() works.
+    auto& outVar = *grp->outVariables[0];
+    outVar.bb.update(outVar);
+
+    float leftMargin, rightMargin;
+    grp->margins(leftMargin, rightMargin);
+    const float z     = grp->zoomFactor();
+    const float xEdge = 0.5f * (grp->iWidth() * z - rightMargin);
+    const float expectedX = grp->x() + xEdge;
+    const float expectedY = grp->y();
+
+    auto found = grp->select(expectedX, expectedY);
+    EXPECT_NE(nullptr, found)
+        << "select() should find the output variable at a non-default zoom";
+
+    // Since `draw()` hasn't positioned the ports dynamically, we check the exact 
+    // underlying static port coordinates of the found variable after positions are set.
+    auto outPort = outVar.ports(0).lock();
+    EXPECT_EQ(ClickType::onPort, grp->clickType(outPort->x(), outPort->y()))
+        << "clickType() on group should yield onPort at a non-default zoom for wire-pulling";
 }