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.

Sign up for GitHub

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

[3D] Fix reset view button #60413

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[3D] Fix reset view button #60413

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3118643
Fix reset view in 3D view for VPCs
Withalion Feb 3, 2025
16cf245
Refactor camera change flow
Withalion Feb 4, 2025
0995c8d
Update tests
Withalion Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/3d/qgs3dmapscene.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ void Qgs3DMapScene::viewZoomFull()
double d = side / 2 / std::tan( cameraController()->camera()->fieldOfView() / 2 * M_PI / 180 );
d += zRange.isInfinite() ? 0. : zRange.upper();
mCameraController->resetView( static_cast<float>( d ) );
return;
}

void Qgs3DMapScene::setViewFrom2DExtent( const QgsRectangle &extent )
Expand Down
8 changes: 3 additions & 5 deletions src/3d/qgscameracontroller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,9 @@ void QgsCameraController::setViewFromTop( float worldX, float worldY, float dist
camPose.setDistanceFromCenterPoint( distance );
camPose.setHeadingAngle( yaw );

// a basic setup to make frustum depth range long enough that it does not cull everything
mCamera->setNearPlane( distance / 2 );
mCamera->setFarPlane( distance * 2 );

setCameraPose( camPose );
// we force the updateCameraNearFarPlanes() in Qgs3DMapScene to properly set the planes
mCameraPose = camPose;
updateCameraFromPose();
Comment on lines +183 to +184
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not calling setCameraPose( camPose ); and add a call to updateCameraFromPose() in it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

setCameraPose( camPose ) already calls updateCameraFromPose(), however the issue was that the setCameraPose( camPose ) would return early as camPose was the same and the near & far plane would not be recalculated according to layers

}

QgsVector3D QgsCameraController::lookingAtPoint() const
Expand Down
24 changes: 21 additions & 3 deletions src/core/pointcloud/qgspointcloudlayerelevationproperties.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "qgsapplication.h"
#include "qgscolorschemeregistry.h"
#include "qgscolorutils.h"
#include "qgsvirtualpointcloudprovider.h"

QgsPointCloudLayerElevationProperties::QgsPointCloudLayerElevationProperties( QObject *parent )
: QgsMapLayerElevationProperties( parent )
Expand Down Expand Up @@ -127,11 +128,28 @@ QgsDoubleRange QgsPointCloudLayerElevationProperties::calculateZRange( QgsMapLay
{
if ( pcLayer->dataProvider() )
{
double zMin = std::numeric_limits<double>::quiet_NaN();
double zMax = std::numeric_limits<double>::quiet_NaN();
const QgsPointCloudStatistics stats = pcLayer->statistics();
if ( !stats.statisticsMap().isEmpty() )
{
// try to fetch z range from provider metadata
zMin = stats.minimum( QStringLiteral( "Z" ) );
zMax = stats.maximum( QStringLiteral( "Z" ) );
}
// try to fetch the elevation properties from virtual point cloud metadata
else if ( QgsVirtualPointCloudProvider *virtualProvider = dynamic_cast< QgsVirtualPointCloudProvider * >( pcLayer->dataProvider() ) )
{
zMin = virtualProvider->subIndexes()[0].zRange().lower();
zMax = virtualProvider->subIndexes()[0].zRange().upper();
Comment on lines +143 to +144
Copy link
Contributor

Choose a reason for hiding this comment

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

Why check against [0] first? all sub indexes will be checked in the iteration below.
([0] could also be out of range for some weird bad vpc file!)

for ( QgsPointCloudSubIndex subIndex : virtualProvider->subIndexes() )
{
const QgsDoubleRange newRange = subIndex.zRange();
Copy link
Contributor

Choose a reason for hiding this comment

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

While not very probable, subIndex.zRange() may be also a default constructed infinite range if the vpc has 4 coordinate instead of 6 coordinate bboxes.
These infinite range subindexes should be skipped or the zMin/zMax doubles will overflow after the multiplication later on.

zMin = std::min( zMin, newRange.lower() );
zMax = std::max( zMax, newRange.upper() );
}
}

// try to fetch z range from provider metadata
const double zMin = stats.minimum( QStringLiteral( "Z" ) );
const double zMax = stats.maximum( QStringLiteral( "Z" ) );
if ( !std::isnan( zMin ) && !std::isnan( zMax ) )
{
return QgsDoubleRange( zMin * mZScale + mZOffset, zMax * mZScale + mZOffset );
Expand Down