Fix "feature rendering order" checkbox is not honored (fix #14323)

python/core/symbology-ng/qgsrendererv2.sip

@@ -352,15 +352,36 @@ class QgsFeatureRendererV2
     /**
      * Get the order in which features shall be processed by this renderer.
      * @note added in QGIS 2.14
+     * @note this property has no effect if orderByEnabled() is false
+     * @see orderByEnabled()
      */
     QgsFeatureRequest::OrderBy orderBy() const;
 
     /**
      * Define the order in which features shall be processed by this renderer.
+     * @note this property has no effect if orderByEnabled() is false
      * @note added in QGIS 2.14
+     * @see setOrderByEnabled()
      */
     void setOrderBy( const QgsFeatureRequest::OrderBy& orderBy );
 
+    /**
+     * Returns whether custom ordering will be applied before features are processed by this renderer.
+     * @note added in QGIS 2.14
+     * @see orderBy()
+     * @see setOrderByEnabled()
+     */
+    bool orderByEnabled() const;
+
+    /**
+     * Sets whether custom ordering should be applied before features are processed by this renderer.
+     * @param enabled set to true to enable custom feature ordering
+     * @note added in QGIS 2.14
+     * @see setOrderBy()
+     * @see orderByEnabled()
+     */
+    void setOrderByEnabled( bool enabled );
+
   protected:
     QgsFeatureRendererV2( const QString& type );
 

src/core/qgsvectorlayerrenderer.cpp

@@ -150,13 +150,14 @@ bool QgsVectorLayerRenderer::render()
   QgsRectangle requestExtent = mContext.extent();
   mRendererV2->modifyRequestExtent( requestExtent, mContext );
 
-  QgsFeatureRequest::OrderBy orderBy = mRendererV2->orderBy();
-
   QgsFeatureRequest featureRequest = QgsFeatureRequest()
                                      .setFilterRect( requestExtent )
                                      .setSubsetOfAttributes( mAttrNames, mFields )
-                                     .setExpressionContext( mContext.expressionContext() )
-                                     .setOrderBy( orderBy );
+                                     .setExpressionContext( mContext.expressionContext() );
+  if ( mRendererV2->orderByEnabled() )
+  {
+    featureRequest.setOrderBy( mRendererV2->orderBy() );
+  }
 
   const QgsFeatureFilterProvider* featureFilterProvider = mContext.featureFilterProvider();
   if ( featureFilterProvider )

src/core/symbology-ng/qgscategorizedsymbolrendererv2.cpp

@@ -784,6 +784,7 @@ QDomElement QgsCategorizedSymbolRendererV2::save( QDomDocument& doc )
     mOrderBy.save( orderBy );
     rendererElem.appendChild( orderBy );
   }
+  rendererElem.setAttribute( "enableorderby", ( mOrderByEnabled ? "1" : "0" ) );
 
   return rendererElem;
 }
@@ -1052,6 +1053,7 @@ QgsCategorizedSymbolRendererV2* QgsCategorizedSymbolRendererV2::convertFromRende
   }
 
   r->setOrderBy( renderer->orderBy() );
+  r->setOrderByEnabled( renderer->orderByEnabled() );
 
   return r;
 }

src/core/symbology-ng/qgsgraduatedsymbolrendererv2.cpp

@@ -1201,6 +1201,7 @@ QDomElement QgsGraduatedSymbolRendererV2::save( QDomDocument& doc )
     mOrderBy.save( orderBy );
     rendererElem.appendChild( orderBy );
   }
+  rendererElem.setAttribute( "enableorderby", ( mOrderByEnabled ? "1" : "0" ) );
 
   return rendererElem;
 }
@@ -1745,6 +1746,7 @@ QgsGraduatedSymbolRendererV2* QgsGraduatedSymbolRendererV2::convertFromRenderer(
   }
 
   r->setOrderBy( renderer->orderBy() );
+  r->setOrderByEnabled( renderer->orderByEnabled() );
 
   return r;
 }

src/core/symbology-ng/qgsheatmaprenderer.cpp

@@ -361,6 +361,7 @@ QDomElement QgsHeatmapRenderer::save( QDomDocument& doc )
     rendererElem.appendChild( colorRampElem );
   }
   rendererElem.setAttribute( "invert_ramp", QString::number( mInvertRamp ) );
+  rendererElem.setAttribute( "forceraster", ( mForceRaster ? "1" : "0" ) );
 
   if ( mPaintEffect && !QgsPaintEffectRegistry::isDefaultStack( mPaintEffect ) )
     mPaintEffect->saveProperties( doc, rendererElem );
@@ -371,6 +372,7 @@ QDomElement QgsHeatmapRenderer::save( QDomDocument& doc )
     mOrderBy.save( orderBy );
     rendererElem.appendChild( orderBy );
   }
+  rendererElem.setAttribute( "enableorderby", ( mOrderByEnabled ? "1" : "0" ) );
 
   return rendererElem;
 }

src/core/symbology-ng/qgsinvertedpolygonrenderer.cpp

