Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge ign-gui6 ➡️ main #446

Merged
merged 46 commits into from
Aug 4, 2022
Merged

Merge ign-gui6 ➡️ main #446

merged 46 commits into from
Aug 4, 2022

Conversation

chapulina
Copy link
Contributor

➡️ Forward port

Port ign-gui6 ➡️ main

Branch comparision: main...ign-gui6

Note to maintainers: Remember to Merge with commit (not squash-merge or rebase)

mabelzhang and others added 30 commits April 14, 2022 09:20
Override the org level issue templates in order to make the issue templates more specific to this repo.

Signed-off-by: Addisu Z. Taddese <[email protected]>
* allow SDF FOV tag

Signed-off-by: youhy <[email protected]>
* search bar text highlight

* add search bar shortcut "/"

Signed-off-by: youhy <[email protected]>

Co-authored-by: Jenn Nguyen <[email protected]>
* search bar text highlight

* add search bar shortcut "/"

Signed-off-by: youhy <[email protected]>

Co-authored-by: Jenn Nguyen <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>

Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
* common widget GzColor

* implement Grid3D with the common widget

Signed-off-by: youhy <[email protected]>

Co-authored-by: Jenn Nguyen <[email protected]>
* Example running a dialog before the main window

Signed-off-by: Louise Poubel <[email protected]>

* Revert FIXMEs

Signed-off-by: Louise Poubel <[email protected]>
* Add common widget pose GUI

Signed-off-by: youhy <[email protected]>

Co-authored-by: Jenn Nguyen <[email protected]>
* common widget variables fix

* remove spacer

* change show to expand

Signed-off-by: youhy <[email protected]>
Signed-off-by: Mohamad <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>

Co-authored-by: Louise Poubel <[email protected]>
Signed-off-by: youhy <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
Co-authored-by: Jenn Nguyen <[email protected]>
Signed-off-by: Jenn Nguyen <[email protected]>
Signed-off-by: youhy <[email protected]>
Forward-port 3 to 6
Signed-off-by: Jenn Nguyen <[email protected]>
Signed-off-by: Jenn Nguyen <[email protected]>
Signed-off-by: Jenn Nguyen <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>

Co-authored-by: Nate Koenig <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
@chapulina
Copy link
Contributor Author

I found a couple more renaming places. Do we need to move include/ignition/gui/* to include/gz/gui? Also, are we renaming IgnHelpers.qml to GzHelpers.qml?

Nice catch, I'll look into that soon

@chapulina chapulina marked this pull request as draft July 29, 2022 20:20
chapulina added a commit that referenced this pull request Aug 1, 2022
Signed-off-by: Louise Poubel <[email protected]>
@chapulina
Copy link
Contributor Author

Do we need to move include/ignition/gui/* to include/gz/gui? Also, are we renaming IgnHelpers.qml to GzHelpers.qml?

Ahh I forgot that those were duplicated. Fixed in 593ccb0. I'd like to revisit that eventually and see if we can get away without duplicated files.

@chapulina chapulina marked this pull request as ready for review August 1, 2022 17:07
Copy link
Contributor

@jennuine jennuine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these aliases still needed with 593ccb0?:

<file alias="qml/IgnCard.qml">qml/GzCard.qml</file>
<file alias="qml/IgnCardSettings.qml">qml/GzCardSettings.qml</file>
<file alias="qml/IgnHelpers.qml">qml/GzHelpers.qml</file>
<file alias="qml/IgnRulers.qml">qml/GzRulers.qml</file>
<file alias="qml/IgnSnackBar.qml">qml/GzSnackBar.qml</file>
<file alias="qml/IgnSortFilterModel.qml">qml/GzSortFilterModel.qml</file>
<file alias="qml/IgnSpinBox.qml">qml/GzSpinBox.qml</file>
<file alias="qml/IgnSplit.qml">qml/GzSplit.qml</file>
<file alias="IgnSnackBar.qml">qml/GzSnackBar.qml</file>
<file alias="IgnSpinBox.qml">qml/GzSpinBox.qml</file>

include/gz/gui/resources.qrc Outdated Show resolved Hide resolved
jennuine and others added 6 commits August 2, 2022 09:05
Signed-off-by: Jenn Nguyen <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
chapulina added a commit that referenced this pull request Aug 2, 2022
Signed-off-by: Louise Poubel <[email protected]>
chapulina added a commit that referenced this pull request Aug 2, 2022
Signed-off-by: Louise Poubel <[email protected]>
@chapulina
Copy link
Contributor Author

I can reproduce the HFOV test failure locally, but I haven't figured out how to fix it:

   /github/workspace/test/integration/minimal_scene.cc:158: Failure
  The difference between 60 and camera->HFOV().Degree() is 15.009121067327513, which exceeds 1e-4, where
  60 evaluates to 60,
  camera->HFOV().Degree() evaluates to 75.009121067327513, and
  1e-4 evaluates to 0.0001.

It looks like the aspect ratio is changing in the middle of the test, so the HFOV changes. @AzulRadio @jennuine , any tips?

@jennuine
Copy link
Contributor

jennuine commented Aug 4, 2022

Currently, I think there is a bug in gz-rendering. This line here seems to be the source of the issue:

this->dataPtr->camera->PreRender();

Looking at camera->HFOV() before the camera->PreRender() call, the HFOV() returns 60 degrees. After camera->PreRender() it returns 75.0091 degrees, which causes the test to fail. I'll take a closer look at gz-rendering tomorrow

PS. This issue doesn't happen on fortress (after I set textureDirty = true to match main)

@chapulina
Copy link
Contributor Author

Looking at camera->HFOV() before the camera->PreRender() call, the HFOV() returns 60 degrees. After camera->PreRender() it returns 75.0091 degrees

Thanks for the lead, @jennuine ! I believe the issue is that we were setting the aspect ratio every time the texture is dirty, but now that we want to keep a constant FOV, we should be setting that instead. Also since gazebosim/gz-rendering#635 the aspect ratio is updated automatically with the SetImage[Height|Width] calls.

See the fix in c7cac24 and let me know if you're ok with it. The test passes now, and I checked locally that resizing a window behaves correctly.

Copy link
Contributor

@jennuine jennuine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the fix in c7cac24 and let me know if you're ok with it

LGTM!

@jennuine
Copy link
Contributor

jennuine commented Aug 4, 2022

Are we still deprecating gz-sim scene3d? If not, we should update it too (and update the deprecated warning version numbers)l: https://github.com/gazebosim/gz-sim/blob/ff101099698da7795fe25f9c03b0bc34a7a25f5e/src/gui/plugins/scene3d/Scene3D.cc#L688-L689

@chapulina
Copy link
Contributor Author

Are we still deprecating gz-sim scene3d?

Yup, that's being removed in

@chapulina chapulina merged commit b5f354a into main Aug 4, 2022
@chapulina chapulina deleted the chapulina/6_to_7 branch August 4, 2022 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants