Bug 110231 - Association labels aren't moved correctly when moving the corresponding classes (class diagram)
Summary: Association labels aren't moved correctly when moving the corresponding class...
Status: RESOLVED FIXED
Alias: None
Product: umbrello
Classification: Applications
Component: general (show other bugs)
Version: 1.4.2
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Umbrello Development Group
URL:
Keywords:
: 107171 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-08-05 13:53 UTC by Tassilo Horn
Modified: 2005-09-06 23:32 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
The diagram where this bug is reproducible (117.59 KB, text/plain)
2005-08-05 13:55 UTC, Tassilo Horn
Details
A very simple class diagram I mentioned in my second posting. (149.88 KB, text/plain)
2005-08-05 14:08 UTC, Tassilo Horn
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tassilo Horn 2005-08-05 13:53:54 UTC
Version:           1.4.2 (using KDE 3.4.2, Gentoo)
Compiler:          gcc version 3.4.4 (Gentoo 3.4.4, ssp-3.4.4-1.0, pie-8.7.8)
OS:                Linux (i686) release 2.6.12-nitro2

In my class diagram I have several associations with labels / multiplicities I've moved manually. They are moved incorrectly when moving the corresponding classes.

I'll demonstrate this behaviour with the attached diagram:

1. Open it!
2. Open the class diagram "CoreClasses".
3. Move the class "Source". => The association "containsSource" with all its labels are moved correctly.
4. Move the class "Project". => The association "containsSource" name and the multiplicity on the "Project"-side aren't moved, so it seems. Now move the class in huge circles over the whole diagram. After that the labels are at totally wrong positions. This applies to all associations connected to "Project" with manually moved labels / multiplicity.
5. Do a "Reset Label Positions" on the association. => The labels are positioned correctly, but "whirling" confuses them again.

To mention it again: This only seems to apply to associations which have manually moved labels / roles / multiplicity.

In my opinion it would be good if Umbrello forgets if an label has ever been moved when clicking "Reset Label Positions".
Comment 1 Tassilo Horn 2005-08-05 13:55:03 UTC
Created attachment 12097 [details]
The diagram where this bug is reproducible
Comment 2 Tassilo Horn 2005-08-05 14:06:58 UTC
I investigated a little further and it seems to apply to assiciation which weren't moved manually, too.

Have a look at the very simple class diagram I below this message. I created 3 classes and 2 associations. I never moved labels manually.

Now take class b and move it in circles arround class c and all labels get messed up.
Comment 3 Tassilo Horn 2005-08-05 14:08:37 UTC
Created attachment 12098 [details]
A very simple class diagram I mentioned in my second posting.

Here you can see that it applies to labels not moved manually, too.
Comment 4 Oliver Kellogg 2005-08-05 17:36:09 UTC
A well known bug. Alas, nobody has mustered the spunk to fix it :)
Comment 5 Oliver Kellogg 2005-08-07 15:03:10 UTC
SVN commit 443800 by okellogg:

CCBUG:110231 - First stab at constraining the text positions.
Only works for exactly vertical and horizontal association lines.
Only works when actively dragging around a FloatingText, i.e. does
not yet work as a side effect of moving one of the role objects.
In other words, all the hard stuff is still To Be Done :)


 M  +107 -0    associationwidget.cpp  
 M  +13 -0     associationwidget.h  
 M  +1 -4      floatingtext.cpp  
 M  +0 -5      linkwidget.cpp  
 M  +4 -3      linkwidget.h  


--- branches/KDE/3.5/kdesdk/umbrello/umbrello/associationwidget.cpp #443799:443800
@@ -1870,6 +1870,113 @@
     return p;
 }
 