@@ -385,6 +385,7 @@ QDomElement QgsInvertedPolygonRenderer::save( QDomDocument& doc )
     mOrderBy.save( orderBy );
     rendererElem.appendChild( orderBy );
   }
+  rendererElem.setAttribute( "enableorderby", ( mOrderByEnabled ? "1" : "0" ) );
 
   return rendererElem;
 }

src/core/symbology-ng/qgspointdisplacementrenderer.cpp

@@ -426,6 +426,7 @@ QDomElement QgsPointDisplacementRenderer::save( QDomDocument& doc )
     mOrderBy.save( orderBy );
     rendererElement.appendChild( orderBy );
   }
+  rendererElement.setAttribute( "enableorderby", ( mOrderByEnabled ? "1" : "0" ) );
 
   return rendererElement;
 }

src/core/symbology-ng/qgsrendererv2.cpp

@@ -195,6 +195,7 @@ void QgsFeatureRendererV2::copyRendererData( QgsFeatureRendererV2* destRenderer
 
   destRenderer->setPaintEffect( mPaintEffect->clone() );
   destRenderer->mOrderBy = mOrderBy;
+  destRenderer->mOrderByEnabled = mOrderByEnabled;
 }
 
 void QgsFeatureRendererV2::copyPaintEffect( QgsFeatureRendererV2 *destRenderer ) const
@@ -212,6 +213,7 @@ QgsFeatureRendererV2::QgsFeatureRendererV2( const QString& type )
     , mCurrentVertexMarkerSize( 3 )
     , mPaintEffect( nullptr )
     , mForceRaster( false )
+    , mOrderByEnabled( false )
 {
   mPaintEffect = QgsPaintEffectRegistry::defaultStack();
   mPaintEffect->setEnabled( false );
@@ -334,6 +336,7 @@ QgsFeatureRendererV2* QgsFeatureRendererV2::load( QDomElement& element )
     // restore order by
     QDomElement orderByElem = element.firstChildElement( "orderby" );
     r->mOrderBy.load( orderByElem );
+    r->setOrderByEnabled( element.attribute( "enableorderby", "0" ).toInt() );
   }
   return r;
 }
@@ -353,6 +356,7 @@ QDomElement QgsFeatureRendererV2::save( QDomDocument& doc )
     mOrderBy.save( orderBy );
     rendererElem.appendChild( orderBy );
   }
+  rendererElem.setAttribute( "enableorderby", ( mOrderByEnabled ? "1" : "0" ) );
   return rendererElem;
 }
 
@@ -617,6 +621,16 @@ void QgsFeatureRendererV2::setOrderBy( const QgsFeatureRequest::OrderBy& orderBy
   mOrderBy = orderBy;
 }
 
+bool QgsFeatureRendererV2::orderByEnabled() const
+{
+  return mOrderByEnabled;
+}
+
+void QgsFeatureRendererV2::setOrderByEnabled( bool enabled )
+{
+  mOrderByEnabled = enabled;
+}
+
 void QgsFeatureRendererV2::convertSymbolSizeScale( QgsSymbolV2 * symbol, QgsSymbolV2::ScaleMethod method, const QString & field )
 {
   if ( symbol->type() == QgsSymbolV2::Marker )

src/core/symbology-ng/qgsrendererv2.h

@@ -375,15 +375,36 @@ class CORE_EXPORT QgsFeatureRendererV2
     /**
      * Get the order in which features shall be processed by this renderer.
      * @note added in QGIS 2.14
+     * @note this property has no effect if orderByEnabled() is false
+     * @see orderByEnabled()
      */
     QgsFeatureRequest::OrderBy orderBy() const;
 
     /**
      * Define the order in which features shall be processed by this renderer.
+     * @note this property has no effect if orderByEnabled() is false
      * @note added in QGIS 2.14
+     * @see setOrderByEnabled()
      */
     void setOrderBy( const QgsFeatureRequest::OrderBy& orderBy );
 
+    /**
+     * Returns whether custom ordering will be applied before features are processed by this renderer.
+     * @note added in QGIS 2.14
+     * @see orderBy()
+     * @see setOrderByEnabled()
+     */
+    bool orderByEnabled() const;
+
+    /**
+     * Sets whether custom ordering should be applied before features are processed by this renderer.
+     * @param enabled set to true to enable custom feature ordering
+     * @note added in QGIS 2.14
+     * @see setOrderBy()
+     * @see orderByEnabled()
+     */
+    void setOrderByEnabled( bool enabled );
+
   protected:
     QgsFeatureRendererV2( const QString& type );
 
@@ -447,6 +468,8 @@ class CORE_EXPORT QgsFeatureRendererV2
 
     QgsFeatureRequest::OrderBy mOrderBy;
 
+    bool mOrderByEnabled;
+
   private:
     Q_DISABLE_COPY( QgsFeatureRendererV2 )
 };

src/core/symbology-ng/qgsrulebasedrendererv2.cpp

@@ -993,6 +993,7 @@ QDomElement QgsRuleBasedRendererV2::save( QDomDocument& doc )
     mOrderBy.save( orderBy );
     rendererElem.appendChild( orderBy );
   }
