Skip to content

Comments

[Backport release-3_42] qgsmaterial: Fix removal of parameters when clipping is disabled#61147

Closed
qgis-bot wants to merge 5 commits intorelease-3_42from
backport-61135-to-release-3_42
Closed

[Backport release-3_42] qgsmaterial: Fix removal of parameters when clipping is disabled#61147
qgis-bot wants to merge 5 commits intorelease-3_42from
backport-61135-to-release-3_42

Conversation

@qgis-bot
Copy link
Collaborator

@qgis-bot qgis-bot commented Mar 23, 2025

Backport #61135
Authored by: @ptitjano

Withalion and others added 5 commits March 23, 2025 21:33
@Withalion @github-actions
qgsmaterial: Fix removal of parameters when clipping is disabled
653ead8 
Plane clipping uses two `QParameters` named
`CLIP_PLANE_ARRAY_PARAMETER_NAME` and
`CLIP_PLANE_MAX_PLANE_PARAMETER_NAME`. When clipping is disabled both
parameters need to be removed to prevent side effects on the shader
code. However, the loops which iterates through the parameters breaks
as soon one parameter is found. This means that the second parameter
is never removed.

This issue is fixed by removing the `break` condition. This way, both
parameters are always removed from the material.
@ptitjano @github-actions
testqgs3dmaterial: Add tests for clipping
de00b52
@ptitjano @github-actions
testqgs3drendering: Disable opacity for the polygons clipping test
636b89e 
This will make it easier to test enabling/disabling clipping multiple
times with the other reference image.
@ptitjano @github-actions
testqgs3drendering: Improve coverage for plane clipping
d48bcde 
This tests that enabling and disabling clipping multiple times works.
@ptitjano @github-actions
testqgspointcloud3drendering: Improve coverage for plane clipping
8456967 
This tests that enabling and disabling clipping multiple times works.
@github-actions github-actions bot added this to the 3.42.2 milestone Mar 23, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2025 •
edited
Loading

🪟 Windows builds

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

🪟 Windows Qt6 builds

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

@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2025 •
edited
Loading

Tests failed for Qt 6

One or more tests failed using the build from commit 8456967

polygon3d_extrusion_clipping (testExtrudedPolygonsClipping)

polygon3d_extrusion_clipping

Test failed at testExtrudedPolygonsClipping at tests/src/3d/testqgs3drendering.cpp:506

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

polygon3d_extrusion_data_defined_phong_clipping (testExtrudedPolygonsDataDefinedPhongClipping)

polygon3d_extrusion_data_defined_phong_clipping

Test failed at testExtrudedPolygonsDataDefinedPhongClipping at tests/src/3d/testqgs3drendering.cpp:747

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

polygon3d_extrusion_data_defined_gooch_clipping (testExtrudedPolygonsDataDefinedGoochClipping)

polygon3d_extrusion_data_defined_gooch_clipping

Test failed at testExtrudedPolygonsDataDefinedGoochClipping at tests/src/3d/testqgs3drendering.cpp:890

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

line_rendering_clipping (testLineRenderingClipping)

line_rendering_clipping

Test failed at testLineRenderingClipping at tests/src/3d/testqgs3drendering.cpp:1172

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

buffered_lines_clipping (testBufferedLineRenderingClipping)

buffered_lines_clipping

Test failed at testBufferedLineRenderingClipping at tests/src/3d/testqgs3drendering.cpp:1407

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

pointcloud_3d_singlecolor_clipping (testPointCloudSingleColorClipping)

pointcloud_3d_singlecolor_clipping

Test failed at testPointCloudSingleColorClipping at tests/src/3d/testqgspointcloud3drendering.cpp:327

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.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2025 •
edited
Loading

Tests failed for Qt 5

One or more tests failed using the build from commit 8456967

polygon3d_extrusion_clipping (testExtrudedPolygonsClipping)

polygon3d_extrusion_clipping

Test failed at testExtrudedPolygonsClipping at tests/src/3d/testqgs3drendering.cpp:506

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

polygon3d_extrusion_data_defined_phong_clipping (testExtrudedPolygonsDataDefinedPhongClipping)

polygon3d_extrusion_data_defined_phong_clipping

Test failed at testExtrudedPolygonsDataDefinedPhongClipping at tests/src/3d/testqgs3drendering.cpp:747

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

polygon3d_extrusion_data_defined_gooch_clipping (testExtrudedPolygonsDataDefinedGoochClipping)

polygon3d_extrusion_data_defined_gooch_clipping

Test failed at testExtrudedPolygonsDataDefinedGoochClipping at tests/src/3d/testqgs3drendering.cpp:890

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

line_rendering_clipping (testLineRenderingClipping)

line_rendering_clipping

Test failed at testLineRenderingClipping at tests/src/3d/testqgs3drendering.cpp:1172

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

buffered_lines_clipping (testBufferedLineRenderingClipping)

buffered_lines_clipping

Test failed at testBufferedLineRenderingClipping at tests/src/3d/testqgs3drendering.cpp:1407

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

pointcloud_3d_singlecolor_clipping (testPointCloudSingleColorClipping)

pointcloud_3d_singlecolor_clipping

Test failed at testPointCloudSingleColorClipping at tests/src/3d/testqgspointcloud3drendering.cpp:327

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.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2025

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Apr 7, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 14, 2025

While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 21 days. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the QGIS project can do to help push this PR forward please let us know how we can assist.

@github-actions github-actions bot closed this Apr 14, 2025
@ptitjano ptitjano reopened this Apr 14, 2025
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Apr 14, 2025
@nyalldawson
Copy link
Collaborator

nyalldawson commented Apr 29, 2025

@ptitjano can you update this to fix the tests or close please?

@github-actions
Copy link
Contributor

github-actions bot commented May 14, 2025

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label May 14, 2025
@agiudiceandrea agiudiceandrea deleted the backport-61135-to-release-3_42 branch January 24, 2026 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nyalldawson nyalldawson nyalldawson approved these changes

Assignees

No one assigned

Labels

stale Uh oh! Seems this work is abandoned, and the PR is about to close.

Projects

None yet

Milestone

3.42.2

Development

Successfully merging this pull request may close these issues.

4 participants

@qgis-bot @nyalldawson @ptitjano @Withalion