+void AssociationWidget::constrainTextPos(int &textX, int &textY,
+                                         int textWidth, int textHeight,
+                                         Uml::Text_Role tr) {
+    const int CORRIDOR_HALFWIDTH = 30;
+    const int textHalfWidth = textWidth / 2;
+    const int textHalfHeight = textHeight / 2;
+    const int textCenterX = textX + textHalfWidth;
+    const int textCenterY = textY + textHalfHeight;
+    const uint lastSegment = m_LinePath.count() - 1;
+    QPoint p0, p1;
+    bool atBSide = false;
+    switch (tr) {
+        case tr_RoleAName:
+        case tr_MultiA:
+        case tr_ChangeA:
+            p0 = m_LinePath.getPoint(0);
+            p1 = m_LinePath.getPoint(1);
+            break;
+        case tr_RoleBName:
+        case tr_MultiB:
+        case tr_ChangeB:
+            p0 = m_LinePath.getPoint(lastSegment - 1);
+            p1 = m_LinePath.getPoint(lastSegment);
+            atBSide = true;
+            break;
+        case tr_Name:
+            // @todo Find the linepath segment to which the (textX,textY) is closest
+            //       and constrain to the corridor of that segment.
+            return;
+            break;
+        default:
+            kdError() << "AssociationWidget::constrainTextPos(): unexpected Text_Role "
+                      << tr << endl;
+            return;
+            break;
+    }
+    if (p0.x() == p1.x()) {
+        // vertical line
+        // CAUTION: This is calculated in Qt coordinates!
+        ////////////////////////// constrain horizontally /////////////////////////
+        const int lineX = p0.x();
+        if (textX + textWidth < lineX - CORRIDOR_HALFWIDTH)  // constrain at left
+            textX = lineX - CORRIDOR_HALFWIDTH - textWidth;
+        else if (textX > lineX + CORRIDOR_HALFWIDTH)         // constrain at right
+            textX = lineX + CORRIDOR_HALFWIDTH;
+        ////////////////////////// constrain vertically ///////////////////////////
+        // pre-constrain the corridor to the appropriate half:
+        if (atBSide) {
+            if (p0.y() > p1.y())
+                p0.setY(p1.y() + (p0.y() - p1.y()) / 2);
+            else
+                p0.setY(p1.y() - (p1.y() - p0.y()) / 2);
+        } else {
+            if (p0.y() < p1.y())
+                p1.setY(p0.y() + (p1.y() - p0.y()) / 2);
+            else
+                p1.setY(p0.y() - (p0.y() - p1.y()) / 2);
+        }
+        // swap points so that p0 contains the one with the smaller Y
+        if (p0.y() > p1.y()) {
+            QPoint tmp = p0;
+            p0 = p1;
+            p1 = tmp;
+        }
+        if (textY + textHeight < p0.y())  // constrain at top
+            textY = p0.y() - textHeight;
+        else if (textY > p1.y())          // constrain at bottom
+            textY = p1.y();
+        return;
+    }
+    if (p0.y() == p1.y()) {
+        // horizontal line
+        // CAUTION: This is calculated in Qt coordinates!
+        ////////////////////////// constrain verticallly ///////////////////////////
+        const int lineY = p0.y();
+        if (textY + textHeight < lineY - CORRIDOR_HALFWIDTH)  // constrain at top
+            textY = lineY - CORRIDOR_HALFWIDTH - textHeight;
+        else if (textY > lineY + CORRIDOR_HALFWIDTH)          // constrain at bottom
+            textY = lineY + CORRIDOR_HALFWIDTH;
+        ////////////////////////// constrain horizontally //////////////////////////
+        // pre-constrain the corridor to the appropriate half:
+        if (atBSide) {
+            if (p0.x() < p1.x())
+                p0.setX(p1.x() - (p1.x() - p0.x()) / 2);
+            else
+                p0.setX(p1.x() + (p0.x() - p1.x()) / 2);
+        } else {
+            if (p0.x() < p1.x())
+                p1.setX(p0.x() + (p1.x() - p0.x()) / 2);
+            else
+                p1.setX(p0.x() - (p0.x() - p1.x()) / 2);
+        }
+        // swap points so that p0 contains the one with the smaller X
+        if (p0.x() > p1.x()) {
+            QPoint tmp = p0;
+            p0 = p1;
+            p1 = tmp;
+        }
+        if (textX + textWidth < p0.x())   // constrain at left
+            textX = p0.x() - textWidth;
+        else if (textX > p1.x())          // constrain at right
+            textX = p1.x();
+        return;
+    }
+    // @todo: deal with slopes
+}
+
 void AssociationWidget::calculateNameTextSegment() {
     if(!m_pName) {
         return;
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/associationwidget.h #443799:443800
@@ -490,6 +490,19 @@
     void resetTextPositions();
 
     /**
+     * Constrains the FloatingText X and Y values supplied.
+     * Implements the abstract operation from LinkWidget.
+     *
+     * @param textX       Candidate X value (may be modified by the constraint.)
+     * @param textY       Candidate Y value (may be modified by the constraint.)
+     * @param textWidth   Width of the text.
+     * @param textHeight  Height of the text.
+     * @param tr          Uml::Text_Role of the text.
+     */
+    void constrainTextPos(int &textX, int &textY, int textWidth, int textHeight,
+                          Uml::Text_Role tr);
+
+    /**
      * Shows the association properties dialog and updates the
      * corresponding texts if its execution is successful.
      * Returns true for success.
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/floatingtext.cpp #443799:443800
@@ -304,10 +304,7 @@
     int newX = newPosition.x();
     int newY = newPosition.y();
 
-    //implement specific rules for a sequence diagram
-    if (m_Role == Uml::tr_Seq_Message || m_Role == Uml::tr_Seq_Message_Self) {
-        m_pLink->constrainTextPos(newX, newY, width(), height(), m_Role);
-    }
+    m_pLink->constrainTextPos(newX, newY, width(), height(), m_Role);
     m_nOldX = newX;
     m_nOldY = newY;
     setX( newX );
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/linkwidget.cpp #443799:443800
@@ -55,11 +55,6 @@
     return true;
 }
 
-void LinkWidget::constrainTextPos(int & /*textX*/, int & /*textY*/,
-                                  int /*textWidth*/, int /*textHeight*/,
-                                  Uml::Text_Role /*tr*/) {
-}
-
 void LinkWidget::calculateNameTextSegment() {
 }
 
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/linkwidget.h #443799:443800
@@ -113,11 +113,12 @@
     virtual void setSeqNumAndOp(const QString &seqNum, const QString &op) = 0;
 
     /**
+     * Abstract operation implemented by inheriting classes.
      * Motivated by FloatingText::mouseMoveEvent()
-     * Only applies to MessageWidget.
      */
-    virtual void constrainTextPos(int &textX, int &textY, int textWidth, int textHeight,
-                                  Uml::Text_Role tr);
+    virtual void constrainTextPos(int &textX, int &textY,
+                                  int textWidth, int textHeight,
+                                  Uml::Text_Role tr) = 0;
 
     /**
      * Motivated by FloatingText::setLink().
Comment 6 Oliver Kellogg 2005-08-08 01:31:43 UTC
SVN commit 443930 by okellogg:

calculateTextPosition(tr_Name): Add missing assignment of `text'.
Also, apply the constrained coordinates to the FloatingText.
BUG:110231


 M  +12 -5     associationwidget.cpp  


--- branches/KDE/3.5/kdesdk/umbrello/umbrello/associationwidget.cpp #443929:443930
@@ -1699,7 +1699,7 @@
     int textW = 0, textH = 0;
     int slope = 0, divisor = 1;
     const int SPACE = 2;
-    FloatingText const * text = 0;
+    FloatingText *text = 0;
 
     if(role == tr_MultiA) {
         text = getMultiWidget(A);
@@ -1778,6 +1778,8 @@
 
     } else if(role == tr_Name) {
 
+        calculateNameTextSegment();
+        text = m_pName;
         x = (int)( ( m_LinePath.getPoint(m_unNameLineSegment).x() +
                      m_LinePath.getPoint(m_unNameLineSegment + 1).x() ) / 2 );
 
@@ -1868,9 +1870,14 @@
     }
     if (text) {
         constrainTextPos(x, y, textW, textH, role);
-        // kdDebug() << "AssociationWidget::calculateTextPosition(" 
-        //   << p.x() << "," << p.y() << "): newPoint=("
-     	//   << x << "," << y << ")" << endl;
+        if (x != p.x() || y != p.y()) {
+            // kdDebug() << "AssociationWidget::calculateTextPosition(" 
+            //   << text->getName() << ") textrole " << role
+            //   << ": oldpoint=(" << p.x() << "," << p.y() << ")"
+            //   << ", newPoint=(" << x << "," << y << ")" << endl;
+            text->setX(x);
+            text->setY(y);
+        }
     }
     p = QPoint( x, y );
     return p;
@@ -1973,7 +1980,7 @@
     if (p0.y() == p1.y()) {
         // horizontal line
         // CAUTION: This is calculated in Qt coordinates!
-        ////////////////////////// constrain verticallly ///////////////////////////
+        ////////////////////////// constrain vertically /////////////////////////////
         const int lineY = p0.y();
         if (textY + textHeight < lineY - CORRIDOR_HALFWIDTH)  // constrain at top
             textY = lineY - CORRIDOR_HALFWIDTH - textHeight;
Comment 7 Oliver Kellogg 2005-09-06 23:32:13 UTC
*** Bug 107171 has been marked as a duplicate of this bug. ***