Use SplitView from QtQuick.Controls 2 for GzSplit #651

azeey · 2025-01-29T00:33:46Z

🎉 New feature

Toward #586

Summary

The SplitView from QtQuick.Controls 1 has been deprecated for a while and has been removed from Qt6. This change is a stepping stone for migrating to Qt6.

The new SplitView is significantly different from the previous one and so major changes had to be made. The original GzSplit component was also designed with more capabilities in mind for the future, which complicated the code. This has been simplified:

Instead of determining whether to add a new split for each card added, two splits are hard coded: (1) the main view port (2) the side panel. For every new card added, the code simply checks adds the card to the main view port if its empty. Otherwise, it puts it in the side panel.
Animations have been removed. It was difficult to get consistent behavior with different split sizes.
Card items are returned instead of their names when calling addCard. This simplifies the code that searches for items by name
Logic to detect if cards have been collapsed manually by resizing the card has been removed. It's not clear if it is even possible to do this with any of the cards/plugins in Gazebo since the minimumHeight constraint would prevent resizing the cards that small.

New SplitView

Old SplitView

Test it

gz gui -c examples/config/layout.config
gz sim camera_sensor.sdf

Checklist

Signed all commits for DCO
Added tests
Updated documentation (as needed)
Updated migration guide (as needed)
Consider updating Python bindings (if the library has them)
codecheck passed (See contributing)
All tests passed (See test coverage)
While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

The one from QtQuick.Controls 1 has been deprecated for a while and has been removed from Qt6. This change is a stepping stone for migrating to Qt6. The new SplitView is significantly different from the previous one and so major changes had to be made. The original GzSplit component was also designed with more capabilities in mind for the future, which complicated the code. This has been simplified: * Instead of determining whether to add a new split for each card added, two splits are hard coded: (1) the main view port (2) the side panel. For every new card added, the code simply checks adds the card to the main view port if its empty. Otherwise, it puts it in the side panel. * Animations have been removed. It was difficult to get consistent behavior with different split sizes. * Card items are returned instead of their names when calling `addCard`. This simplifies the code that searches for items by name * Logic to detect if cards have been collapsed manually by resizing the card has been removed. It's not clear if it is even possible to do this with any of the cards/plugins in Gazebo since the `minimumHeight` constraint would prevent resizing the cards that small. Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

iche033

Nice! works great for me. Not sure if it's possible to make the split view handle thinner or change its color to gray but those are minor details that can come later.

iche033 · 2025-01-30T01:51:07Z

src/Application.cc


    // Add card to main window
-    cardItem->setParentItem(splitItem);
+    // gzdbg << "Adding to:" << splitItem.toString().toStdString() << std::endl;


iche033 · 2025-01-30T02:14:24Z

include/gz/gui/qml/GzCard.qml

  /**
   * Minimum width of the card pane
   */
-  property int cardMinimumWidth: 250;
+  property alias cardMinimumWidth: content.minimumWidth;


I think this we need to update the test for this change:

/github/workspace/src/Plugin_TEST.cc:304: Failure Expected equality of these values: propElem->Attribute("type") Which is: "double" pluginTypes[propElem->Attribute("key")] Which is: "int"

Fixed in 2570a75

I opted to ignore card* properties from being parsed from XML since that wasn't working anyway. Looking at the implementation, I don't think they were intended to be exposed since they are updated internally to collapse/expand the card.

The MainWindow_TEST was failing because the requested size of the window was below the minimum allowed size. Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

azeey · 2025-01-31T23:45:24Z

Nice! works great for me. Not sure if it's possible to make the split view handle thinner or change its color to gray but those are minor details that can come later.

It's possible to change the color and make it thinner, but it causes the mouse area to become thinner as well, which makes it really hard to use. The previous version had a wider mouse area with a thin (1px wide) rectangle display. With the SplitView in QtQuick.Control2, it's not possible to control the mouse area and the displayed rectangle independently.

2570a75 also fixes a failing test in UNIT_MainWindow_TEST. The test was requesting to resize the window to a size below the minimum size set in

