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

Don't assume all flat polygons face up #60895

Merged
merged 3 commits into from
Mar 12, 2025
+153 −29
Merged

Don't assume all flat polygons face up #60895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8f58808
Don't assume all flat polygons face up
uclaros Mar 6, 2025
4918a1b
Treat flat polygons as always facing up when extruding
uclaros Mar 9, 2025
944c08d
Adjust profile tests
uclaros Mar 10, 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
28 changes: 18 additions & 10 deletions src/core/qgstessellator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,16 +256,25 @@ static void _makeWalls( const QgsLineString &ring, bool ccw, float extrusionHeig
}
}

static QVector3D _calculateNormal( const QgsLineString *curve, double originX, double originY, bool invertNormal )
static QVector3D _calculateNormal( const QgsLineString *curve, double originX, double originY, bool invertNormal, float extrusionHeight )
{
// if it is just plain 2D curve there is no need to calculate anything
// because it will be a flat horizontally oriented patch
if ( !QgsWkbTypes::hasZ( curve->wkbType() ) || curve->isEmpty() )
return QVector3D( 0, 0, 1 );
if ( !QgsWkbTypes::hasZ( curve->wkbType() ) )
{
// In case of extrusions, flat polygons always face up
if ( extrusionHeight != 0 )
return QVector3D( 0, 0, 1 );

// For non-extrusions, decide based on polygon winding order and invertNormal flag
float orientation = 1.f;
if ( curve->orientation() == Qgis::AngularDirection::Clockwise )
orientation = -orientation;
if ( invertNormal )
orientation = -orientation;
return QVector3D( 0, 0, orientation );
}

// often we have 3D coordinates, but Z is the same for all vertices
// so in order to save calculation and avoid possible issues with order of vertices
// (the calculation below may decide that a polygon faces downwards)
// if these flat polygons are extruded, we consider them up-facing regardless of winding order
bool sameZ = true;
QgsPoint pt1 = curve->pointN( 0 );
QgsPoint pt2;
Expand All @@ -278,7 +287,7 @@ static QVector3D _calculateNormal( const QgsLineString *curve, double originX, d
break;
}
}
if ( sameZ )
if ( sameZ && extrusionHeight != 0 )
return QVector3D( 0, 0, 1 );

