Skip to content

Commit

Permalink
[Oracle] Take into account orientation when writing polygon rings
Browse files Browse the repository at this point in the history 
Fixes #29085
  • Loading branch information
@rouault
rouault committed Jan 21, 2020
1 parent e770cdd commit 60e37d6
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 37 deletions.
112 changes: 75 additions & 37 deletions src/providers/oracle/qgsoracleprovider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
#include "qgsfeature.h"
#include "qgsfields.h"
#include "qgsgeometry.h"
#include "qgscurve.h"
#include "qgscompoundcurve.h"
#include "qgspolygon.h"
#include "qgsmultipolygon.h"
#include "qgsmultisurface.h"
#include "qgsmessageoutput.h"
#include "qgsmessagelog.h"
#include "qgsrectangle.h"
Expand Down Expand Up @@ -2037,7 +2042,10 @@ void QgsOracleProvider::appendGeomParam( const QgsGeometry &geom, QSqlQuery &qry
{
g.gtype = SDO_GTYPE( dim, GtPolygon );
int nPolygons = 1;
if ( QgsWkbTypes::flatType( type ) == QgsWkbTypes::MultiPolygon )
const QgsMultiPolygon *multipoly =
( QgsWkbTypes::flatType( type ) == QgsWkbTypes::MultiPolygon ) ?
dynamic_cast<const QgsMultiPolygon *>( geom.constGet() ) : nullptr;
if ( multipoly )
{
g.gtype = SDO_GTYPE( dim, GtMultiPolygon );
nPolygons = *ptr.iPtr++;
Expand All @@ -2048,19 +2056,44 @@ void QgsOracleProvider::appendGeomParam( const QgsGeometry &geom, QSqlQuery &qry

for ( int iPolygon = 0; iPolygon < nPolygons; iPolygon++ )
{
const QgsPolygon *poly = multipoly ?
dynamic_cast<const QgsPolygon *>( multipoly->geometryN( iPolygon ) ) :
dynamic_cast<const QgsPolygon *>( geom.constGet() );
for ( int iRing = 0, nRings = *ptr.iPtr++; iRing < nRings; iRing++ )
{
g.eleminfo << iOrdinate << ( iRing == 0 ? 1003 : 2003 ) << 1;

// TODO: verify ring orientation
for ( int i = 0, n = *ptr.iPtr++; i < n; i++ )
// Oracle polygons must have their exterior ring in counterclockwise
// order, and the interior ring(s) in clockwise order.
const bool reverseRing =
iRing == 0 ? poly->exteriorRing()->orientation() == QgsCurve::Orientation::Clockwise :
poly->interiorRing( iRing - 1 )->orientation() == QgsCurve::Orientation::CounterClockwise;

const int n = *ptr.iPtr++;
if ( reverseRing )
{
g.ordinates << *ptr.dPtr++;
g.ordinates << *ptr.dPtr++;
if ( dim == 3 )
const double *dPtr = ptr.dPtr + n * dim;
for ( int i = 0; i < n; i++ )
{
dPtr -= dim;
g.ordinates << dPtr[0];
g.ordinates << dPtr[1];
if ( dim == 3 )
g.ordinates << dPtr[2];
}
ptr.dPtr += n * dim;
}
else
{
for ( int i = 0; i < n; i++ )
{
g.ordinates << *ptr.dPtr++;
iOrdinate += dim;
g.ordinates << *ptr.dPtr++;
if ( dim == 3 )
g.ordinates << *ptr.dPtr++;
}
}
iOrdinate += n * dim;
}

ptr.ucPtr++; // Skip endianness of next polygon
Expand Down Expand Up @@ -2183,40 +2216,43 @@ void QgsOracleProvider::appendGeomParam( const QgsGeometry &geom, QSqlQuery &qry
{
g.gtype = SDO_GTYPE( dim, GtPolygon );
int nSurfaces = 1;
if ( type == QgsWkbTypes::MultiSurface || type == QgsWkbTypes::MultiSurfaceZ )
const QgsMultiSurface *multisurface =
( QgsWkbTypes::flatType( type ) == QgsWkbTypes::MultiSurface ) ?
dynamic_cast<const QgsMultiSurface *>( geom.constGet() ) : nullptr;
if ( multisurface )
{
g.gtype = SDO_GTYPE( dim, GtMultiPolygon );
nSurfaces = *ptr.iPtr++;
nSurfaces = multisurface->numGeometries();
}

for ( int iSurface = 0; iSurface < nSurfaces; iSurface++ )
{
QgsWkbTypes::Type surfaceType = QgsWkbTypes::CurvePolygon;
if ( type == QgsWkbTypes::MultiSurface || type == QgsWkbTypes::MultiSurfaceZ )
{
ptr.ucPtr++; // Skip endianness of surface
surfaceType = ( QgsWkbTypes::Type ) * ptr.iPtr++; // type of surface
}
const QgsCurvePolygon *curvepoly = multisurface ?
dynamic_cast<const QgsCurvePolygon *>( multisurface->geometryN( iSurface ) ) :
dynamic_cast<const QgsCurvePolygon *>( geom.constGet() );

const int nRings = *ptr.iPtr++;
const int nRings = ( curvepoly->exteriorRing() ? 1 : 0 ) + curvepoly->numInteriorRings();

for ( int iRing = 0; iRing < nRings; iRing++ )
{
QgsWkbTypes::Type ringType = QgsWkbTypes::LineString;
if ( surfaceType == QgsWkbTypes::CurvePolygon || surfaceType == QgsWkbTypes::CurvePolygonZ )
{
ptr.ucPtr++; // Skip endianness of ring
ringType = ( QgsWkbTypes::Type ) * ptr.iPtr++; // type of ring
}

const QgsCurve *ring = iRing == 0 ? curvepoly->exteriorRing() : curvepoly->interiorRing( iRing - 1 );
const QgsWkbTypes::Type ringType = ring->wkbType();

// Oracle polygons must have their exterior ring in counterclockwise
// order, and the interior ring(s) in clockwise order.
const bool reverseRing =
iRing == 0 ? ring->orientation() == QgsCurve::Orientation::Clockwise :
ring->orientation() == QgsCurve::Orientation::CounterClockwise;
std::unique_ptr<QgsCurve> reversedRing( reverseRing ? ring->reversed() : nullptr );
const QgsCurve *correctedRing = reversedRing ? reversedRing.get() : ring;
const QgsCompoundCurve *compound = dynamic_cast<const QgsCompoundCurve *>( correctedRing );
int nLines = 1;
QgsWkbTypes::Type lineType = ringType;
if ( QgsWkbTypes::flatType( ringType ) == QgsWkbTypes::CompoundCurve )
if ( compound )
{
nLines = *ptr.iPtr++;

ptr.ucPtr++; // Skip endianness of first linestring
lineType = ( QgsWkbTypes::Type ) * ptr.iPtr++; // type of first linestring
nLines = compound->nCurves();
if ( nLines )
lineType = compound->curveAt( 0 )->wkbType();
}

// Oracle don't store compound curve with only one line
Expand All @@ -2228,29 +2264,31 @@ void QgsOracleProvider::appendGeomParam( const QgsGeometry &geom, QSqlQuery &qry
{
if ( iLine > 0 )
{
ptr.ucPtr++; // Skip endianness of linestring
lineType = ( QgsWkbTypes::Type ) * ptr.iPtr++; // type of linestring
lineType = compound->curveAt( iLine )->wkbType();
}
if ( nLines > 1 )
{
g.eleminfo << iOrdinate << 2 << ( QgsWkbTypes::flatType( lineType ) == QgsWkbTypes::CircularString ? 2 : 1 );
}
const QgsCurve *lineCurve = compound ? compound->curveAt( iLine ) : correctedRing;

for ( int i = 0, n = *ptr.iPtr++; i < n; i++ )
for ( int i = 0, n = lineCurve->numPoints(); i < n; i++ )
{
// Inside a compound polygon, two consecutives lines share start/end points
// We don't repeat this point in ordinates, so we skip the last point (except for last line)
if ( QgsWkbTypes::flatType( ringType ) == QgsWkbTypes::CompoundCurve
if ( compound
&& i == n - 1 && iLine < nLines - 1 )
{
ptr.dPtr += dim;
continue;
break;
}

g.ordinates << *ptr.dPtr++;
g.ordinates << *ptr.dPtr++;
QgsPoint p;
QgsVertexId::VertexType ignored;
lineCurve->pointAt( i, p, ignored );
g.ordinates << p.x();
g.ordinates << p.y();
if ( dim == 3 )
g.ordinates << *ptr.dPtr++;
g.ordinates << p.z();

iOrdinate += dim;
}
Expand Down
26 changes: 26 additions & 0 deletions tests/src/python/test_provider_oracle.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,15 @@ def testEditSurfaces(self):
fid += 1
self.check_geom(polygon, fid, 'Polygon ((1 2, 11 2, 11 22, 1 22, 1 2),(5 6, 8 9, 8 6, 5 6),(3 4, 3 6, 5 6, 3 4))')
fid += 1
# Outer ring in clockwise order --> reversed on writing
self.check_geom(polygon, fid, 'Polygon ((0 0, 0 1, 1 1, 0 0))', 'Polygon ((0 0, 1 1, 0 1, 0 0))')
fid += 1
# Outer ring in clockwise order --> reversed on writing. Inner ring in clockwise order --> unmodified
self.check_geom(polygon, fid, 'Polygon ((0 0, 0 1, 1 1, 0 0),(0.1 0.2, 0.1 0.9, 0.7 0.9, 0.1 0.2))', 'Polygon ((0 0, 1 1, 0 1, 0 0),(0.1 0.2, 0.1 0.9, 0.7 0.9, 0.1 0.2))')
fid += 1
# Inner ring in counterclockwise order --> reversed on writing. Outer ring in counterclockwise order --> unmodified
self.check_geom(polygon, fid, 'Polygon ((0 0, 1 1, 0 1, 0 0),(0.1 0.2, 0.7 0.9, 0.1 0.9, 0.1 0.2))', 'Polygon ((0 0, 1 1, 0 1, 0 0),(0.1 0.2, 0.1 0.9, 0.7 0.9, 0.1 0.2))')
fid += 1

polygon_z = QgsVectorLayer(
self.dbconn + ' sslmode=disable key=\'pk\' srid=5698 type=PolygonZ table="QGIS"."EDIT_SURFACEZ_DATA" (GEOM) sql=',
Expand Down Expand Up @@ -485,6 +494,12 @@ def testEditSurfaces(self):
fid += 1
self.check_geom(curve_polygon, fid, 'CurvePolygon((0 0, 30 0, 30 20, 0 30, 0 0), CircularString (1 3, 3 5, 4 7, 7 3, 1 3))')
fid += 1
# Reverse orientation of outer ring
self.check_geom(curve_polygon, fid, 'CurvePolygon((0 0, 0 30, 30 20, 30 0, 0 0), CircularString (1 3, 3 5, 4 7, 7 3, 1 3))', 'CurvePolygon((0 0, 30 0, 30 20, 0 30, 0 0), CircularString (1 3, 3 5, 4 7, 7 3, 1 3))')
fid += 1
# Reverse orientation of outer ring
self.check_geom(curve_polygon, fid, 'CurvePolygon( CompoundCurve ((0 0, 0 1, 1 1),(1 1,0 0)))', 'CurvePolygon (CompoundCurve ((0 0, 1 1),(1 1, 0 1, 0 0)))')
fid += 1

curve_polygon_z = QgsVectorLayer(
self.dbconn + ' sslmode=disable key=\'pk\' srid=5698 type=CurvePolygonZ table="QGIS"."EDIT_SURFACEZ_DATA" (GEOM) sql=',
Expand Down Expand Up @@ -518,6 +533,17 @@ def testEditSurfaces(self):
wkt = 'MultiSurface (((0 0, 1 0, 1 1, 0 0)),((100 100, 101 100, 101 101, 100 100)))'
self.check_geom(multi_surface, fid, wkt, wkt.replace('MultiSurface', 'MultiPolygon'))
fid += 1

# Outer ring in clockwise order --> reversed on writing
self.check_geom(multi_surface, fid, 'MultiSurface( Polygon ((0 0, 0 1, 1 1, 0 0)))', 'MultiPolygon (((0 0, 1 1, 0 1, 0 0)))')
fid += 1
# Outer ring in clockwise order --> reversed on writing. Inner ring in clockwise order --> unmodified
self.check_geom(multi_surface, fid, 'MultiSurface(Polygon ((0 0, 0 1, 1 1, 0 0),(0.1 0.2, 0.1 0.9, 0.7 0.9, 0.1 0.2)))', 'MultiPolygon (((0 0, 1 1, 0 1, 0 0),(0.1 0.2, 0.1 0.9, 0.7 0.9, 0.1 0.2)))')
fid += 1
# Inner ring in counterclockwise order --> reversed on writing. Outer ring in counterclockwise order --> unmodified
self.check_geom(multi_surface, fid, 'MultiSurface(Polygon ((0 0, 1 1, 0 1, 0 0),(0.1 0.2, 0.7 0.9, 0.1 0.9, 0.1 0.2)))', 'MultiPolygon (((0 0, 1 1, 0 1, 0 0),(0.1 0.2, 0.1 0.9, 0.7 0.9, 0.1 0.2)))')
fid += 1

self.check_geom(multi_surface, fid, 'MultiSurface (Polygon ((0 0, 1 0, 1 1, 0 0)),CurvePolygon (CompoundCurve (CircularString (100 100, 105.5 99.5, 101 100, 101.5 100.5, 101 101),(101 101, 100 100))))')
fid += 1
self.check_geom(multi_surface, fid, 'MultiSurface (CurvePolygon (CompoundCurve (CircularString (100 100, 101 100, 101 101),(101 101, 100 100))),Polygon ((0 0, 1 0, 1 1, 0 0)))')
Expand Down

0 comments on commit 60e37d6

Please sign in to comment.