Bug 449622 - fit to screen after loading file
Summary: fit to screen after loading file
Status: RESOLVED WORKSFORME
Alias: None
Product: umbrello
Classification: Applications
Component: general (show other bugs)
Version: Git
Platform: Microsoft Windows Microsoft Windows
: NOR normal
Target Milestone: ---
Assignee: Umbrello Development Group
URL:
Keywords:
: 449347 449621 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-02-04 21:43 UTC by Jessica
Modified: 2022-12-30 09:57 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 2.33.80 (KDE releases 22.03.80)


Attachments
test file (1.13 MB, application/xml)
2022-02-04 21:44 UTC, Jessica
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jessica 2022-02-04 21:43:32 UTC
When loading an existing file, the diagram is not displayed fit to screen. It's displayed very tiny in the middle of the screen (unreadable) but this view has no real use. When an existing file is loaded, it should be fitted to the screen immediately after loading (same as option "fit").

Example file attached

OBSERVED RESULT
An existing file is not fitted to the screen when loading. 

EXPECTED RESULT
When an existing file is loaded, it should be fitted to the screen immediately after loading.


SOFTWARE/OS VERSIONS
Windows 10
git 37b9e8e
Comment 1 Jessica 2022-02-04 21:44:00 UTC
Created attachment 146278 [details]
test file
Comment 2 Oliver Kellogg 2022-02-04 21:55:30 UTC
*** Bug 449621 has been marked as a duplicate of this bug. ***
Comment 3 Oliver Kellogg 2022-02-10 15:14:58 UTC
It looks as though version 2.32 may produce some weird diagram coordinate values.
First line in <diagram>:

<diagram showopsig="1" [...] canvasheight="4.29497e+09" canvaswidth="4.29487e+09" 

The canvasheight and canvaswidth values are unusable.
By fortunate chance, the diagram loadFromXMI function happens to ignores these attributes.

However, there are also huge offsets on the x/y coordinates in the diagram  elements.
First <classwidget> in <diagram>:
<classwidget linecolor="#ff0000" [...] x="-95708.9" showattsigs="601" showstereotype="1" y="-36843.3" 

Looking at the x / y values of the further widgets, they all have a huge offset factor.

For the load code this would mean there would need to be an extra first pass which determines the minimum of all x and the minimum of all y values, and in the actual loading pass the minX / minY would need to be subtracted from the x / y values.
I'm not sure if and when I get around to implementing that - but there is a fairly simple workaround:
Select the arrow tool and draw a rectangle around the objects (group selection). Then move the group to the top left corner of the diagram.
Comment 4 Oliver Kellogg 2022-02-10 15:21:50 UTC
(In reply to Oliver Kellogg from comment #3)
> [...]
> However, there are also huge offsets on the x/y coordinates in the diagram 
> elements.
> First <classwidget> in <diagram>:
> <classwidget linecolor="#ff0000" [...] x="-95708.9" showattsigs="601"
> showstereotype="1" y="-36843.3" 
> 
> Looking at the x / y values of the further widgets, they all have a huge
> offset factor.
> 
> For the load code this would mean there would need to be an extra first pass
> which determines the minimum of all x and the minimum of all y values, and
> [...]

I looked into this and there is one line in the file that does not fit with this approach.
It is line 1895,
<floatingtext linecolor="#ff0000" [...] x="-4.29496e+09" [...] y="-4.295e+09" text="sprites" 

In order for the approach to work, this line needs to be removed manually.
Reason: The x and y values are completely bonkers. The rest of x / y values have offsets of approx. -98000 and -38000, respectively.
Comment 5 Oliver Kellogg 2022-02-11 12:50:47 UTC
Git commit d8fe02e4f1f963d67fdfc7072ca8cc9c4292b0d9 by Oliver Kellogg.
Committed on 11/02/2022 at 12:50.
Pushed by okellogg into branch 'master'.

https://bugs.kde.org/show_bug.cgi?id=449622#c3

> For the load code this would mean there would need to be an extra
> first pass which determines the minimum of all x and the minimum of
> all y values, [...]

umbrello/umlscene.h
- Split static member defaultCanvasSize into the two static members
  s_defaultCanvasWidth and s_defaultCanvasHeight for finer control.
- At static member m_showDocumentationIndicator change prefix from "m_"
  to "s_" for symmetry with other static members.
- New members m_minX, m_minY, m_maxX, m_maxY gather data used for
  estimating required canvas size. They are filled during loadFromXMI().
- New members m_fixX and m_fixY are used for compensating for undue
  offsets of x/y values in previous version XMI files.
  Further, the QGraphicsScene coordinate system is changed to originate
  at x/y position (0,0).
  This means there are no longer negative values for x/y coordinates.
- New function updateCanvasSizeEstimate is called during loadFromXMI()
  of the individual widgets. The members m_minX, m_minY, m_maxX, m_maxY
  may be updated by the arguments x, y, w, h.
- New getters fixX() and fixY() return m_fixX and m_fixY, respectively.
  They are called from the loadFromXMI functions of individual widgets.

umbrello/umlscene.cpp
- In function loadFromXMI :
  - Parse attributes "canvaswidth" and "canvasheight" into qreals
    canvasWidth and canvasHeight but leave their value at 0.0 if the XML
    attribute values are implausible.
  - Before the loop which loads widgets from XMI add a loop which only
    parses the x and y values of the widgets and sets variables
    xNegOffset, yNegOffset, xPosOffset, yPosOffset to the minimum resp.
    maximum values encountered. After end of loop the members m_fixX and
    m_fixY may be altered depending on the [xy]{Neg,Pos}Offset values.
  - After the widget-load loop, if the calculated m_maxX, m_maxY exceed
    canvasWidth or canvasHeight then call setSceneRect with arguments
    x=0, y=0, w=m_maxX, h=m_maxY.

umbrello/umlwidgets/umlwidget.cpp
- In function loadFromXMI :
  - Local qreals nX, nY receive result of toDoubleFromAnyLocale(x) and
    toDoubleFromAnyLocale(y) respectively.
  - Local qreals fixedX, fixedY receive result of nX+umlScene()->fixX()
    and nY + umlScene()->fixY().
  - Local qreals scaledX, scaledY receive result of fixedX * dpiScale
    and fixedY * dpiScale.
  - Add call to umlScene()->updateCanvasSizeEstimate() with arguments
    scaledX, scaledY, scaledW, scaledH.

umbrello/umlwidgets/associationline.cpp
- In function loadFromXMI, on initializing nX add umlScene->fixX() and
  on initializing nY add umlScene->fixY().

M  +185  -7    umbrello/umlscene.cpp
M  +12   -2    umbrello/umlscene.h
M  +3    -2    umbrello/umlwidgets/associationline.cpp
M  +13   -5    umbrello/umlwidgets/umlwidget.cpp

https://invent.kde.org/sdk/umbrello/commit/d8fe02e4f1f963d67fdfc7072ca8cc9c4292b0d9
Comment 6 Oliver Kellogg 2022-02-12 21:07:51 UTC
Git commit 73b335b985bfa9a1f3ad5d4339d4b8a561384e68 by Oliver Kellogg.
Committed on 12/02/2022 at 21:06.
Pushed by okellogg into branch 'master'.

umbrello/umlwidgets/associationline.cpp function loadFromXMI followup to commit d8fe02e :

- On assigning to nX from attribute "endx" add umlScene->fixX().
- On assigning to nY from attribute "endy" add umlScene->fixY().
- In loop reading <point> subelements,
  - in call to point.setX() add umlScene->fixX();
  - in call to point.setY() add umlScene->fixY().

M  +6    -6    umbrello/umlwidgets/associationline.cpp

https://invent.kde.org/sdk/umbrello/commit/73b335b985bfa9a1f3ad5d4339d4b8a561384e68
Comment 7 Jessica 2022-02-18 18:23:49 UTC
*** Removed by KDE Sysadmin for violation of the KDE Community Code of Conduct ***
Comment 8 Oliver Kellogg 2022-03-12 06:32:45 UTC
Git commit 796a451ac8811401b6f37e3286707da636933dda by Oliver Kellogg.
Committed on 12/03/2022 at 06:32.
Pushed by okellogg into branch 'master'.

https://bugs.kde.org/show_bug.cgi?id=449622#c5 fix for displaced pins, ports, and their associated floating texts

umbrello/umlwidgets/umlwidget.cpp followup to commit d8fe02e
- In function loadFromXMI :
  - Initialize fixedX to just nX.
  - Initialize fixedY to just nY.
  - Local bool usesRelativeCoords indicates whther this widget's
    coordinates are relative to its parentItem().
  - Apply umlScene()->fixX() to fixedX and apply umlScene()->fixY() to
    fixedY only if usesRelativeCoords is false.
  - Call umlScene()->updateCanvasSizeEstimate() only if
    usesRelativeCoords is false.

umbrello/umlwidgets/pinportbase.cpp
- In function loadFromXMI, on loading optional child floatingtext move
  call to m_pName->setParentItem(this) to before m_pName->loadFromXMI().
  Reason: UMLWidget::loadFromXMI needs to evaluate whether the widget
  coordinates are relative to a parent UMLWidget (see above).

M  +2    -1    umbrello/umlwidgets/pinportbase.cpp
M  +14   -3    umbrello/umlwidgets/umlwidget.cpp

https://invent.kde.org/sdk/umbrello/commit/796a451ac8811401b6f37e3286707da636933dda
Comment 9 Oliver Kellogg 2022-12-30 02:33:37 UTC
*** Bug 449347 has been marked as a duplicate of this bug. ***
Comment 10 Oliver Kellogg 2022-12-30 09:57:33 UTC
Git commit 980f0c120fe13b822e998a8e6af67913d5d4b40a by Oliver Kellogg.
Committed on 30/12/2022 at 09:57.
Pushed by okellogg into branch 'master'.

Address https://bugs.kde.org/show_bug.cgi?id=449622#c4 ,

> I looked into this and there is one line in the file that does not fit
> with this approach.  It is line 1895,
> <floatingtext [...] x="-4.29496e+09"  y="-4.295e+09" text="sprites"
> In order for the approach to work, this line needs to be removed
> manually.

Improve widget coordinate error handling so that no manual intervention
is needed:

umbrello/umlscene.{h,cpp}
- Make function maxCanvasSize() static and document it.

umbrello/umlwidgets/umlwidget.cpp
- In functions setX and setY call UMLScene::maxCanvasSize() as a static
  function.
- In function loadFromXMI, if nX or nY are outside the range
    -UMLScene::maxCanvasSize() .. UMLScene::maxCanvasSize()
  then log a warning, set them to a fixed small value, and do not
  apply offset correction.

M  +14   -0    umbrello/umlscene.cpp
M  +2    -2    umbrello/umlscene.h
M  +19   -6    umbrello/umlwidgets/umlwidget.cpp

https://invent.kde.org/sdk/umbrello/commit/980f0c120fe13b822e998a8e6af67913d5d4b40a