Bug 446020 - Bird's eye view shows nothing at first open
Summary: Bird's eye view shows nothing at first open
Status: CONFIRMED
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: 2021-11-24 09:55 UTC by Robert Hairgrove
Modified: 2024-03-16 19:50 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Screenshot showing the issue (70.10 KB, image/png)
2024-03-07 22:44 UTC, Ralf Habacker
Details
Screenshot with UI extension for "Adjust scene size when pressing 'Fit'" (56.19 KB, image/png)
2024-03-07 22:55 UTC, Ralf Habacker
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Hairgrove 2021-11-24 09:55:44 UTC
SUMMARY
Bitd's eye view shows nothing until the splitter control is dragged or the main window is resized.

STEPS TO REPRODUCE
1. Open Umbrello and add something to the empty diagram (e.g. two or three new classes, perhaps draw some association lines);
2. Move things around with the mouse. 

OBSERVED RESULT
The bird's eye view remains empty until the splitter control is moved or the main window is resized.

EXPECTED RESULT
The bird's eye view should refresh the display immediately when items are added to the diagram or moved.

SOFTWARE/OS VERSIONS
Linux: Ubuntu 18.04.6 LTS
KDE Plasma Version:  ???
KDE Frameworks Version: 5 
Qt Version: 5.12.9

ADDITIONAL INFORMATION
Umbrello compiled from Git master -- Version 2.31.70-d8c161b2a (experimental) shown in "About Umbrello"
Comment 1 Robert Hairgrove 2021-11-26 21:01:38 UTC
In the meantime, I was able to get the bird's eye view to show when a new widget is added to the diagram. I noticed that selecting something in the tree view would also trigger the view, so I looked at UMLScene::slotObjectCreated(UMLObject* o). At the end of the method body, right after the last line which calls "resizeSceneToItems()", I added these lines:

    UMLView* cv = activeView();
    if (cv) {
        // this should activate the bird view:
        UMLApp::app()->setCurrentView(cv, false);
    }

The last parameter to setCurrentView() has a default value of "true", which causes the tree view focus to change e.g. from "class_A" (or whatever the new widget was named) to the class diagram. Passing "false" keeps the focus on the widget in the tree view, which seems much more intuitive to me since many properties would normally need to be edited after creating and naming the widget. In UMLApp::setCurrentView(), there is a call to "createBirdView()" which deletes any existing BirdView object and creates a new one. Seems rather heavy-handed to me, but I couldn't figure out which signals to catch that would actually do anything useful.

However, there is much more "bugginess" about the bird's eye view (and the drawing behavior in general) which needs to be looked at. For example, if I have added two new classes to an empty class diagram and move one of them, the other moves as well but in the opposite direction! There seems to be code somewhere causing the scene to always be centered ... is this by design? I would probably have to put widgets at the four corners of the virtual page to keep things in the middle area from wandering about on the screen when one of them is moved or resized. Takes a bit of getting used to.
Comment 2 Robert Hairgrove 2021-11-28 20:02:41 UTC
When running Umbrello as a debug build started from a terminal, after examining the debug output, it seems that the problem here arises from the fact that the default configuration gives us an empty class diagram which can immediately be populated with classes, packages, etc. However, the debug output "view was NULL" on opening the application with no other command-line options seems to indicate that this initial diagram was not yet added to the diagram/view stack ... even though there is a "class_diagram" object present in the tree view? This would certainly explain the fact that there cannot already be a bird's eye view of a view that (officially) does not yet exist.

Is this a configurable option? Why doesn't the inital screen show an emty diagram area (dock widget) instead? I would think that with a new project, where there is no code as yet in existence (and perhaps the programming language is not yet determined), it might make more sense to have either no diagram, or else perhaps a use case diagram, presented to the user.

At any rate, if there is a valid diagram, there should also be a valid bird's eye view.
Comment 3 Robert Hairgrove 2021-12-27 13:53:58 UTC
(In reply to Robert Hairgrove from comment #1)
>(...)  However, there is much more "bugginess" about the bird's eye view (and the
> drawing behavior in general) which needs to be looked at. For example, if I
> have added two new classes to an empty class diagram and move one of them,
> the other moves as well but in the opposite direction! There seems to be
> code somewhere causing the scene to always be centered ... is this by
> design? I would probably have to put widgets at the four corners of the
> virtual page to keep things in the middle area from wandering about on the
> screen when one of them is moved or resized. Takes a bit of getting used to.

In the class UMLScene, there is this slot: 
void UMLScene::slotObjectCreated(UMLObject* o)

At the end of the slot's method body, there is a call to the member function
    resizeSceneToItems();
which is implemented thus:

/**
 * Sets the size of the scene to just fit on all the items
 */
void UMLScene::resizeSceneToItems()
{
    // let QGraphicsScene handle scene size by itself
    setSceneRect(itemsBoundingRect());
}

Why is this necessary? It seems to be the cause of the behavior I described in the above post in this thread. One would think that the scene should begin with a default size (user-configurable, of course) which can expand if moving an object would cross one of the boundaries, but certainly not shrink if it is larger.

What happens is that on creation of the first widget in the scene, the bird's eye view will fill up its available display area with a huge rendition of the widget (much larger than 100%).

Try this:
Open a new class diagram, if one isn't already there;
Create a new class and give it a very short name (such as "A").
The bird's eye will remain blank until the main window is resized or the splitter handle is dragged. Do that.
Observe the resulting display in the bird's eye view.

Is this intentional?
Comment 4 Oliver Kellogg 2021-12-29 21:11:16 UTC
Git commit 9726f9100a564814c2daa77d4b5bd053465d4d26 by Oliver Kellogg.
Committed on 29/12/2021 at 21:10.
Pushed by okellogg into branch 'master'.

Application of patch suggested in https://bugs.kde.org/show_bug.cgi?id=446020#c1 plus minor cleanups

umbrello/umlscene.cpp
- In function addWidgetCmd print debug message showing widget's x and y
  coordinates.
- In function setupNewWidget,
  - remove unneeded parentheses in if-condition testing w->isPinWidget()
    and others;
  - call UMLApp::app()->setCurrentView with activeView() as argument.
- In function dropEvent loop over tidList, on calling setupNewWidget
  provide `true' for argument setPosition.
- At functions selectedCount, setClassWidgetOptions fix minor issues in
  documentation.
- In function resizeSceneToItems, as an experimental change comment out
  call to setSceneRect(itemsBoundingRect()).
  It appears that the function resizeSceneToItems may be removed
  altogether (subject to further tests).

M  +15   -8    umbrello/umlscene.cpp

https://invent.kde.org/sdk/umbrello/commit/9726f9100a564814c2daa77d4b5bd053465d4d26
Comment 5 Robert Hairgrove 2021-12-30 13:09:50 UTC
(In reply to Oliver Kellogg from comment #4)
> (...) - In function resizeSceneToItems, as an experimental change comment out
>   call to setSceneRect(itemsBoundingRect()).
>   It appears that the function resizeSceneToItems may be removed
>   altogether (subject to further tests).

Thanks for looking into this, Oliver. In the meantime, I saw that QGraphicsScene will use itemsBoundingRect() anyway if no scene rect was previously set (in Qt docs). However, the strange drawing behavior of the diagrams when only a few objects/widgets are shown, and by extension of the bird's view, seems to stem from the fact that the scene has no minimum rect with sensible defaults for width and height.

If I add two dummy widgets, one in the upper left corner and the other in the lower right corner, and keep moving them farther apart by scaling the view size down, then I can draw and resize other objects normally which lie inside of the bounding rect determined by the two dummy widgets. Also, the bird's eye view then works as one would expect. Dragging one object in one direction and having others move in the opposite direction is an optical illusion created by the fact that QGraphicssScene will try to keep all objects contained by it centered. Looking at the Qt example code in the "diagramscene" example, the scene's area is set in the constructor  of MainWindow to (0,0,5000,5000) and it never changes.

I don't know if this would work for Umbrello (when calling saveTo/loadFromXMI(), for example), but we could try starting out with a default size of 800 x 1100 pixels (which would correspond roughly to a single A4 page in portrait orientation with 96 DPI) and call resizeSceneToItems() only if the scene needs to grow its size or if the user selects "Fit" from the scale control. Perhaps this could be user-configurable? I recall reading somewhere that it is considered best practice only to include as much detail in a single UML diagram as will fit on a single A4 or US Letter size page... but cannot remember where I saw this.
Comment 6 Oliver Kellogg 2022-02-10 20:50:26 UTC
Git commit edf7206bbc25809bcfd921bb3de98ed652d32f72 by Oliver Kellogg.
Committed on 10/02/2022 at 20:49.
Pushed by okellogg into branch 'master'.

Followup to commit 9726f91,
> It appears that the function resizeSceneToItems may be removed
> altogether (subject to further tests).

umbrello/umlscene.{h,cpp}
- Remove function resizeSceneToItems().

umbrello/umlscene.cpp
- In functions setupNewWidget, slotObjectCreated, applyLayout,
  fileLoaded, saveToXMI, loadFromXMI remove call to
  resizeSceneToItems().

umbrello/birdview.cpp
- In function slotDockSizeChanged remove call to resizeSceneToItems()
  of m_birdView->scene().

umbrello/toolbarstateother.cpp
- In function mouseReleaseEmpty remove call to resizeSceneToItems().

umbrello/uml.cpp
- In function slotZoomFit remove call to resizeSceneToItems().

umbrello/umlview.cpp
- In functions setZoom() and show() remove call to resizeSceneToItems().

umbrello/umlwidgets/associationwidget.cpp
- In function mouseMoveEvent remove call to resizeSceneToItems().

umbrello/umlwidgets/seqlinewidget.cpp
- In function setEndOfLine remove call to resizeSceneToItems().

umbrello/umlwidgets/umlwidget.cpp
- In functions mouseMoveEvent, mouseReleaseEvent, resize() remove call
  to resizeSceneToItems().

M  +0    -1    umbrello/birdview.cpp
M  +0    -3    umbrello/toolbarstateother.cpp
M  +0    -1    umbrello/uml.cpp
M  +0    -15   umbrello/umlscene.cpp
M  +0    -2    umbrello/umlscene.h
M  +2    -5    umbrello/umlview.cpp
M  +0    -1    umbrello/umlwidgets/associationwidget.cpp
M  +0    -1    umbrello/umlwidgets/seqlinewidget.cpp
M  +0    -5    umbrello/umlwidgets/umlwidget.cpp

https://invent.kde.org/sdk/umbrello/commit/edf7206bbc25809bcfd921bb3de98ed652d32f72
Comment 7 Oliver Kellogg 2022-02-11 18:05:27 UTC
Git commit 6da5003e65521bf28aab738c54c2b6d876d78dc5 by Oliver Kellogg.
Committed on 11/02/2022 at 18:05.
Pushed by okellogg into branch 'master'.

https://bugs.kde.org/show_bug.cgi?id=446020#c5

> [...] we could try starting out with a default size of 800 x 1100
> pixels [...] Perhaps this could be user-configurable?

In Diagram Properties page add input fields for changing canvas width
and height:

umbrello/dialogs/pages/diagrampropertiespage.ui
- In QHBoxLayout containing ui_labelZoom and ui_zoom, at right edge,
  - add QLabel ui_labelWidth with text "Width:";
  - add QDoubleSpinBox ui_width;
  - add spacer;
  - add QLabel ui_labelHeight with text "Height:";
  - add QDoubleSpinBox ui_height.

umbrello/dialogs/pages/diagrampropertiespage.cpp
- In constructor set ui_width from scene->width() and set ui_height
  from scene->height().
- In function apply() call m_scene->setSceneRect() with arguments
  w = ui_width->value() and h = ui_height->value().

M  +3    -0    umbrello/dialogs/pages/diagrampropertiespage.cpp
M  +65   -0    umbrello/dialogs/pages/diagrampropertiespage.ui

https://invent.kde.org/sdk/umbrello/commit/6da5003e65521bf28aab738c54c2b6d876d78dc5
Comment 8 Ralf Habacker 2024-03-07 22:44:20 UTC
Created attachment 166642 [details]
Screenshot showing the issue

(In reply to Oliver Kellogg from comment #7)
> In Diagram Properties page add input fields for changing canvas width
> and height:

One problem with this implementation is that if the origin is set to a fixed value, elements in a diagram can also have coordinates < 0 (e.g. https://invent.kde.org/sdk/umbrello/-/blob/master/models/diagrams/sequence/all-elements.xmi), which means that they are no longer visible at certain zoom levels and cannot be selected in the overview screen./master/models/diagrams/sequence/all-elements.xmi
Comment 9 Ralf Habacker 2024-03-07 22:48:37 UTC
(In reply to Ralf Habacker from comment #8)
> Created attachment 166642 [details]
> Screenshot showing the issue

BTW: For better visibility, I have used the scene rectangle as a frame for the grid display. A similar visual debug information can be activated with the umbrello:UMLScene checkbox in the "Debug" window.
Comment 10 Ralf Habacker 2024-03-07 22:55:49 UTC
Created attachment 166643 [details]
Screenshot with UI extension for "Adjust scene size when pressing 'Fit'"

I added a checkbox that allows automatic resizing of the scene via the 'Fit' button - might be a possibility
Comment 11 Oliver Kellogg 2024-03-16 19:50:32 UTC
(In reply to Ralf Habacker from comment #8)
> 
> One problem with this implementation is that if the origin is set to a fixed
> value, elements in a diagram can also have coordinates < 0  [...]

I agree that there is a hole in the current implementation.

See https://bugs.kde.org/show_bug.cgi?id=449622#c3 /
    https://bugs.kde.org/show_bug.cgi?id=449622#c5 :
Perhaps the first pass in loadFromXMI could be extended by detecting negative offsets and determining their maximum (i.e. most negative value), which could then be added to all coordinates in the second pass in order to avoid the negative values.

Of course, more work is needed: To prevent negative coordinates from emerging in the first place, e.g. when moving widgets.
To extend the diagram area at the left or at the the top, all widget coordinates would need to be shifted rightwards or downwards.
It would probably be better to add an extra menu option for adding such space in the diagram, to prevent the user from unintentionally creating huge offsets as described in bug 449622 (IMO the lesson learned from that bug is that a fixed frame of reference is needed).