Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
When scaling geometries for tesselation, ensure we don't change the
geometry's aspect ratio

If we scale by an uneven amount in the x vs y plane, then the resultant
tesselation uses a misrepresentation of the actual shape of the geometry,
resulting in a poor quality tesselation.

Follow up 8ee1c20

Fixes #37077
  • Loading branch information
nyalldawson committed Jun 10, 2020
1 parent b5b4221 commit 61e7a5f
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 13 deletions.
25 changes: 12 additions & 13 deletions src/core/qgstessellator.cpp
Expand Up @@ -268,7 +268,7 @@ inline double _round_coord( double x )
}


static QgsCurve *_transform_ring_to_new_base( const QgsLineString &curve, const QgsPoint &pt0, const QMatrix4x4 *toNewBase, float scaleX, float scaleY )
static QgsCurve *_transform_ring_to_new_base( const QgsLineString &curve, const QgsPoint &pt0, const QMatrix4x4 *toNewBase, const float scale )
{
int count = curve.numPoints();
QVector<double> x;
Expand All @@ -295,8 +295,8 @@ static QgsCurve *_transform_ring_to_new_base( const QgsLineString &curve, const
v = toNewBase->map( v );

// scale coordinates
v.setX( v.x() * scaleX );
v.setY( v.y() * scaleY );
v.setX( v.x() * scale );
v.setY( v.y() * scale );

// we also round coordinates before passing them to poly2tri triangulation in order to fix possible numerical
// stability issues. We had crashes with nearly collinear points where one of the points was off by a tiny bit (e.g. by 1e-20).
Expand All @@ -316,12 +316,12 @@ static QgsCurve *_transform_ring_to_new_base( const QgsLineString &curve, const
}


static QgsPolygon *_transform_polygon_to_new_base( const QgsPolygon &polygon, const QgsPoint &pt0, const QMatrix4x4 *toNewBase, float scaleX, float scaleY )
static QgsPolygon *_transform_polygon_to_new_base( const QgsPolygon &polygon, const QgsPoint &pt0, const QMatrix4x4 *toNewBase, const float scale )
{
QgsPolygon *p = new QgsPolygon;
p->setExteriorRing( _transform_ring_to_new_base( *qgsgeometry_cast< const QgsLineString * >( polygon.exteriorRing() ), pt0, toNewBase, scaleX, scaleY ) );
p->setExteriorRing( _transform_ring_to_new_base( *qgsgeometry_cast< const QgsLineString * >( polygon.exteriorRing() ), pt0, toNewBase, scale ) );
for ( int i = 0; i < polygon.numInteriorRings(); ++i )
p->addInteriorRing( _transform_ring_to_new_base( *qgsgeometry_cast< const QgsLineString * >( polygon.interiorRing( i ) ), pt0, toNewBase, scaleX, scaleY ) );
p->addInteriorRing( _transform_ring_to_new_base( *qgsgeometry_cast< const QgsLineString * >( polygon.interiorRing( i ) ), pt0, toNewBase, scale ) );
return p;
}

Expand Down Expand Up @@ -494,13 +494,12 @@ void QgsTessellator::addPolygon( const QgsPolygon &polygon, float extrusionHeigh
const QgsPoint ptStart( exterior->startPoint() );
const QgsPoint pt0( QgsWkbTypes::PointZ, ptStart.x(), ptStart.y(), std::isnan( ptStart.z() ) ? 0 : ptStart.z() );

const float scaleX = !mBounds.isNull() ? 10000.0 / mBounds.width() : 1.0;
const float scaleY = !mBounds.isNull() ? 10000.0 / mBounds.height() : 1.0;
const float scale = mBounds.isNull() ? 1.0 : std::max( 10000.0 / mBounds.width(), 10000.0 / mBounds.height() );

// subtract ptFirst from geometry for better numerical stability in triangulation
// and apply new 3D vector base if the polygon is not horizontal

std::unique_ptr<QgsPolygon> polygonNew( _transform_polygon_to_new_base( polygon, pt0, toNewBase.get(), scaleX, scaleY ) );
std::unique_ptr<QgsPolygon> polygonNew( _transform_polygon_to_new_base( polygon, pt0, toNewBase.get(), scale ) );

if ( _minimum_distance_between_coordinates( *polygonNew ) < 0.001 )
{
Expand Down Expand Up @@ -574,8 +573,8 @@ void QgsTessellator::addPolygon( const QgsPolygon &polygon, float extrusionHeigh
QVector4D pt( p->x, p->y, mNoZ ? 0 : z[p], 0 );
if ( toOldBase )
pt = *toOldBase * pt;
const double fx = ( pt.x() / scaleX ) - mOriginX + pt0.x();
const double fy = ( pt.y() / scaleY ) - mOriginY + pt0.y();
const double fx = ( pt.x() / scale ) - mOriginX + pt0.x();
const double fy = ( pt.y() / scale ) - mOriginY + pt0.y();
const double fz = mNoZ ? 0 : ( pt.z() + extrusionHeight + pt0.z() );
if ( fz < zMin )
zMin = fz;
Expand All @@ -596,8 +595,8 @@ void QgsTessellator::addPolygon( const QgsPolygon &polygon, float extrusionHeigh
QVector4D pt( p->x, p->y, mNoZ ? 0 : z[p], 0 );
if ( toOldBase )
pt = *toOldBase * pt;
const double fx = ( pt.x() / scaleX ) - mOriginX + pt0.x();
const double fy = ( pt.y() / scaleY ) - mOriginY + pt0.y();
const double fx = ( pt.x() / scale ) - mOriginX + pt0.x();
const double fy = ( pt.y() / scale ) - mOriginY + pt0.y();
const double fz = mNoZ ? 0 : ( pt.z() + extrusionHeight + pt0.z() );
mData << fx << fz << -fy;
if ( mAddNormals )
Expand Down
17 changes: 17 additions & 0 deletions tests/src/3d/testqgstessellator.cpp
Expand Up @@ -21,6 +21,8 @@
#include "qgspolygon.h"
#include "qgstessellator.h"
#include "qgsmultipolygon.h"
#include "qgslogger.h"
#include "qgsgeometry.h"

static bool qgsVectorNear( const QVector3D &v1, const QVector3D &v2, double eps )
{
Expand Down Expand Up @@ -140,6 +142,7 @@ class TestQgsTessellator : public QObject
void testNoZ();
void testTriangulationDoesNotCrash();
void testCrash2DTriangle();
void narrowPolygon();

private:
};
Expand Down Expand Up @@ -407,5 +410,19 @@ void TestQgsTessellator::testCrash2DTriangle()
t.addPolygon( polygon, 0 ); // must not crash - that's all we test here
}

void TestQgsTessellator::narrowPolygon()
{
// test that tesselation of a long narrow polygon results in a "nice" tesselation (i.e. not lots of long thin triangles)
// refs https://github.com/qgis/QGIS/issues/37077
QgsPolygon polygon;
polygon.fromWkt( "Polygon ((383393.53728186257649213 4902093.79335568379610777, 383383.73728171654511243 4902092.99335567187517881, 383375.25399118528002873 4902092.8891992112621665, 383368.08741026872303337 4902093.48088630195707083, 383362.87084332120139152 4902093.91129046399146318, 383359.60429034277331084 4902094.18041169829666615, 383357.23274148383643478 4902093.6530067715793848, 383355.75619674433255568 4902092.32907568290829659, 383355.57501344417687505 4902090.56084021460264921, 383356.68919158342760056 4902088.34830036386847496, 383361.07830215193098411 4902086.48156050778925419, 383368.74234514962881804 4902084.96062064450234175, 383380.44909519288921729 4902084.10479906108230352, 383396.19855228182859719 4902083.91409575659781694, 383406.97328086948255077 4902084.21874411031603813, 383412.77328095590928569 4902085.01874412223696709, 383416.70496230950811878 4902086.31405469868332148, 383418.76832493022084236 4902088.1046758396551013, 383419.69647055567475036 4902089.60464367642998695, 383419.48939918575342745 4902090.81395820714533329, 383418.40130056580528617 4902092.01381655503064394, 383416.43217469577211887 4902093.20421871729195118, 383411.19502930447924882 4902093.89790377207100391, 383402.68986439192667603 4902094.09487171657383442, 383393.53728186257649213 4902093.79335568379610777))" );
QgsTessellator t( polygon.boundingBox(), false );
t.addPolygon( polygon, 0 );
QgsGeometry res( t.asMultiPolygon() );
res.translate( polygon.boundingBox().xMinimum(), polygon.boundingBox().yMinimum() );
QgsDebugMsg( res.asWkt( 0 ) );
QCOMPARE( res.asWkt( 0 ), QStringLiteral( "MultiPolygonZ (((383357 4902094 0, 383356 4902092 0, 383356 4902091 0, 383357 4902094 0)),((383357 4902088 0, 383357 4902094 0, 383356 4902091 0, 383357 4902088 0)),((383357 4902088 0, 383361 4902086 0, 383357 4902094 0, 383357 4902088 0)),((383357 4902094 0, 383361 4902086 0, 383360 4902094 0, 383357 4902094 0)),((383363 4902094 0, 383360 4902094 0, 383361 4902086 0, 383363 4902094 0)),((383368 4902093 0, 383363 4902094 0, 383361 4902086 0, 383368 4902093 0)),((383368 4902093 0, 383361 4902086 0, 383369 4902085 0, 383368 4902093 0)),((383368 4902093 0, 383369 4902085 0, 383375 4902093 0, 383368 4902093 0)),((383375 4902093 0, 383369 4902085 0, 383380 4902084 0, 383375 4902093 0)),((383375 4902093 0, 383380 4902084 0, 383384 4902093 0, 383375 4902093 0)),((383384 4902093 0, 383380 4902084 0, 383396 4902084 0, 383384 4902093 0)),((383394 4902094 0, 383384 4902093 0, 383396 4902084 0, 383394 4902094 0)),((383403 4902094 0, 383394 4902094 0, 383396 4902084 0, 383403 4902094 0)),((383396 4902084 0, 383407 4902084 0, 383403 4902094 0, 383396 4902084 0)),((383411 4902094 0, 383403 4902094 0, 383407 4902084 0, 383411 4902094 0)),((383411 4902094 0, 383407 4902084 0, 383413 4902085 0, 383411 4902094 0)),((383411 4902094 0, 383413 4902085 0, 383416 4902093 0, 383411 4902094 0)),((383416 4902093 0, 383413 4902085 0, 383417 4902086 0, 383416 4902093 0)),((383419 4902088 0, 383416 4902093 0, 383417 4902086 0, 383419 4902088 0)),((383418 4902092 0, 383416 4902093 0, 383419 4902088 0, 383418 4902092 0)),((383418 4902092 0, 383419 4902088 0, 383419 4902091 0, 383418 4902092 0)),((383419 4902091 0, 383419 4902088 0, 383420 4902090 0, 383419 4902091 0)))" ) );
}

QGSTEST_MAIN( TestQgsTessellator )
#include "testqgstessellator.moc"

0 comments on commit 61e7a5f

Please sign in to comment.