// Calculate the polygon's normal vector, based on Newell's method
Expand All @@ -287,7 +296,6 @@ static QVector3D _calculateNormal( const QgsLineString *curve, double originX, d
// Order of vertices is important here as it determines the front/back face of the polygon

double nx = 0, ny = 0, nz = 0;
pt1 = curve->pointN( 0 );

// shift points by the tessellator's origin - this does not affect normal calculation and it may save us from losing some precision
pt1.setX( pt1.x() - originX );
Expand Down Expand Up @@ -487,7 +495,7 @@ void QgsTessellator::addPolygon( const QgsPolygon &polygon, float extrusionHeigh
if ( !exterior )
return;

const QVector3D pNormal = !mNoZ ? _calculateNormal( exterior, mOriginX, mOriginY, mInvertNormals ) : QVector3D();
const QVector3D pNormal = !mNoZ ? _calculateNormal( exterior, mOriginX, mOriginY, mInvertNormals, extrusionHeight ) : QVector3D();
const int pCount = exterior->numPoints();
if ( pCount == 0 )
return;
Expand Down
116 changes: 116 additions & 0 deletions tests/src/3d/testqgstessellator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ class TestQgsTessellator : public QgsTest
void cleanupTestCase(); // will be called after the last testfunction was executed.

void testBasic();
void testBasicClockwise();
void testWalls();
void testBackEdges();
void test2DTriangle();
Expand Down Expand Up @@ -237,6 +238,121 @@ void TestQgsTessellator::testBasic()

QCOMPARE( tNZ.zMinimum(), 3 );
QCOMPARE( tNZ.zMaximum(), 3 );

// with invert normals enabled, normals point down and triangles are reversed to clockwise
const QVector3D down( 0, 0, -1 ); // surface normal pointing straight down
QList<TriangleCoords> tcInvertedNormals;
tcInvertedNormals << TriangleCoords( QVector3D( 2, 1, 0 ), QVector3D( 1, 1, 0 ), QVector3D( 1, 2, 0 ), down, down, down );
tcInvertedNormals << TriangleCoords( QVector3D( 3, 2, 0 ), QVector3D( 2, 1, 0 ), QVector3D( 1, 2, 0 ), down, down, down );

QList<TriangleCoords> tcInvertedNormalsZ;
tcInvertedNormalsZ << TriangleCoords( QVector3D( 2, 1, 3 ), QVector3D( 1, 1, 3 ), QVector3D( 1, 2, 3 ), down, down, down );
tcInvertedNormalsZ << TriangleCoords( QVector3D( 3, 2, 3 ), QVector3D( 2, 1, 3 ), QVector3D( 1, 2, 3 ), down, down, down );

QgsTessellator tIN( 0, 0, true, true );
tIN.setOutputZUp( true );
tIN.addPolygon( polygon, 0 );
QVERIFY( checkTriangleOutput( tIN.data(), true, tcInvertedNormals ) );

QCOMPARE( tIN.zMinimum(), 0 );
QCOMPARE( tIN.zMaximum(), 0 );

QgsTessellator tINZ( 0, 0, true, true );
tINZ.setOutputZUp( true );
tINZ.addPolygon( polygonZ, 0 );
QVERIFY( checkTriangleOutput( tINZ.data(), true, tcInvertedNormalsZ ) );

QCOMPARE( tINZ.zMinimum(), 3 );
QCOMPARE( tINZ.zMaximum(), 3 );
}

void TestQgsTessellator::testBasicClockwise()
{
// Clockwise polygons are facing down, not up, even when flat
// The triangles also face down and are clockwise
QgsPolygon polygon;
polygon.fromWkt( "POLYGON((1 1, 1 2, 3 2, 2 1, 1 1))" );

QgsPolygon polygonZ;
polygonZ.fromWkt( "POLYGONZ((1 1 3, 1 2 3, 3 2 3, 2 1 3, 1 1 3))" );

QList<TriangleCoords> tc;
tc << TriangleCoords( QVector3D( 2, 1, 0 ), QVector3D( 1, 1, 0 ), QVector3D( 1, 2, 0 ) );
tc << TriangleCoords( QVector3D( 3, 2, 0 ), QVector3D( 2, 1, 0 ), QVector3D( 1, 2, 0 ) );

QList<TriangleCoords> tcZ;
tcZ << TriangleCoords( QVector3D( 2, 1, 3 ), QVector3D( 1, 1, 3 ), QVector3D( 1, 2, 3 ) );
tcZ << TriangleCoords( QVector3D( 3, 2, 3 ), QVector3D( 2, 1, 3 ), QVector3D( 1, 2, 3 ) );

const QVector3D down( 0, 0, -1 ); // surface normal pointing straight down
QList<TriangleCoords> tcNormals;
tcNormals << TriangleCoords( QVector3D( 2, 1, 0 ), QVector3D( 1, 1, 0 ), QVector3D( 1, 2, 0 ), down, down, down );
tcNormals << TriangleCoords( QVector3D( 3, 2, 0 ), QVector3D( 2, 1, 0 ), QVector3D( 1, 2, 0 ), down, down, down );

QList<TriangleCoords> tcNormalsZ;
tcNormalsZ << TriangleCoords( QVector3D( 2, 1, 3 ), QVector3D( 1, 1, 3 ), QVector3D( 1, 2, 3 ), down, down, down );
tcNormalsZ << TriangleCoords( QVector3D( 3, 2, 3 ), QVector3D( 2, 1, 3 ), QVector3D( 1, 2, 3 ), down, down, down );

// without normals

QgsTessellator t( 0, 0, false );
t.setOutputZUp( true );
t.addPolygon( polygon, 0 );
QVERIFY( checkTriangleOutput( t.data(), false, tc ) );

QCOMPARE( t.zMinimum(), 0 );
QCOMPARE( t.zMaximum(), 0 );

QgsTessellator tZ( 0, 0, false );
tZ.setOutputZUp( true );
tZ.addPolygon( polygonZ, 0 );
QVERIFY( checkTriangleOutput( tZ.data(), false, tcZ ) );

QCOMPARE( tZ.zMinimum(), 3 );
QCOMPARE( tZ.zMaximum(), 3 );

// with normals

QgsTessellator tN( 0, 0, true );
tN.setOutputZUp( true );
tN.addPolygon( polygon, 0 );
QVERIFY( checkTriangleOutput( tN.data(), true, tcNormals ) );

QCOMPARE( tN.zMinimum(), 0 );
QCOMPARE( tN.zMaximum(), 0 );

QgsTessellator tNZ( 0, 0, true );
tNZ.setOutputZUp( true );
tNZ.addPolygon( polygonZ, 0 );
QVERIFY( checkTriangleOutput( tNZ.data(), true, tcNormalsZ ) );

QCOMPARE( tNZ.zMinimum(), 3 );
QCOMPARE( tNZ.zMaximum(), 3 );

// with invert normals enabled, normals point up and triangles are reversed to counter clockwise
const QVector3D up( 0, 0, 1 ); // surface normal pointing straight up
QList<TriangleCoords> tcInvertedNormals;
tcInvertedNormals << TriangleCoords( QVector3D( 1, 2, 0 ), QVector3D( 2, 1, 0 ), QVector3D( 3, 2, 0 ), up, up, up );
tcInvertedNormals << TriangleCoords( QVector3D( 1, 2, 0 ), QVector3D( 1, 1, 0 ), QVector3D( 2, 1, 0 ), up, up, up );

QList<TriangleCoords> tcInvertedNormalsZ;
tcInvertedNormalsZ << TriangleCoords( QVector3D( 1, 2, 3 ), QVector3D( 2, 1, 3 ), QVector3D( 3, 2, 3 ), up, up, up );
tcInvertedNormalsZ << TriangleCoords( QVector3D( 1, 2, 3 ), QVector3D( 1, 1, 3 ), QVector3D( 2, 1, 3 ), up, up, up );
QgsTessellator tIN( 0, 0, true, true );
tIN.setOutputZUp( true );
tIN.addPolygon( polygon, 0 );
QVERIFY( checkTriangleOutput( tIN.data(), true, tcInvertedNormals ) );

QCOMPARE( tIN.zMinimum(), 0 );
QCOMPARE( tIN.zMaximum(), 0 );

QgsTessellator tINZ( 0, 0, true, true );
tINZ.setOutputZUp( true );
tINZ.addPolygon( polygonZ, 0 );
QVERIFY( checkTriangleOutput( tINZ.data(), true, tcInvertedNormalsZ ) );

QCOMPARE( tINZ.zMinimum(), 3 );
QCOMPARE( tINZ.zMaximum(), 3 );
}

void TestQgsTessellator::testWalls()
Expand Down
38 changes: 19 additions & 19 deletions tests/src/python/test_qgsvectorlayerprofilegenerator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok. Let's give it a shot then ... I noticed you aren't backporting this anyway, so there's plenty of time to catch regressions

1049.5: 55.2,
1070.2: 55.2,
1073.1: 55.2,
Expand Down Expand Up @@ -1012,7 +1012,7 @@ def testPolygonGenerationRelative(self):
self.round_dict(results.distanceToHeightMap(), 1),
{
1041.8: 60.3,
1042.4: 60.2,
1042.4: 60.3,
1049.5: 60.2,
1070.2: 60.2,
1073.1: 60.2,
Expand All @@ -1023,7 +1023,7 @@ def testPolygonGenerationRelative(self):
1186.8: 59.3,
1189.8: 59.2,
1192.7: 59.2,
1199.2: 59.2,
1199.2: 59.3,
1450.0: 65.5,
1455.6: 65.5,
1458.1: 65.5,
Expand All @@ -1035,10 +1035,10 @@ def testPolygonGenerationRelative(self):
self.assertCountEqual(
[g.asWkt(1) for g in results.asGeometries()],
[
"MultiLineString Z ((-346718.7 6632419.8 60.3, -346712 6632417.4 60.3),(-346719.3 6632420 60.3, -346718.7 6632419.8 60.2),(-346689.7 6632409.5 60.3, -346688.2 6632409 60.3),(-346692.5 6632410.5 60.3, -346689.7 6632409.5 60.3))",
"MultiLineString Z ((-346384.8 6632219 65.5, -346383.5 6632216.9 65.5),(-346387.6 6632223.9 65.5, -346384.8 6632219 65.5))",
"MultiLineString Z ((-346582.6 6632371.7 59.3, -346579.7 6632370.7 59.3),(-346579.7 6632370.7 59.3, -346577 6632369.7 59.3, -346570.8 6632367.9 59.3))",
"MultiLineString Z ((-346684.3 6632407.6 62, -346679.6 6632406 62),(-346679.6 6632406 62, -346672.8 6632403.6 62))",
"MultiLineString Z ((-346582.6 6632371.7 59.3, -346579.7 6632370.7 59.3),(-346579.7 6632370.7 59.3, -346577 6632369.7 59.2, -346570.8 6632367.9 59.3))",
"MultiLineString Z ((-346387.6 6632223.9 65.5, -346384.8 6632219 65.5),(-346384.8 6632219 65.5, -346383.5 6632216.9 65.5))",
"MultiLineString Z ((-346719.3 6632420 60.3, -346718.7 6632419.8 60.3),(-346689.7 6632409.5 60.3, -346688.2 6632409 60.3),(-346692.5 6632410.5 60.3, -346689.7 6632409.5 60.3),(-346718.7 6632419.8 60.3, -346712 6632417.4 60.3))",
],
)

Expand Down Expand Up @@ -1088,7 +1088,7 @@ def testPolygonGenerationRelativeExtrusion(self):
self.round_dict(results.distanceToHeightMap(), 1),
{
1041.8: 60.3,
1042.4: 60.2,
1042.4: 60.3,
1049.5: 60.2,
1070.2: 60.2,
1073.1: 60.2,
Expand All @@ -1099,7 +1099,7 @@ def testPolygonGenerationRelativeExtrusion(self):
1186.8: 59.3,
1189.8: 59.2,
1192.7: 59.2,
1199.2: 59.2,
1199.2: 59.3,
1450.0: 65.5,
1455.6: 65.5,
1458.1: 65.5,
Expand All @@ -1111,10 +1111,10 @@ def testPolygonGenerationRelativeExtrusion(self):
self.assertCountEqual(
[g.asWkt(1) for g in results.asGeometries()],
[
"MultiPolygon Z (((-346718.7 6632419.8 60.3, -346712 6632417.4 60.3, -346712 6632417.4 67.3, -346718.7 6632419.8 67.3, -346718.7 6632419.8 60.3)),((-346719.3 6632420 60.3, -346718.7 6632419.8 60.2, -346718.7 6632419.8 67.3, -346719.3 6632420 67.3, -346719.3 6632420 60.3)),((-346689.7 6632409.5 60.3, -346688.2 6632409 60.3, -346688.2 6632409 67.3, -346689.7 6632409.5 67.3, -346689.7 6632409.5 60.3)),((-346692.5 6632410.5 60.3, -346689.7 6632409.5 60.3, -346689.7 6632409.5 67.3, -346692.5 6632410.5 67.3, -346692.5 6632410.5 60.3)))",
"MultiPolygon Z (((-346684.3 6632407.6 62, -346679.6 6632406 62, -346679.6 6632406 69, -346684.3 6632407.6 69, -346684.3 6632407.6 62)),((-346679.6 6632406 62, -346672.8 6632403.6 62, -346672.8 6632403.6 69, -346679.6 6632406 69, -346679.6 6632406 62)))",
"MultiPolygon Z (((-346582.6 6632371.7 59.3, -346579.7 6632370.7 59.3, -346579.7 6632370.7 66.3, -346582.6 6632371.7 66.3, -346582.6 6632371.7 59.3)),((-346579.7 6632370.7 59.3, -346577 6632369.7 59.2, -346570.8 6632367.9 59.3, -346570.8 6632367.9 66.3, -346577 6632369.7 66.3, -346579.7 6632370.7 66.3, -346579.7 6632370.7 59.3)))",
"MultiPolygon Z (((-346387.6 6632223.9 65.5, -346384.8 6632219 65.5, -346384.8 6632219 72.5, -346387.6 6632223.9 72.5, -346387.6 6632223.9 65.5)),((-346384.8 6632219 65.5, -346383.5 6632216.9 65.5, -346383.5 6632216.9 72.5, -346384.8 6632219 72.5, -346384.8 6632219 65.5)))",
"MultiPolygon Z (((-346719.3 6632420 60.3, -346718.7 6632419.8 60.3, -346718.7 6632419.8 67.3, -346719.3 6632420 67.3, -346719.3 6632420 60.3)),((-346689.7 6632409.5 60.3, -346688.2 6632409 60.3, -346688.2 6632409 67.3, -346689.7 6632409.5 67.3, -346689.7 6632409.5 60.3)),((-346692.5 6632410.5 60.3, -346689.7 6632409.5 60.3, -346689.7 6632409.5 67.3, -346692.5 6632410.5 67.3, -346692.5 6632410.5 60.3)),((-346718.7 6632419.8 60.3, -346712 6632417.4 60.3, -346712 6632417.4 67.3, -346718.7 6632419.8 67.3, -346718.7 6632419.8 60.3)))",
"MultiPolygon Z (((-346384.8 6632219 65.5, -346383.5 6632216.9 65.5, -346383.5 6632216.9 72.5, -346384.8 6632219 72.5, -346384.8 6632219 65.5)),((-346387.6 6632223.9 65.5, -346384.8 6632219 65.5, -346384.8 6632219 72.5, -346387.6 6632223.9 72.5, -346387.6 6632223.9 65.5)))",
"MultiPolygon Z (((-346582.6 6632371.7 59.3, -346579.7 6632370.7 59.3, -346579.7 6632370.7 66.3, -346582.6 6632371.7 66.3, -346582.6 6632371.7 59.3)),((-346579.7 6632370.7 59.3, -346577 6632369.7 59.3, -346570.8 6632367.9 59.3, -346570.8 6632367.9 66.3, -346577 6632369.7 66.3, -346579.7 6632370.7 66.3, -346579.7 6632370.7 59.3)))",
],
)

Expand Down Expand Up @@ -1296,7 +1296,7 @@ def testDataDefinedExtrusionOffset(self):
self.assertCountEqual(
[g.asWkt(1) for g in results.asGeometries()],
[
"MultiPolygon Z (((321906.5 129918.5 36, 321907.7 129918.8 36, 321907.7 129918.8 53, 321906.5 129918.5 53, 321906.5 129918.5 36)),((321902.8 129917.9 36, 321906.5 129918.5 36, 321906.5 129918.5 53, 321902.8 129917.9 53, 321902.8 129917.9 36)),((321917.9 129920.6 36, 321921 129921.1 36, 321921 129921.1 53, 321917.9 129920.6 53, 321917.9 129920.6 36)),((321912.4 129919.6 36, 321917.9 129920.6 36, 321917.9 129920.6 53, 321912.4 129919.6 53, 321912.4 129919.6 36)))",
"MultiPolygon Z (((321902.8 129917.9 36, 321906.5 129918.5 36, 321906.5 129918.5 53, 321902.8 129917.9 53, 321902.8 129917.9 36)),((321917.9 129920.6 36, 321921 129921.1 36, 321921 129921.1 53, 321917.9 129920.6 53, 321917.9 129920.6 36)),((321912.4 129919.6 36, 321917.9 129920.6 36, 321917.9 129920.6 53, 321912.4 129919.6 53, 321912.4 129919.6 36)),((321906.5 129918.5 36, 321907.7 129918.8 36, 321907.7 129918.8 53, 321906.5 129918.5 53, 321906.5 129918.5 36)))",
"MultiPolygon Z (((321922.9 129921.5 37, 321927.8 129922.4 37, 321927.8 129922.4 54, 321922.9 129921.5 54, 321922.9 129921.5 37)),((321927.8 129922.4 37, 321929.5 129922.7 37, 321929.5 129922.7 54, 321927.8 129922.4 54, 321927.8 129922.4 37)))",
],
)
Expand All @@ -1319,8 +1319,8 @@ def testDataDefinedExtrusionOffset(self):
self.assertCountEqual(
[g.asWkt(1) for g in results.asGeometries()],
[
"MultiPolygon Z (((321902.8 129917.9 8, 321906.5 129918.5 8, 321906.5 129918.5 9, 321902.8 129917.9 9, 321902.8 129917.9 8)),((321917.9 129920.6 8, 321921 129921.1 8, 321921 129921.1 9, 321917.9 129920.6 9, 321917.9 129920.6 8)),((321912.4 129919.6 8, 321917.9 129920.6 8, 321917.9 129920.6 9, 321912.4 129919.6 9, 321912.4 129919.6 8)),((321906.5 129918.5 8, 321907.7 129918.8 8, 321907.7 129918.8 9, 321906.5 129918.5 9, 321906.5 129918.5 8)))",
"MultiPolygon Z (((321922.9 129921.5 4, 321927.8 129922.4 4, 321927.8 129922.4 6, 321922.9 129921.5 6, 321922.9 129921.5 4)),((321927.8 129922.4 4, 321929.5 129922.7 4, 321929.5 129922.7 6, 321927.8 129922.4 6, 321927.8 129922.4 4)))",
"MultiPolygon Z (((321906.5 129918.5 8, 321907.7 129918.8 8, 321907.7 129918.8 9, 321906.5 129918.5 9, 321906.5 129918.5 8)),((321902.8 129917.9 8, 321906.5 129918.5 8, 321906.5 129918.5 9, 321902.8 129917.9 9, 321902.8 129917.9 8)),((321917.9 129920.6 8, 321921 129921.1 8, 321921 129921.1 9, 321917.9 129920.6 9, 321917.9 129920.6 8)),((321912.4 129919.6 8, 321917.9 129920.6 8, 321917.9 129920.6 9, 321912.4 129919.6 9, 321912.4 129919.6 8)))",
],
)

Expand Down Expand Up @@ -1849,9 +1849,9 @@ def testIdentifyPolygons(self):
res[0].results(),
[
{
"delta": 1.635491751097172,
"delta": 1.6354917510971725,
"distance": 40.70584650527578,
"elevation": 2.0000000000000093,
"elevation": 2.0000000000000084,
"id": 2,
}
],
Expand All @@ -1866,9 +1866,9 @@ def testIdentifyPolygons(self):
res[0].results(),
[
{
"delta": 0.9999999999968199,
"delta": 0.9999999999968203,
"distance": 35.0,
"elevation": 2.00000000000318,
"elevation": 2.0000000000031797,
"id": 2,
}
],
Expand Down Expand Up @@ -1966,9 +1966,9 @@ def testIdentifyExtrudedPolygons(self):
res[0].results(),
[
{
"delta": 1.635491751097172,
"delta": 1.6354917510971725,
"distance": 40.70584650527578,
"elevation": 2.0000000000000093,
"elevation": 2.0000000000000084,
"id": 2,
}
],
Expand Down
Loading