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

Don't assume all flat polygons face up #60895

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

Conversation

uclaros
Copy link
Contributor

@uclaros uclaros commented Mar 7, 2025

Description

QgsTessellator uses a shortcut and considers all 2D polygons and 3D flat (all vertices having the same Z) polygons to be facing up, returning a positive normal and counter clockwise triangles, regardless of the original geometry's winding order. This was also unaffected if the invertNormals parameter was set to true, leading to more confusion.

With this PR, the flat polygons' winding order is taken into account, producing counter clockwise triangles and positive normals for counter clockwise polygons, and clockwise triangles and negative normals for clockwise polygons.
If invertNormals == true then both the normals and the triangle winding order is reversed.

Edit: However, there are polygons that are not made to be rendered in 3d but rather are just used as footprints for extrusion. Their data provider might even force a winding order, like the buildings shapefile in the test suite. So when we have flat polygons that are extruded, we always treat them as up-facing, regardless of their winding order or the invertNormals parameter.

Fixes: #52832

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

github-actions bot commented Mar 7, 2025

🪟 Windows builds

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

🪟 Windows Qt6 builds

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

@qgis qgis deleted a comment from github-actions bot Mar 11, 2025
@@ -948,7 +948,7 @@ def testPolygonGenerationTerrain(self):
self.round_dict(results.distanceToHeightMap(), 1),
{
1041.8: 55.3,
1042.4: 55.2,
1042.4: 55.3,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you explain why these have changed? The tesselator used in QgsVectorLayerProfileGenerator is set not to calculate normals...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The normals are always calculated and used to define the winding order of the resulting triangles. The tessellator addNormals parameter sets whether the normals will be included in the result data, which in this case are not needed.
Still, I'm not exactly sure why the tests needed adjustment. It seems there were some rounding errors caused by the triangles being calculated in the opposite orientation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3D view - completely flat MultiPolygonZ geometries don't show color correctly
3 participants