gz-gui/include/gz/gui/qml/Main.qml

Lines 30 to 31 in 2570a75

    
           minimumWidth: 300 
        
           minimumHeight: 300

, so I'm not sure how it was working in the first place.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

iche033 · 2025-02-04T01:51:22Z

the noble build picked up some weird warnings:

[GNU C Compiler (gcc)] [-ERROR-] - '/home/jenkins/workspace/gz_gui-ci-pr_any-noble-amd64/build/file:/usr/lib/x86_64-linux-gnu/qt5/qml/QtQuick/Controls.2/Material/SplitView.qml' file not found

azeey · 2025-02-04T16:17:04Z

the noble build picked up some weird warnings:

Not sure wht's going on. I can't reproduce this locally. I'm going to try to retrigger the job to see if it happens consistently.

azeey · 2025-02-04T18:03:33Z

It's possible to change the color and make it thinner, but it causes the mouse area to become thinner as well, which makes it really hard to use.

There's a bug reported on this (https://bugreports.qt.io/browse/QTBUG-122929), and there's a workaround in qt6, so will take a look again after we migrate.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

codecov · 2025-02-04T19:14:44Z

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 68.70%. Comparing base (8edf24d) to head (c5ea53f).
Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
src/Plugin.cc	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #651      +/-   ##
==========================================
+ Coverage   68.49%   68.70%   +0.21%     
==========================================
  Files          38       38              
  Lines        5567     5532      -35     
==========================================
- Hits         3813     3801      -12     
+ Misses       1754     1731      -23

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

azeey · 2025-02-04T19:17:27Z

the noble build picked up some weird warnings:

Fixed in c5ea53f

azeey requested a review from jennuine as a code owner January 29, 2025 00:33

github-actions bot added the 🪵 jetty Gazebo Jetty label Jan 29, 2025

iche033 reviewed Jan 30, 2025

View reviewed changes

Fix tests

2570a75

The MainWindow_TEST was failing because the requested size of the window was below the minimum allowed size. Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

Revert type change

e5e03a6

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

azeey added 2 commits February 4, 2025 12:15

Try hardcoding handle size

cdf6b4d

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

Provide a parent object to avoid warnings

c5ea53f

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

iche033 approved these changes Feb 5, 2025

View reviewed changes

azeey merged commit daf1a6c into gazebosim:main Feb 5, 2025
12 checks passed

azeey deleted the splitview2 branch February 5, 2025 20:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use SplitView from QtQuick.Controls 2 for GzSplit #651

Use SplitView from QtQuick.Controls 2 for GzSplit #651

azeey commented Jan 29, 2025

iche033 left a comment

iche033 Jan 30, 2025

azeey Jan 31, 2025

iche033 Jan 30, 2025

azeey Jan 31, 2025

azeey commented Jan 31, 2025

iche033 commented Feb 4, 2025

azeey commented Feb 4, 2025

azeey commented Feb 4, 2025

codecov bot commented Feb 4, 2025 •

edited

Loading

azeey commented Feb 4, 2025

Use SplitView from QtQuick.Controls 2 for GzSplit #651

Use SplitView from QtQuick.Controls 2 for GzSplit #651

Conversation

azeey commented Jan 29, 2025

🎉 New feature

Summary

Test it

Checklist

iche033 left a comment

Choose a reason for hiding this comment

iche033 Jan 30, 2025

Choose a reason for hiding this comment

azeey Jan 31, 2025

Choose a reason for hiding this comment

iche033 Jan 30, 2025

Choose a reason for hiding this comment

azeey Jan 31, 2025

Choose a reason for hiding this comment

azeey commented Jan 31, 2025

iche033 commented Feb 4, 2025

azeey commented Feb 4, 2025

azeey commented Feb 4, 2025

codecov bot commented Feb 4, 2025 • edited Loading

Codecov Report

azeey commented Feb 4, 2025

codecov bot commented Feb 4, 2025 •

edited

Loading