Skip to content

Comments

[3D] Clipping planes tool#61057

Merged
wonder-sk merged 13 commits intoqgis:masterfrom
Withalion:clipping-planes-tool
Mar 29, 2025
Merged

[3D] Clipping planes tool#61057
wonder-sk merged 13 commits intoqgis:masterfrom
Withalion:clipping-planes-tool

Conversation

@Withalion
Copy link
Contributor

@Withalion Withalion commented Mar 17, 2025

Description

This PR adds a new tool to create cross sections in 3D scene. User enables this tool in 3D window, selects the area on 2D canvas and after confirmation the 3D scene will be edited + the camera will change to profile view.
Left click -> adds point
BACKSPACE or DEL -> remove last step in selection
Right click or ESCAPE -> resets the selection

Screencast_20250320_005829.mp4

Funded by: Septima, Klimadatastyrelsen

@github-actions github-actions bot added this to the 3.44.0 milestone Mar 17, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Mar 17, 2025

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit bb14eab)

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit bb14eab)

@nyalldawson
Copy link
Collaborator

Why does this require multiple attempts in the screencast?

@Withalion
Copy link
Contributor Author

I see that I won't be a youtuber... I was trying to show that users can undo/remove the selection

Copy link
Member

@wonder-sk wonder-sk left a comment

Choose a reason for hiding this comment

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

Clipping in 3D - yay! 🎉

@nyalldawson
Copy link
Collaborator

@Withalion

I see that I won't be a youtuber... I was trying to show that users can undo/remove the selection

Ah, makes sense! Can you remove the incorrect warning about "no vector layer selected" which is shown when undoing?

@ptitjano ptitjano added 3D Relates to QGIS' 3D engine or rendering Feature labels Mar 21, 2025
@ptitjano
Copy link
Collaborator

  1. I create a new 3D scene. I define clipping planes in the scene. It works. Then I change the 3D scene extent with "Set 3D extent on 2D map view" and select a new extent outside of the clipping planes. I end up with an empty view.

Yes, you are correct

In that case, could you add a comment explaining this situation in the relevant part of the code please?

@Withalion
Copy link
Contributor Author

Withalion commented Mar 27, 2025

It's not necessary as the PR with fix will come soon. (today/tomorrow)

Also I'll try to fix up this PR today so we can move forward.
The PR should be ready and I also implemented fix for the bug you had

@Withalion Withalion force-pushed the clipping-planes-tool branch from 900d711 to 22a84bf Compare March 27, 2025 14:58
{
clear();
QgsMessageBar *msgBar = QgisApp::instance()->messageBar();
msgBar->pushInfo( QString(), tr( "The cross section is outside of the scene extent, please select a new one!" ) );
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick. No need for a of here: The cross section is outside the scene extent, please select a new one!

@ptitjano
Copy link
Collaborator

Thanks @Withalion I did more testing and now I cannot define a cross-sections outside the scene.

Regarding my other issue, I'm afraid I haven't understood your answer. If the cross-sections are already active and I change the 3D scene extent outside the cross-sections extent, I end up with an empty view. Is it supposed to work and it won't be covered by this PR?

@Withalion
Copy link
Contributor Author

Anything related to changing extents, which also means moving the origin of the scene, is not covered by this PR. It will be covered by another one. Take this as a base PR for the clipping tool and we will hammer out any bugs with next PRs.

@ptitjano
Copy link
Collaborator

Anything related to changing extents, which also means moving the origin of the scene, is not covered by this PR. It will be covered by another one. Take this as a base PR for the clipping tool and we will hammer out any bugs with next PRs.

In that case, update this PR once the handling of the origin of the scene is ready. This will ensure to merge a version without known issues or limitation.

@wonder-sk
Copy link
Member

Handling of origin rebasing added in bb14eab

@wonder-sk wonder-sk merged commit ad32431 into qgis:master Mar 29, 2025
31 checks passed
@ptitjano
Copy link
Collaborator

@Withalion @wonder-sk I have just tried an up to date QGIS master and I can still reproduce the issue previously mentioned.

Capture.video.du.2025-03-31.11-32-03.mp4

Whether or not this issue would have been fixed, I also want to point out that I find it somewhat disrespectful to merge a PR without asking for my opinion, especially since I spent a lot of time reviewing and testing it.

@wonder-sk
Copy link
Member

Whether or not this issue would have been fixed, I also want to point out that I find it somewhat disrespectful to merge a PR without asking for my opinion, especially since I spent a lot of time reviewing and testing it.

@ptitjano Understood. We didn't intend any disrespect, and apologize if it came across that way. We felt the PR was ready and didn't think you had further feedback, but we should have checked in. We'll be more mindful in the future.

I am sure @Withalion will check your bug and fix if it can be replicated.

@zacharlie zacharlie added the Changelog Items that are queued to appear in the visual changelog - remove after harvesting label May 7, 2025
@qgis-bot
Copy link
Collaborator

qgis-bot commented May 7, 2025

@Withalion

This pull request has been tagged for the changelog.

  • The description will be harvested so please provide a "nearly-ready" text for the final changelog
  • If possible, add a nice illustration of the feature. Only the first one in the description will be harvested (GIF accepted as well)
  • If you can, it's better to give credits to your sponsor, see below for different formats.

You can edit the description.

Format available for credits
  • Funded by NAME
  • Funded by URL
  • Funded by NAME URL
  • Sponsored by NAME
  • Sponsored by URL
  • Sponsored by NAME URL

Thank you!

@zacharlie zacharlie added ChangelogHarvested This PR description has been harvested in the Changelog already. and removed Changelog Items that are queued to appear in the visual changelog - remove after harvesting labels May 7, 2025
@DelazJ DelazJ added the Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. label Jun 17, 2025
@qgis-bot
Copy link
Collaborator

@Withalion
A documentation ticket has been opened at qgis/QGIS-Documentation#9970
It is your responsibility to visit this ticket and add as much detail as possible for the documentation team to correctly document this change.
Thank you!

@qgis-bot
Copy link
Collaborator

@Withalion
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3D Relates to QGIS' 3D engine or rendering ChangelogHarvested This PR description has been harvested in the Changelog already. Feature Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants