diff --git a/python/core/auto_generated/symbology/qgssymbol.sip.in b/python/core/auto_generated/symbology/qgssymbol.sip.in index 3356093e138a..fdd9a17be10c 100644 --- a/python/core/auto_generated/symbology/qgssymbol.sip.in +++ b/python/core/auto_generated/symbology/qgssymbol.sip.in @@ -370,6 +370,32 @@ side effects for certain symbol types. .. seealso:: :py:func:`setClipFeaturesToExtent` .. versionadded:: 2.9 +%End + + void setForceRHR( bool force ); +%Docstring +Sets whether polygon features drawn by the symbol should be reoriented to follow the +standard right-hand-rule orientation, in which the area that is +bounded by the polygon is to the right of the boundary. In particular, the exterior +ring is oriented in a clockwise direction and the interior rings in a counter-clockwise +direction. + +.. seealso:: :py:func:`forceRHR` + +.. versionadded:: 3.6 +%End + + bool forceRHR() const; +%Docstring +Returns true if polygon features drawn by the symbol will be reoriented to follow the +standard right-hand-rule orientation, in which the area that is +bounded by the polygon is to the right of the boundary. In particular, the exterior +ring is oriented in a clockwise direction and the interior rings in a counter-clockwise +direction. + +.. seealso:: :py:func:`setForceRHR` + +.. versionadded:: 3.6 %End QSet usedAttributes( const QgsRenderContext &context ) const; @@ -428,14 +454,20 @@ Creates a point in screen coordinates from a QgsPoint in map coordinates Creates a line string in screen coordinates from a QgsCurve in map coordinates %End - static QPolygonF _getPolygonRing( QgsRenderContext &context, const QgsCurve &curve, bool clipToExtent ); + static QPolygonF _getPolygonRing( QgsRenderContext &context, const QgsCurve &curve, bool clipToExtent, bool isExteriorRing = false, bool correctRingOrientation = false ); %Docstring -Creates a polygon ring in screen coordinates from a QgsCurve in map coordinates +Creates a polygon ring in screen coordinates from a QgsCurve in map coordinates. + +If ``correctRingOrientation`` is true then the ring will be oriented to match standard ring orientation, e.g. +clockwise for exterior rings and counter-clockwise for interior rings. %End - static void _getPolygon( QPolygonF &pts, QList &holes, QgsRenderContext &context, const QgsPolygon &polygon, bool clipToExtent = true ); + static void _getPolygon( QPolygonF &pts, QList &holes, QgsRenderContext &context, const QgsPolygon &polygon, bool clipToExtent = true, bool correctRingOrientation = false ); %Docstring Creates a polygon in screen coordinates from a QgsPolygonXYin map coordinates + +If ``correctRingOrientation`` is true then the ring will be oriented to match standard ring orientation, e.g. +clockwise for exterior rings and counter-clockwise for interior rings. %End QgsSymbolLayerList cloneLayers() const /Factory/; diff --git a/src/core/symbology/qgssymbol.cpp b/src/core/symbology/qgssymbol.cpp index 8231176dd7b9..3e773d4a1f7e 100644 --- a/src/core/symbology/qgssymbol.cpp +++ b/src/core/symbology/qgssymbol.cpp @@ -141,7 +141,7 @@ QPolygonF QgsSymbol::_getLineString( QgsRenderContext &context, const QgsCurve & return pts; } -QPolygonF QgsSymbol::_getPolygonRing( QgsRenderContext &context, const QgsCurve &curve, bool clipToExtent ) +QPolygonF QgsSymbol::_getPolygonRing( QgsRenderContext &context, const QgsCurve &curve, const bool clipToExtent, const bool isExteriorRing, const bool correctRingOrientation ) { const QgsCoordinateTransform ct = context.coordinateTransform(); const QgsMapToPixel &mtp = context.mapToPixel(); @@ -155,6 +155,15 @@ QPolygonF QgsSymbol::_getPolygonRing( QgsRenderContext &context, const QgsCurve if ( curve.numPoints() < 1 ) return QPolygonF(); + if ( correctRingOrientation ) + { + // ensure consistent polygon ring orientation + if ( isExteriorRing && curve.orientation() != QgsCurve::Clockwise ) + std::reverse( poly.begin(), poly.end() ); + else if ( !isExteriorRing && curve.orientation() != QgsCurve::CounterClockwise ) + std::reverse( poly.begin(), poly.end() ); + } + //clip close to view extent, if needed const QRectF ptsRect = poly.boundingRect(); if ( clipToExtent && !context.extent().contains( ptsRect ) ) @@ -184,14 +193,14 @@ QPolygonF QgsSymbol::_getPolygonRing( QgsRenderContext &context, const QgsCurve return poly; } -void QgsSymbol::_getPolygon( QPolygonF &pts, QList &holes, QgsRenderContext &context, const QgsPolygon &polygon, bool clipToExtent ) +void QgsSymbol::_getPolygon( QPolygonF &pts, QList &holes, QgsRenderContext &context, const QgsPolygon &polygon, const bool clipToExtent, const bool correctRingOrientation ) { holes.clear(); - pts = _getPolygonRing( context, *polygon.exteriorRing(), clipToExtent ); + pts = _getPolygonRing( context, *polygon.exteriorRing(), clipToExtent, true, correctRingOrientation ); for ( int idx = 0; idx < polygon.numInteriorRings(); idx++ ) { - const QPolygonF hole = _getPolygonRing( context, *( polygon.interiorRing( idx ) ), clipToExtent ); + const QPolygonF hole = _getPolygonRing( context, *( polygon.interiorRing( idx ) ), clipToExtent, false, correctRingOrientation ); if ( !hole.isEmpty() ) holes.append( hole ); } } @@ -850,7 +859,7 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont QgsDebugMsg( QStringLiteral( "cannot render polygon with no exterior ring" ) ); break; } - _getPolygon( pts, holes, context, polygon, !tileMapRendering && clipFeaturesToExtent() ); + _getPolygon( pts, holes, context, polygon, !tileMapRendering && clipFeaturesToExtent(), mForceRHR ); static_cast( this )->renderPolygon( pts, ( !holes.isEmpty() ? &holes : nullptr ), &feature, context, layer, selected ); if ( drawVertexMarker && !usingSegmentizedGeometry ) @@ -980,7 +989,7 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont if ( !polygon.exteriorRing() ) break; - _getPolygon( pts, holes, context, polygon, !tileMapRendering && clipFeaturesToExtent() ); + _getPolygon( pts, holes, context, polygon, !tileMapRendering && clipFeaturesToExtent(), mForceRHR ); static_cast( this )->renderPolygon( pts, ( !holes.isEmpty() ? &holes : nullptr ), &feature, context, layer, selected ); if ( drawVertexMarker && !usingSegmentizedGeometry ) @@ -1574,6 +1583,7 @@ QgsMarkerSymbol *QgsMarkerSymbol::clone() const cloneSymbol->setLayer( mLayer ); Q_NOWARN_DEPRECATED_POP cloneSymbol->setClipFeaturesToExtent( mClipFeaturesToExtent ); + cloneSymbol->setForceRHR( mForceRHR ); return cloneSymbol; } @@ -1793,6 +1803,7 @@ QgsLineSymbol *QgsLineSymbol::clone() const cloneSymbol->setLayer( mLayer ); Q_NOWARN_DEPRECATED_POP cloneSymbol->setClipFeaturesToExtent( mClipFeaturesToExtent ); + cloneSymbol->setForceRHR( mForceRHR ); return cloneSymbol; } @@ -1913,6 +1924,7 @@ QgsFillSymbol *QgsFillSymbol::clone() const cloneSymbol->setLayer( mLayer ); Q_NOWARN_DEPRECATED_POP cloneSymbol->setClipFeaturesToExtent( mClipFeaturesToExtent ); + cloneSymbol->setForceRHR( mForceRHR ); return cloneSymbol; } diff --git a/src/core/symbology/qgssymbol.h b/src/core/symbology/qgssymbol.h index 23b2c3b961ec..6187dd95fffe 100644 --- a/src/core/symbology/qgssymbol.h +++ b/src/core/symbology/qgssymbol.h @@ -379,6 +379,28 @@ class CORE_EXPORT QgsSymbol */ bool clipFeaturesToExtent() const { return mClipFeaturesToExtent; } + /** + * Sets whether polygon features drawn by the symbol should be reoriented to follow the + * standard right-hand-rule orientation, in which the area that is + * bounded by the polygon is to the right of the boundary. In particular, the exterior + * ring is oriented in a clockwise direction and the interior rings in a counter-clockwise + * direction. + * \see forceRHR() + * \since QGIS 3.6 + */ + void setForceRHR( bool force ) { mForceRHR = force; } + + /** + * Returns true if polygon features drawn by the symbol will be reoriented to follow the + * standard right-hand-rule orientation, in which the area that is + * bounded by the polygon is to the right of the boundary. In particular, the exterior + * ring is oriented in a clockwise direction and the interior rings in a counter-clockwise + * direction. + * \see setForceRHR() + * \since QGIS 3.6 + */ + bool forceRHR() const { return mForceRHR; } + /** * Returns a list of attributes required to render this feature. * This should include any attributes required by the symbology including @@ -447,14 +469,21 @@ class CORE_EXPORT QgsSymbol static QPolygonF _getLineString( QgsRenderContext &context, const QgsCurve &curve, bool clipToExtent = true ); /** - * Creates a polygon ring in screen coordinates from a QgsCurve in map coordinates + * Creates a polygon ring in screen coordinates from a QgsCurve in map coordinates. + * + * If \a correctRingOrientation is true then the ring will be oriented to match standard ring orientation, e.g. + * clockwise for exterior rings and counter-clockwise for interior rings. */ - static QPolygonF _getPolygonRing( QgsRenderContext &context, const QgsCurve &curve, bool clipToExtent ); + static QPolygonF _getPolygonRing( QgsRenderContext &context, const QgsCurve &curve, bool clipToExtent, bool isExteriorRing = false, bool correctRingOrientation = false ); /** * Creates a polygon in screen coordinates from a QgsPolygonXYin map coordinates + * + * If \a correctRingOrientation is true then the ring will be oriented to match standard ring orientation, e.g. + * clockwise for exterior rings and counter-clockwise for interior rings. + * */ - static void _getPolygon( QPolygonF &pts, QList &holes, QgsRenderContext &context, const QgsPolygon &polygon, bool clipToExtent = true ); + static void _getPolygon( QPolygonF &pts, QList &holes, QgsRenderContext &context, const QgsPolygon &polygon, bool clipToExtent = true, bool correctRingOrientation = false ); /** * Retrieve a cloned list of all layers that make up this symbol. @@ -487,6 +516,7 @@ class CORE_EXPORT QgsSymbol RenderHints mRenderHints = nullptr; bool mClipFeaturesToExtent = true; + bool mForceRHR = false; Q_DECL_DEPRECATED const QgsVectorLayer *mLayer = nullptr; //current vectorlayer diff --git a/src/core/symbology/qgssymbollayerutils.cpp b/src/core/symbology/qgssymbollayerutils.cpp index 443b0b0605ef..8df971c93dbf 100644 --- a/src/core/symbology/qgssymbollayerutils.cpp +++ b/src/core/symbology/qgssymbollayerutils.cpp @@ -960,7 +960,7 @@ QgsSymbol *QgsSymbolLayerUtils::loadSymbol( const QDomElement &element, const Qg } symbol->setOpacity( element.attribute( QStringLiteral( "alpha" ), QStringLiteral( "1.0" ) ).toDouble() ); symbol->setClipFeaturesToExtent( element.attribute( QStringLiteral( "clip_to_extent" ), QStringLiteral( "1" ) ).toInt() ); - + symbol->setForceRHR( element.attribute( QStringLiteral( "force_rhr" ), QStringLiteral( "0" ) ).toInt() ); return symbol; } @@ -1031,6 +1031,7 @@ QDomElement QgsSymbolLayerUtils::saveSymbol( const QString &name, QgsSymbol *sym symEl.setAttribute( QStringLiteral( "name" ), name ); symEl.setAttribute( QStringLiteral( "alpha" ), QString::number( symbol->opacity() ) ); symEl.setAttribute( QStringLiteral( "clip_to_extent" ), symbol->clipFeaturesToExtent() ? QStringLiteral( "1" ) : QStringLiteral( "0" ) ); + symEl.setAttribute( QStringLiteral( "force_rhr" ), symbol->forceRHR() ? QStringLiteral( "1" ) : QStringLiteral( "0" ) ); //QgsDebugMsg( "num layers " + QString::number( symbol->symbolLayerCount() ) ); for ( int i = 0; i < symbol->symbolLayerCount(); i++ ) diff --git a/src/gui/symbology/qgssymbolslistwidget.cpp b/src/gui/symbology/qgssymbolslistwidget.cpp index f09264cdf137..3330e56deff8 100644 --- a/src/gui/symbology/qgssymbolslistwidget.cpp +++ b/src/gui/symbology/qgssymbolslistwidget.cpp @@ -117,6 +117,9 @@ QgsSymbolsListWidget::QgsSymbolsListWidget( QgsSymbol *symbol, QgsStyle *style, mClipFeaturesAction = new QAction( tr( "Clip Features to Canvas Extent" ), this ); mClipFeaturesAction->setCheckable( true ); connect( mClipFeaturesAction, &QAction::toggled, this, &QgsSymbolsListWidget::clipFeaturesToggled ); + mStandardizeRingsAction = new QAction( tr( "Force Right-Hand-Rule Orientation" ), this ); + mStandardizeRingsAction->setCheckable( true ); + connect( mStandardizeRingsAction, &QAction::toggled, this, &QgsSymbolsListWidget::forceRHRToggled ); double iconSize = Qgis::UI_SCALE_FACTOR * fontMetrics().width( 'X' ) * 10; viewSymbols->setIconSize( QSize( static_cast< int >( iconSize ), static_cast< int >( iconSize * 0.9 ) ) ); // ~100, 90 on low dpi @@ -218,6 +221,7 @@ QgsSymbolsListWidget::~QgsSymbolsListWidget() // This action was added to the menu by this widget, clean it up // The menu can be passed in the constructor, so may live longer than this widget btnAdvanced->menu()->removeAction( mClipFeaturesAction ); + btnAdvanced->menu()->removeAction( mStandardizeRingsAction ); } void QgsSymbolsListWidget::registerDataDefinedButton( QgsPropertyOverrideButton *button, QgsSymbolLayer::Property key ) @@ -394,6 +398,15 @@ void QgsSymbolsListWidget::updateModelFilters() } } +void QgsSymbolsListWidget::forceRHRToggled( bool checked ) +{ + if ( !mSymbol ) + return; + + mSymbol->setForceRHR( checked ); + emit changed(); +} + void QgsSymbolsListWidget::openStyleManager() { // prefer to use global window manager to open the style manager, if possible! @@ -687,7 +700,7 @@ void QgsSymbolsListWidget::updateSymbolInfo() mOpacityWidget->setOpacity( mSymbol->opacity() ); - // Remove all previous clip actions + // Clean up previous advanced symbol actions const QList actionList( btnAdvanced->menu()->actions() ); for ( const auto &action : actionList ) { @@ -695,6 +708,10 @@ void QgsSymbolsListWidget::updateSymbolInfo() { btnAdvanced->menu()->removeAction( action ); } + else if ( mStandardizeRingsAction->text() == action->text() ) + { + btnAdvanced->menu()->removeAction( action ); + } } if ( mSymbol->type() == QgsSymbol::Line || mSymbol->type() == QgsSymbol::Fill ) @@ -702,12 +719,15 @@ void QgsSymbolsListWidget::updateSymbolInfo() //add clip features option for line or fill symbols btnAdvanced->menu()->addAction( mClipFeaturesAction ); } + if ( mSymbol->type() == QgsSymbol::Fill ) + { + btnAdvanced->menu()->addAction( mStandardizeRingsAction ); + } btnAdvanced->setVisible( mAdvancedMenu || !btnAdvanced->menu()->isEmpty() ); - mClipFeaturesAction->blockSignals( true ); - mClipFeaturesAction->setChecked( mSymbol->clipFeaturesToExtent() ); - mClipFeaturesAction->blockSignals( false ); + whileBlocking( mClipFeaturesAction )->setChecked( mSymbol->clipFeaturesToExtent() ); + whileBlocking( mStandardizeRingsAction )->setChecked( mSymbol->forceRHR() ); } void QgsSymbolsListWidget::setSymbolFromStyle( const QModelIndex &index ) diff --git a/src/gui/symbology/qgssymbolslistwidget.h b/src/gui/symbology/qgssymbolslistwidget.h index 9512fa3502f0..cfa3b98e0e18 100644 --- a/src/gui/symbology/qgssymbolslistwidget.h +++ b/src/gui/symbology/qgssymbolslistwidget.h @@ -118,6 +118,7 @@ class GUI_EXPORT QgsSymbolsListWidget : public QWidget, private Ui::SymbolsListW void opacityChanged( double value ); void createAuxiliaryField(); void updateModelFilters(); + void forceRHRToggled( bool checked ); private: QgsSymbol *mSymbol = nullptr; @@ -125,6 +126,7 @@ class GUI_EXPORT QgsSymbolsListWidget : public QWidget, private Ui::SymbolsListW QgsStyle *mStyle = nullptr; QMenu *mAdvancedMenu = nullptr; QAction *mClipFeaturesAction = nullptr; + QAction *mStandardizeRingsAction = nullptr; QgsVectorLayer *mLayer = nullptr; QgsMapCanvas *mMapCanvas = nullptr; QgsStyleProxyModel *mModel = nullptr; diff --git a/tests/src/python/test_qgssymbol.py b/tests/src/python/test_qgssymbol.py index 0043e5d1c858..0c84d777ba5f 100644 --- a/tests/src/python/test_qgssymbol.py +++ b/tests/src/python/test_qgssymbol.py @@ -27,8 +27,9 @@ from utilities import unitTestDataPath -from qgis.PyQt.QtCore import QDir +from qgis.PyQt.QtCore import QDir, Qt from qgis.PyQt.QtGui import QImage, QColor, QPainter +from qgis.PyQt.QtXml import QDomDocument from qgis.core import (QgsGeometry, QgsRectangle, @@ -47,9 +48,13 @@ QgsRenderChecker, QgsSimpleMarkerSymbolLayer, QgsSimpleMarkerSymbolLayerBase, + QgsSimpleFillSymbolLayer, QgsUnitTypes, QgsWkbTypes, - QgsProject + QgsProject, + QgsReadWriteContext, + QgsSymbolLayerUtils, + QgsMarkerLineSymbolLayer ) from qgis.testing import unittest, start_app @@ -623,5 +628,107 @@ def testSizeMapUnitScale(self): self.assertEqual(markerSymbol.symbolLayer(2).sizeMapUnitScale(), QgsMapUnitScale(3000, 4000)) +class TestQgsFillSymbol(unittest.TestCase): + + def setUp(self): + self.report = "

Python QgsFillSymbol Tests

\n" + + def tearDown(self): + report_file_path = "%s/qgistest.html" % QDir.tempPath() + with open(report_file_path, 'a') as report_file: + report_file.write(self.report) + + def testForceRHR(self): + # test forcing right hand rule during rendering + + s = QgsFillSymbol() + s.deleteSymbolLayer(0) + s.appendSymbolLayer( + QgsSimpleFillSymbolLayer(color=QColor(255, 0, 0), strokeColor=QColor(0, 255, 0))) + self.assertFalse(s.forceRHR()) + s.setForceRHR(True) + self.assertTrue(s.forceRHR()) + s.setForceRHR(False) + self.assertFalse(s.forceRHR()) + + s.setForceRHR(True) + doc = QDomDocument() + context = QgsReadWriteContext() + element = QgsSymbolLayerUtils.saveSymbol('test', s, doc, context) + + s2 = QgsSymbolLayerUtils.loadSymbol(element, context) + self.assertTrue(s2.forceRHR()) + + # rendering test + s3 = QgsFillSymbol() + s3.deleteSymbolLayer(0) + s3.appendSymbolLayer( + QgsSimpleFillSymbolLayer(color=QColor(255, 200, 200), strokeColor=QColor(0, 255, 0), strokeWidth=2)) + marker_line = QgsMarkerLineSymbolLayer(True) + marker_line.setPlacement(QgsMarkerLineSymbolLayer.FirstVertex) + marker = QgsSimpleMarkerSymbolLayer(QgsSimpleMarkerSymbolLayer.Triangle, 4) + marker.setColor(QColor(255, 0, 0)) + marker.setStrokeStyle(Qt.NoPen) + marker_symbol = QgsMarkerSymbol() + marker_symbol.changeSymbolLayer(0, marker) + marker_line.setSubSymbol(marker_symbol) + s3.appendSymbolLayer(marker_line) + + g = QgsGeometry.fromWkt('Polygon((0 0, 10 0, 10 10, 0 10, 0 0),(1 1, 1 2, 2 2, 2 1, 1 1),(8 8, 9 8, 9 9, 8 9, 8 8))') + rendered_image = self.renderGeometry(s3, g) + assert self.imageCheck('force_rhr_off', 'polygon_forcerhr_off', rendered_image) + + s3.setForceRHR(True) + rendered_image = self.renderGeometry(s3, g) + assert self.imageCheck('force_rhr_on', 'polygon_forcerhr_on', rendered_image) + + def renderGeometry(self, symbol, geom): + f = QgsFeature() + f.setGeometry(geom) + + image = QImage(200, 200, QImage.Format_RGB32) + + painter = QPainter() + ms = QgsMapSettings() + extent = geom.get().boundingBox() + # buffer extent by 10% + if extent.width() > 0: + extent = extent.buffered((extent.height() + extent.width()) / 20.0) + else: + extent = extent.buffered(10) + + ms.setExtent(extent) + ms.setOutputSize(image.size()) + context = QgsRenderContext.fromMapSettings(ms) + context.setPainter(painter) + context.setScaleFactor(96 / 25.4) # 96 DPI + + painter.begin(image) + try: + image.fill(QColor(0, 0, 0)) + symbol.startRender(context) + symbol.renderFeature(f, context) + symbol.stopRender(context) + finally: + painter.end() + + return image + + def imageCheck(self, name, reference_image, image): + self.report += "

Render {}

\n".format(name) + temp_dir = QDir.tempPath() + '/' + file_name = temp_dir + 'symbol_' + name + ".png" + image.save(file_name, "PNG") + checker = QgsRenderChecker() + checker.setControlPathPrefix("symbol") + checker.setControlName("expected_" + reference_image) + checker.setRenderedImage(file_name) + checker.setColorTolerance(2) + result = checker.compareImages(name, 20) + self.report += checker.report() + print((self.report)) + return result + + if __name__ == '__main__': unittest.main() diff --git a/tests/testdata/control_images/symbol/expected_polygon_forcerhr_off/expected_polygon_forcerhr_off.png b/tests/testdata/control_images/symbol/expected_polygon_forcerhr_off/expected_polygon_forcerhr_off.png new file mode 100644 index 000000000000..960cee9b3efa Binary files /dev/null and b/tests/testdata/control_images/symbol/expected_polygon_forcerhr_off/expected_polygon_forcerhr_off.png differ diff --git a/tests/testdata/control_images/symbol/expected_polygon_forcerhr_on/expected_polygon_forcerhr_on.png b/tests/testdata/control_images/symbol/expected_polygon_forcerhr_on/expected_polygon_forcerhr_on.png new file mode 100644 index 000000000000..9beb691b18b0 Binary files /dev/null and b/tests/testdata/control_images/symbol/expected_polygon_forcerhr_on/expected_polygon_forcerhr_on.png differ