+  rendererElem.setAttribute( "enableorderby", ( mOrderByEnabled ? "1" : "0" ) );
 
   return rendererElem;
 }

src/core/symbology-ng/qgssinglesymbolrendererv2.cpp

@@ -387,6 +387,8 @@ QDomElement QgsSingleSymbolRendererV2::save( QDomDocument& doc )
     mOrderBy.save( orderBy );
     rendererElem.appendChild( orderBy );
   }
+  rendererElem.setAttribute( "enableorderby", ( mOrderByEnabled ? "1" : "0" ) );
+
   return rendererElem;
 }
 
@@ -481,6 +483,7 @@ QgsSingleSymbolRendererV2* QgsSingleSymbolRendererV2::convertFromRenderer( const
   if ( r )
   {
     r->setOrderBy( renderer->orderBy() );
+    r->setOrderByEnabled( renderer->orderByEnabled() );
   }
 
   return r;

src/gui/symbology-ng/qgsrendererv2propertiesdialog.cpp

@@ -136,16 +136,16 @@ QgsRendererV2PropertiesDialog::QgsRendererV2PropertiesDialog( QgsVectorLayer* la
   // setup slot rendererChanged()
   connect( cboRenderers, SIGNAL( currentIndexChanged( int ) ), this, SLOT( rendererChanged() ) );
   //setup order by
-  if ( mOrderBy.isEmpty() )
+  if ( mLayer->rendererV2()->orderByEnabled() )
+  {
+    checkboxEnableOrderBy->setChecked( true );
+  }
+  else
   {
     btnOrderBy->setEnabled( false );
     checkboxEnableOrderBy->setChecked( false );
     lineEditOrderBy->setEnabled( false );
   }
-  else
-  {
-    checkboxEnableOrderBy->setChecked( true );
-  }
   lineEditOrderBy->setReadOnly( true );
   connect( checkboxEnableOrderBy, SIGNAL( toggled( bool ) ), btnOrderBy, SLOT( setEnabled( bool ) ) );
   connect( checkboxEnableOrderBy, SIGNAL( toggled( bool ) ), lineEditOrderBy, SLOT( setEnabled( bool ) ) );
@@ -222,7 +222,7 @@ void QgsRendererV2PropertiesDialog::rendererChanged()
     {
       if ( mMapCanvas )
         mActiveWidget->setMapCanvas( mMapCanvas );
-      changeOrderBy( mActiveWidget->renderer()->orderBy() );
+      changeOrderBy( mActiveWidget->renderer()->orderBy(), mActiveWidget->renderer()->orderByEnabled() );
       connect( mActiveWidget, SIGNAL( layerVariablesChanged() ), this, SIGNAL( layerVariablesChanged() ) );
     }
   }
@@ -248,6 +248,7 @@ void QgsRendererV2PropertiesDialog::apply()
     renderer->setPaintEffect( mPaintEffect->clone() );
     // set the order by
     renderer->setOrderBy( mOrderBy );
+    renderer->setOrderByEnabled( checkboxEnableOrderBy->isChecked() );
 
     mLayer->setRendererV2( renderer->clone() );
   }
@@ -278,11 +279,11 @@ void QgsRendererV2PropertiesDialog::showOrderByDialog()
   }
 }
 
-void QgsRendererV2PropertiesDialog::changeOrderBy( const QgsFeatureRequest::OrderBy& orderBy )
+void QgsRendererV2PropertiesDialog::changeOrderBy( const QgsFeatureRequest::OrderBy& orderBy, bool orderByEnabled )
 {
   mOrderBy = orderBy;
   lineEditOrderBy->setText( mOrderBy.dump() );
-  checkboxEnableOrderBy->setChecked( !orderBy.isEmpty() );
+  checkboxEnableOrderBy->setChecked( orderByEnabled );
 }
 
 

src/gui/symbology-ng/qgsrendererv2propertiesdialog.h

@@ -65,7 +65,7 @@ class GUI_EXPORT QgsRendererV2PropertiesDialog : public QDialog, private Ui::Qgs
   private slots:
     void showOrderByDialog();
 
-    void changeOrderBy( const QgsFeatureRequest::OrderBy& orderBy );
+    void changeOrderBy( const QgsFeatureRequest::OrderBy& orderBy, bool orderByEnabled );
 
   protected:
 

tests/src/python/test_qgssinglesymbolrenderer.py

@@ -81,14 +81,19 @@ class TestQgsSingleSymbolRenderer(unittest.TestCase):
 
     def testOrderBy(self):
         self.renderer.setOrderBy(QgsFeatureRequest.OrderBy([QgsFeatureRequest.OrderByClause('Value', False)]))
+        self.renderer.setOrderByEnabled(True)
 
         # Setup rendering check
         renderchecker = QgsMultiRenderChecker()
         renderchecker.setMapSettings(self.mapsettings)
         renderchecker.setControlName('expected_singlesymbol_orderby')
         result = renderchecker.runTest('singlesymbol_orderby')
-
         assert result
 
+        # disable order by and retest
+        self.renderer.setOrderByEnabled(False)
+        result = renderchecker.runTest('single')
+
+
 if __name__ == '__main__':
     unittest.main()