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

3d/elevation profile: Use absolute clamping for 3d data #60186

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

Conversation

ptitjano
Copy link
Contributor

@ptitjano ptitjano commented Jan 20, 2025

Description

This is a second attempt based on #59759 which was auto closed because of the lack of reaction

When providing a project DTM/DEM, the default behavior for vector feature with a z-component is to offset the $z coordinates from the terrain elevation model in the elevation profile view. However, the geometry coordinates are supposed to be absolute.

So it makes more sense, to only use the $z coordinate by default in that case.

See: #59757

cc @T4mmi

@ptitjano ptitjano self-assigned this Jan 20, 2025
@ptitjano ptitjano added the 3D Relates to QGIS' 3D engine or rendering label Jan 20, 2025
@github-actions github-actions bot added this to the 3.42.0 milestone Jan 20, 2025
Copy link

github-actions bot commented Jan 20, 2025

🪟 Windows builds

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

🪟 Windows Qt6 builds

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

Copy link

Tests failed for Qt 6

One or more tests failed using the build from commit a38e99c

ambient_occlusion_1 (testAmbientOcclusion)

ambient_occlusion_1

Test failed at testAmbientOcclusion at tests/src/3d/testqgs3drendering.cpp:2024

Rendered image did not match tests/testdata/control_images/3d/expected_ambient_occlusion_1/expected_ambient_occlusion_1.png (found 100023 pixels different)

The full test report (included comparison of rendered vs expected images) can be found here.

Further documentation on the QGIS test infrastructure can be found in the Developer's Guide.

Copy link

Tests failed for Qt 5

One or more tests failed using the build from commit a38e99c

ambient_occlusion_1 (testAmbientOcclusion)

ambient_occlusion_1

Test failed at testAmbientOcclusion at tests/src/3d/testqgs3drendering.cpp:2024

Rendered image did not match tests/testdata/control_images/3d/expected_ambient_occlusion_1/expected_ambient_occlusion_1.png (found 100023 pixels different)

The full test report (included comparison of rendered vs expected images) can be found here.

Further documentation on the QGIS test infrastructure can be found in the Developer's Guide.

@ptitjano ptitjano force-pushed the profile-absolute branch 2 times, most recently from bf04f42 to 46d5638 Compare January 20, 2025 12:59
T4mmi added 2 commits January 20, 2025 15:46
When providing a project DTM/DEM, the default behavior for vector
feature with a z-component is to offset the $z coordinates from the
terrain elevation model in the elevation profile view. However, the
geometry coordinates are supposed to be absolute.

So it makes more sense, to only use the $z coordinate by default in
that case.

Related: qgis#59757
When providing a project DTM/DEM, the default behavior for vector
feature with a z-component is to offset the $z coordinates from the
terrain elevation model in the elevation profile view. However, the
geometry coordinates are supposed to be absolute.

So it makes more sense, to only use the $z coordinate by default in
that case.

The different unit tests are updated accordingly.

Closes: qgis#59757
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants