Bug 447866 - [class diagram] Multiple association ends are drawn on the same edge point
Summary: [class diagram] Multiple association ends are drawn on the same edge point
Status: ASSIGNED
Alias: None
Product: umbrello
Classification: Applications
Component: general (show other bugs)
Version: Git
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Umbrello Development Group
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-01-03 06:28 UTC by Oliver Kellogg
Modified: 2022-02-16 17:22 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Kellogg 2022-01-03 06:28:34 UTC
SUMMARY
***
Multiple association ends are drawn on the same edge point in class diagrams.
***

On class diagram with multiple classes and multiple associations among them, the association end points are lumped together on either a middle point or a corner of the class box edges.
In some cases this may look ugly, see e.g. https://bugs.kde.org/attachment.cgi?id=145055

STEPS TO REPRODUCE
Load https://bugs.kde.org/attachment.cgi?id=145055

OBSERVED RESULT
In some situations there is no way of spacing out the association end points to visually separate them
because the automatic end point computation interferes.
As an example of what the desired layout looks like for the reproducer, see
https://bugsfiles.kde.org/attachment.cgi?id=145048

EXPECTED RESULT
Manually adjusting the association end points shall be possible.
Comment 1 Oliver Kellogg 2022-01-03 07:50:35 UTC
Git commit 33610081b96ad4ebf2abc5839081be7dd40f1e41 by Oliver Kellogg.
Committed on 03/01/2022 at 07:50.
Pushed by okellogg into branch 'master'.

Minimal fix for "Multiple association ends are drawn on the same edge point"

umbrello/umlwidgets/associationline.cpp
- In function mousePressEvent case (event->buttons() & Qt::LeftButton),
  remove the code for "end points are not drawn and hence not active",
  i.e. remove the snippet
  if (m_activePointIndex != -1 && isEndPointIndex(m_activePointIndex)) {
      m_activePointIndex = -1;
  }
- As a peripheral change, fix a typo in documentation of functions
  hoverEnterEvent, hoverMoveEvent, hoverLeaveEvent.

umbrello/umlwidgets/associationwidget.cpp
- In function activate(IDChangeLog*) only call calculateEndingPoints()
  if umlDoc()->loading() returns false.
  Reason: Manually adjusted association endpoint positions saved in the
  XMI file are overwritten by calculateEndingPoints().
- As peripheral changes,
  - in function moveEvent remove a few unneeded parentheses in if-
    conditions;
  - in function updateRegionLineCount debug message print the function
    name.

The above fix is only minimal because as soon as a class is moved on the
diagram, the manual adjustments are overwritten by the automatic
endpoint calculation.
Currently, manual adjustments should be done as the last step.

M  +3    -7    umbrello/umlwidgets/associationline.cpp
M  +10   -6    umbrello/umlwidgets/associationwidget.cpp

https://invent.kde.org/sdk/umbrello/commit/33610081b96ad4ebf2abc5839081be7dd40f1e41
Comment 2 Robert Hairgrove 2022-01-04 12:11:26 UTC
Thank you, Oliver! I tried this, and now I can create two aggregations between the same two class without a problem.

Over all, the drawing behavior is much more natural ... the bird view still has problems, though. Sometimes it will not update after adding objects to the diagram until the window is resized. But at least I can move one widget around without moving the other widgets in the opposite direction.

After saving the file, closing the app and opening the file again, the text labels for role and multiplicity are placed in strange places.

Another thing I noticed is that if I save a file, it does not add the ".xmi" extension if I do not type it explicitly. Also, default permissions are set to 600 instead of the expected 644 or 664 on Linux.
Comment 3 Oliver Kellogg 2022-01-05 15:07:14 UTC
Git commit 136460f667e99f2757f8a9c677b80552e6fb3a18 by Oliver Kellogg.
Committed on 05/01/2022 at 15:06.
Pushed by okellogg into branch 'master'.

Address observation from https://bugs.kde.org/show_bug.cgi?id=447866#c2
> [...]
> After saving the file, closing the app and opening the file again, the
> text labels for role and multiplicity are placed in strange places.

umbrello/umlscene.cpp
- In function addFloatingTextWidget disable range check of pWidget x()
  and y() coordinates.  For details see comment at source location.

M  +9    -1    umbrello/umlscene.cpp

https://invent.kde.org/sdk/umbrello/commit/136460f667e99f2757f8a9c677b80552e6fb3a18
Comment 4 Oliver Kellogg 2022-01-06 07:56:07 UTC
Git commit 4605383f42b8ee61c818231b1d9509b1bc6b59f4 by Oliver Kellogg.
Committed on 06/01/2022 at 07:55.
Pushed by okellogg into branch 'master'.

umbrello/umlwidgets/associationwidget.{h,cpp} followup to commit 136460f
- At function linePathStartsAt add `const' qualifier.
- Add function linePathEndsAt in symmetry with linePathStartsAt.
- In associationwidget.cpp,
  - define preprocessor symbol PIXEL_TOLERANCE for use in functions
    linePathStartsAt/linePathEndsAt;
  - in functions setStereotype and mouseDoubleClickEvent replace call to
    uDebug() by DEBUG(DBG_SRC) for control from within Umbrello's Debug
    dock.

M  +33   -8    umbrello/umlwidgets/associationwidget.cpp
M  +2    -1    umbrello/umlwidgets/associationwidget.h

https://invent.kde.org/sdk/umbrello/commit/4605383f42b8ee61c818231b1d9509b1bc6b59f4
Comment 5 Oliver Kellogg 2022-01-07 13:43:11 UTC
I am changing the automatic line update algorithm so that it does not snap to midpoints/corners but instead uses the widget center as a reference point.
The association end point is mapped onto the widget edge using the connecting line to the center.
Comment 6 Robert Hairgrove 2022-01-07 14:55:20 UTC
(In reply to Oliver Kellogg from comment #5)
> I am changing the automatic line update algorithm so that it does not snap
> to midpoints/corners but instead uses the widget center as a reference point.
> The association end point is mapped onto the widget edge using the
> connecting line to the center.

Sounds good to me.

Thank you for all of your hard work improving Umbrello's stability and usability, Oliver! (BTW, wishing you a very Happy New Year!)
Comment 7 Oliver Kellogg 2022-01-07 16:04:17 UTC
Git commit e3595be8eb40a5a810da39ff397dc29c747bbd77 by Oliver Kellogg.
Committed on 07/01/2022 at 16:03.
Pushed by okellogg into branch 'master'.

Improve instrumentation for debugging of widget geometry :

umbrello/umlscene.cpp
- In function addWidgetCmd,
  - use DEBUG(DBG_SRC) in lieu of uDebug();
  - add printing of name, width, height.

umbrello/umlwidgets/classifierwidget.cpp
- In function calculateSize add DEBUG(DBG_SRC) message printing current
  and calculated width/height.

umbrello/umlwidgets/umlwidget.cpp
- In function constrain() case fixedAspectRatio() add DEBUG(DBG_SRC)
  message printing input and recalculated height.
- In function adjustAssocs replace qDebug() by DEBUG(DBG_SRC) and print
  name/width/height in lieu of `this' pointer.
- In functions setSize, updateGeometry add debug statement printing name
  and geometry related members.

umbrello/umlwidgets/widgetbase.cpp
- In function setRect replace uDebug() by DEBUG(DBG_SRC) and print
  `rect' in lieu of rect.x(), rect.y().

M  +3    -2    umbrello/umlscene.cpp
M  +3    -0    umbrello/umlwidgets/classifierwidget.cpp
M  +8    -2    umbrello/umlwidgets/umlwidget.cpp
M  +2    -2    umbrello/umlwidgets/widgetbase.cpp

https://invent.kde.org/sdk/umbrello/commit/e3595be8eb40a5a810da39ff397dc29c747bbd77
Comment 8 Oliver Kellogg 2022-01-08 08:20:45 UTC
Git commit dc485c16219a61d802ddf462f809e9cd7c514af3 by Oliver Kellogg.
Committed on 08/01/2022 at 08:19.
Pushed by okellogg into branch 'master'.

First stab at smoothing associationwidget endpoint update as announced
  in https://bugs.kde.org/show_bug.cgi?id=447866#c5

umbrello/umlwidgets/associationwidget.{h,cpp}
- At function calculateEndingPoints add argument UMLWidget *pWidget
  indicating the selected widget of move or resize.  If passed in as
  null then updateAssociations() will be called twice, once for the
  current associationwidget's role A widget and again for its role B
  widget.
- Add function findIntercept(const QRectF& rect, const QPointF& point,
                                                       QPointF& result)
  returning bool true if the line from the center of `rect' to `point'
  intersects with one of rect's sides, and returning the intersection
  point in `result'.
- Remove functions updateRegionLineCount and doUpdates.
- Declare function updateAssociations `static' and change arguments to
    UMLWidget *pWidget, AssociationWidgetList list.
  For details see documentation at the implementation.

M  +184  -285  umbrello/umlwidgets/associationwidget.cpp
M  +3    -12   umbrello/umlwidgets/associationwidget.h

https://invent.kde.org/sdk/umbrello/commit/dc485c16219a61d802ddf462f809e9cd7c514af3
Comment 9 Oliver Kellogg 2022-01-10 18:52:22 UTC
Git commit d6bf2bcd53df319450a1f860f9a2fc69131544f4 by Oliver Kellogg.
Committed on 10/01/2022 at 18:51.
Pushed by okellogg into branch 'master'.

umbrello/umlwidgets/associationwidget.cpp followup to commit dc485c1
- In function findIntercept use QLineF type IntersectType and function
  intersect() in lieu of IntersectionType and intersects().
  Reason: This restores the Qt4 based build.

M  +1    -1    umbrello/umlwidgets/associationwidget.cpp

https://invent.kde.org/sdk/umbrello/commit/d6bf2bcd53df319450a1f860f9a2fc69131544f4
Comment 10 Oliver Kellogg 2022-01-15 14:21:00 UTC
Git commit 1816faa68be2ccfa42d870b78755853b300e076a by Oliver Kellogg.
Committed on 15/01/2022 at 14:20.
Pushed by okellogg into branch 'master'.

Address observation from https://bugs.kde.org/show_bug.cgi?id=447866#c2
> [...]
> Another thing I noticed is that if I save a file, it does not add the
> ".xmi" extension if I do not type it explicitly.

umbrello/uml.cpp function slotFileSaveAs
- In loop, after call to QFileDialog::getSaveFileUrl() :
  - Use `break' instead of setting `cont' false to avoid unnecessary
    if-nesting depth.
  - Before testing QFile::exists(file), if `file' does not contain "."
    then append extension ".xmi".
  - On calling KMessageBox::warningContinueCancel() use `file' in lieu
    of url.toLocalFile().

M  +21   -18   umbrello/uml.cpp

https://invent.kde.org/sdk/umbrello/commit/1816faa68be2ccfa42d870b78755853b300e076a
Comment 11 Oliver Kellogg 2022-02-05 00:48:45 UTC
Git commit 4108be7b389d38d22e013e9cb9b88d03441f0363 by Oliver Kellogg.
Committed on 05/02/2022 at 00:48.
Pushed by okellogg into branch 'master'.

umbrello/umlwidgets/associationwidget.{h,cpp} : Revert most of commits f80bd68 and dc485c1, they destroyed association line attachments.
Related: bug 449463, bug 449626, bug 449627

M  +732  -187  umbrello/umlwidgets/associationwidget.cpp
M  +14   -4    umbrello/umlwidgets/associationwidget.h

https://invent.kde.org/sdk/umbrello/commit/4108be7b389d38d22e013e9cb9b88d03441f0363
Comment 12 Oliver Kellogg 2022-02-16 17:22:12 UTC
Git commit 8bb117e11a887be9704f8711dbb0ac4afa797eb2 by Oliver Kellogg.
Committed on 16/02/2022 at 17:21.
Pushed by okellogg into branch 'master'.

Followup to commit dc485c1 smoothing of associationwidget endpoints:

umbrello/umlwidgets/associationwidget.{h,cpp}
- Readd #define PIXEL_TOLERANCE but with value 30.
  Experiments show that this larger tolerance is required.
- Add function findIntercept(const QRectF& rect, const QPointF& point,
                                                       QPointF& result)
  returning bool true if the line from the center of `rect' to `point'
  intersects with one of rect's sides, and returning the intersection
  point in `result'.
- Remove functions updateRegionLineCount, doUpdates, getRegionCount.
- Re-remove obsoleted members (as previously done in commit f80bd68):
  findInterceptOnEdge, insertIntoLists, m_positions, m_positions_len,
  m_ordered.
- Static function setStartAndEndPoint factors the point calculation
  logic from updateAssociations() for reuse in other contexts.
- Declare function updateAssociations `static' and change arguments to
  UMLWidget *pWidget, AssociationWidgetList list.
  For details see documentation at the implementation.
- Readd function linePathEndsAt in symmetry with linePathStartsAt.

umbrello/umlwidgets/associationwidget.cpp
- In function linePathStartsAt,
  - document why we are not using widget->contains(lpStart);
  - remove bool result1 and its calculation.
- In function cleanup() remove calls that may lead to crash due to
  possible call of calculateEndingPoints on the AssociationWidget being
  removed.
- In function moveEvent test of m_role[RoleType::B].changeabilityWidget
  test movingPoint against pos-1 instead of against 1.
- In function calculateEndingPoints,
  - call setStartAndEndPoint(this, m_role[RoleType::A].umlWidget);
  - call setStartAndEndPoint(this, m_role[RoleType::B].umlWidget);
  - define local assocList as shorthand for m_scene->associationList();
  - call updateAssociations(m_role[RoleType::A].umlWidget, assocList);
  - call updateAssociations(m_role[RoleType::B].umlWidget, assocList).
- Reduce function updateAssociations to loop over the passed in
  AssociationWidgetList and call setStartAndEndPoint on each
  assocwidget.

M  +214  -761  umbrello/umlwidgets/associationwidget.cpp
M  +5    -15   umbrello/umlwidgets/associationwidget.h

https://invent.kde.org/sdk/umbrello/commit/8bb117e11a887be9704f8711dbb0ac4afa797eb2