Fix paint effects sometimes result in aliased rendering

python/PyQt6/core/auto_generated/effects/qgspainteffect.sip.in

@@ -149,7 +149,7 @@ Restores the effect to the state described by a DOM element.
 .. seealso:: :py:func:`saveProperties`
 %End
 
-    virtual void render( QPicture &picture, QgsRenderContext &context );
+    virtual void render( const QPicture &picture, QgsRenderContext &context );
 %Docstring
 Renders a picture using the effect.
 
@@ -246,7 +246,7 @@ to account for the destination painter's DPI.
 .. seealso:: :py:func:`sourceAsImage`
 %End
 
-    const QPicture *source() const;
+    const QPicture &source() const;
 %Docstring
 Returns the source QPicture. The :py:func:`~QgsPaintEffect.draw` member can utilize this when
 drawing the effect.
@@ -258,14 +258,14 @@ drawing the effect.
 .. seealso:: :py:func:`sourceAsImage`
 %End
 
-    QImage *sourceAsImage( QgsRenderContext &context );
+    QImage sourceAsImage( QgsRenderContext &context );
 %Docstring
 Returns the source QPicture rendered to a new QImage. The :py:func:`~QgsPaintEffect.draw` member can
 utilize this when drawing the effect. The image will be padded or cropped from the original
 source QPicture by the results of the :py:func:`~QgsPaintEffect.boundingRect` method.
 The result is cached to speed up subsequent calls to sourceAsImage.
 
-:return: source QPicture rendered to an image
+:return: source QPicture rendered to an image, or a null image if source could not be rendered
 
 .. seealso:: :py:func:`drawSource`
 

python/core/auto_generated/effects/qgspainteffect.sip.in

@@ -149,7 +149,7 @@ Restores the effect to the state described by a DOM element.
 .. seealso:: :py:func:`saveProperties`
 %End
 
-    virtual void render( QPicture &picture, QgsRenderContext &context );
+    virtual void render( const QPicture &picture, QgsRenderContext &context );
 %Docstring
 Renders a picture using the effect.
 
@@ -246,7 +246,7 @@ to account for the destination painter's DPI.
 .. seealso:: :py:func:`sourceAsImage`
 %End
 
-    const QPicture *source() const;
+    const QPicture &source() const;
 %Docstring
 Returns the source QPicture. The :py:func:`~QgsPaintEffect.draw` member can utilize this when
 drawing the effect.
@@ -258,14 +258,14 @@ drawing the effect.
 .. seealso:: :py:func:`sourceAsImage`
 %End
 
-    QImage *sourceAsImage( QgsRenderContext &context );
+    QImage sourceAsImage( QgsRenderContext &context );
 %Docstring
 Returns the source QPicture rendered to a new QImage. The :py:func:`~QgsPaintEffect.draw` member can
 utilize this when drawing the effect. The image will be padded or cropped from the original
 source QPicture by the results of the :py:func:`~QgsPaintEffect.boundingRect` method.
 The result is cached to speed up subsequent calls to sourceAsImage.
 
-:return: source QPicture rendered to an image
+:return: source QPicture rendered to an image, or a null image if source could not be rendered
 
 .. seealso:: :py:func:`drawSource`
 

src/core/effects/qgsblureffect.cpp

@@ -30,7 +30,7 @@ QgsPaintEffect *QgsBlurEffect::create( const QVariantMap &map )
 
 void QgsBlurEffect::draw( QgsRenderContext &context )
 {
-  if ( !source() || !enabled() || !context.painter() )
+  if ( !enabled() || !context.painter() || source().isNull() )
     return;
 
   switch ( mBlurMethod )
@@ -48,7 +48,7 @@ void QgsBlurEffect::drawStackBlur( QgsRenderContext &context )
 {
   const int blurLevel = std::round( context.convertToPainterUnits( mBlurLevel, mBlurUnit, mBlurMapUnitScale, Qgis::RenderSubcomponentProperty::BlurSize ) );
 
-  QImage im = sourceAsImage( context )->copy();
+  QImage im = sourceAsImage( context ).copy();
   QgsImageOperation::stackBlur( im, blurLevel, false, context.feedback() );
   drawBlurredImage( context, im );
 }
@@ -57,7 +57,8 @@ void QgsBlurEffect::drawGaussianBlur( QgsRenderContext &context )
 {
   const int blurLevel = std::round( context.convertToPainterUnits( mBlurLevel, mBlurUnit, mBlurMapUnitScale, Qgis::RenderSubcomponentProperty::BlurSize ) );
 
-  QImage *im = QgsImageOperation::gaussianBlur( *sourceAsImage( context ), blurLevel, context.feedback() );
+  QImage source = sourceAsImage( context ).copy();
+  QImage *im = QgsImageOperation::gaussianBlur( source, blurLevel, context.feedback() );
   if ( !im->isNull() )
     drawBlurredImage( context, *im );
   delete im;

src/core/effects/qgscoloreffect.cpp

@@ -37,13 +37,13 @@ QgsColorEffect::QgsColorEffect()
 
 void QgsColorEffect::draw( QgsRenderContext &context )
 {
-  if ( !source() || !enabled() || !context.painter() )
+  if ( !enabled() || !context.painter() || source().isNull() )
     return;
 
   QPainter *painter = context.painter();
 
   //rasterize source and apply modifications
-  QImage image = sourceAsImage( context )->copy();
+  QImage image = sourceAsImage( context ).copy();
 
   QgsImageOperation::adjustBrightnessContrast( image, mBrightness, mContrast / 100.0 + 1, context.feedback() );
 

src/core/effects/qgseffectstack.cpp

@@ -84,9 +84,10 @@ void QgsEffectStack::draw( QgsRenderContext &context )
   //first, we build up a list of rendered effects
   //we do this moving backwards through the stack, so that each effect's results
   //becomes the source of the previous effect
-  QPicture *sourcePic = new QPicture( *source() );
-  QPicture *currentPic = sourcePic;
-  QList< QPicture * > results;
+  const QPicture sourcePic = source();
+  const QPicture *currentPic = &sourcePic;
+  std::vector< QPicture > results;
+  results.reserve( mEffectList.count() );
   for ( int i = mEffectList.count() - 1; i >= 0; --i )
   {
     QgsPaintEffect *effect = mEffectList.at( i );
@@ -95,19 +96,19 @@ void QgsEffectStack::draw( QgsRenderContext &context )
       continue;
     }
 
-    QPicture *pic = nullptr;
+    const QPicture *pic = nullptr;
     if ( effect->type() == QLatin1String( "drawSource" ) )
     {
       //draw source is always the original source, regardless of previous effect results
-      pic = sourcePic;
+      pic = &sourcePic;
     }
     else
     {
       pic = currentPic;
     }
 
-    QPicture *resultPic = new QPicture();
-    QPainter p( resultPic );
+    QPicture resultPic;
+    QPainter p( &resultPic );
     context.setPainter( &p );
     //effect stack has it's own handling of the QPicture DPI issue, so
     //we disable QgsPaintEffect's internal workaround
@@ -116,14 +117,12 @@ void QgsEffectStack::draw( QgsRenderContext &context )
     effect->requiresQPainterDpiFix = true;
     p.end();
 
-    results << resultPic;
+    results.emplace_back( std::move( resultPic ) );
     if ( mEffectList.at( i )->drawMode() != QgsPaintEffect::Render )
     {
-      currentPic = resultPic;
+      currentPic = &results.back();
     }
   }
-  delete sourcePic;
-  sourcePic = nullptr;
 
   context.setPainter( destPainter );
   //then, we render all the results in the opposite order
@@ -134,12 +133,11 @@ void QgsEffectStack::draw( QgsRenderContext &context )
       continue;
     }
 
-    QPicture *pic = results.takeLast();
     if ( mEffectList.at( i )->drawMode() != QgsPaintEffect::Modifier )
     {
-      QgsPainting::drawPicture( context.painter(), QPointF( 0, 0 ), *pic );
+      QgsPainting::drawPicture( context.painter(), QPointF( 0, 0 ), results.back() );
     }
-    delete pic;
+    results.pop_back();
   }
 }
 

src/core/effects/qgsgloweffect.cpp

@@ -41,10 +41,10 @@ QgsGlowEffect::~QgsGlowEffect()
 
 void QgsGlowEffect::draw( QgsRenderContext &context )
 {
-  if ( !source() || !enabled() || !context.painter() )
+  if ( !enabled() || !context.painter() || source().isNull() )
     return;
 
-  QImage im = sourceAsImage( context )->copy();
+  QImage im = sourceAsImage( context ).copy();
 
   QgsColorRamp *ramp = nullptr;
   std::unique_ptr< QgsGradientColorRamp > tempRamp;
@@ -98,7 +98,7 @@ void QgsGlowEffect::draw( QgsRenderContext &context )
     QPainter p( &im );
     p.setRenderHint( QPainter::Antialiasing );
     p.setCompositionMode( QPainter::CompositionMode_DestinationIn );
-    p.drawImage( 0, 0, *sourceAsImage( context ) );
+    p.drawImage( 0, 0, sourceAsImage( context ) );
     p.end();
   }
 

src/core/effects/qgspainteffect.cpp

@@ -17,7 +17,6 @@
 
 #include "qgspainteffect.h"
 #include "qgsimageoperation.h"
-#include "qgslogger.h"
 #include "qgsrendercontext.h"
 #include "qgssymbollayerutils.h"
 #include "qgspainting.h"
@@ -33,12 +32,8 @@ QgsPaintEffect::QgsPaintEffect( const QgsPaintEffect &other )
 
 QgsPaintEffect::~QgsPaintEffect()
 {
-  if ( mOwnsImage )
-  {
-    delete mSourceImage;
-  }
-  delete mEffectPainter;
-  delete mTempPicture;
+  // ensure painter is destroyed before picture it may be drawing on, just in case
+  mEffectPainter.reset();
 }
 
 void QgsPaintEffect::setEnabled( const bool enabled )
@@ -76,12 +71,11 @@ bool QgsPaintEffect::readProperties( const QDomElement &element )
   return true;
 }
 
-void QgsPaintEffect::render( QPicture &picture, QgsRenderContext &context )
+void QgsPaintEffect::render( const QPicture &picture, QgsRenderContext &context )
 {
   //set source picture
-  mPicture = &picture;
-  delete mSourceImage;
-  mSourceImage = nullptr;
+  mPicture = picture;
+  mSourceImage = QImage();
 
   draw( context );
 }
@@ -91,14 +85,13 @@ void QgsPaintEffect::begin( QgsRenderContext &context )
   //temporarily replace painter and direct paint operations for context to a QPicture
   mPrevPainter = context.painter();
 
-  delete mTempPicture;
-  mTempPicture = new QPicture();
+  mTempPicture = std::make_unique< QPicture >();
 
-  delete mEffectPainter;
-  mEffectPainter = new QPainter();
-  mEffectPainter->begin( mTempPicture );
+  mEffectPainter = std::make_unique< QPainter >();
+  mEffectPainter->begin( mTempPicture.get() );
 
-  context.setPainter( mEffectPainter );
+  context.setPainterFlagsUsingContext( mEffectPainter.get() );
+  context.setPainter( mEffectPainter.get() );
 }
 
 void QgsPaintEffect::end( QgsRenderContext &context )
@@ -107,8 +100,7 @@ void QgsPaintEffect::end( QgsRenderContext &context )
     return;
 
   mEffectPainter->end();
-  delete mEffectPainter;
-  mEffectPainter = nullptr;
+  mEffectPainter.reset();
 
   //restore previous painter for context
   context.setPainter( mPrevPainter );
@@ -123,44 +115,42 @@ void QgsPaintEffect::end( QgsRenderContext &context )
   render( *mTempPicture, context );
 
   //clean up
-  delete mTempPicture;
-  mTempPicture = nullptr;
+  mTempPicture.reset();
 }
 
 void QgsPaintEffect::drawSource( QPainter &painter )
 {
   if ( requiresQPainterDpiFix )
   {
-    QgsPainting::drawPicture( &painter, QPointF( 0, 0 ), *mPicture );
+    QgsPainting::drawPicture( &painter, QPointF( 0, 0 ), mPicture );
   }
   else
   {
-    painter.drawPicture( 0, 0, *mPicture );
+    painter.drawPicture( 0, 0, mPicture );
   }
 }
 
-QImage *QgsPaintEffect::sourceAsImage( QgsRenderContext &context )
+QImage QgsPaintEffect::sourceAsImage( QgsRenderContext &context )
 {
   //have we already created a source image? if so, return it
-  if ( mSourceImage )
+  if ( !mSourceImage.isNull() )
   {
     return mSourceImage;
   }
 
-  if ( !mPicture )
-    return nullptr;
+  if ( mPicture.isNull() )
+    return QImage();
 
   //else create it
   //TODO - test with premultiplied image for speed
   const QRectF bounds = imageBoundingRect( context );
-  mSourceImage = new QImage( bounds.width(), bounds.height(), QImage::Format_ARGB32 );
-  mSourceImage->fill( Qt::transparent );
-  QPainter imagePainter( mSourceImage );
+  mSourceImage = QImage( bounds.width(), bounds.height(), QImage::Format_ARGB32 );
+  mSourceImage.fill( Qt::transparent );
+  QPainter imagePainter( &mSourceImage );
   imagePainter.setRenderHint( QPainter::Antialiasing );
   imagePainter.translate( -bounds.left(), -bounds.top() );
-  imagePainter.drawPicture( 0, 0, *mPicture );
+  imagePainter.drawPicture( 0, 0, mPicture );
   imagePainter.end();
-  mOwnsImage = true;
   return mSourceImage;
 }
 
@@ -182,7 +172,7 @@ void QgsPaintEffect::fixQPictureDpi( QPainter *painter ) const
 
 QRectF QgsPaintEffect::imageBoundingRect( const QgsRenderContext &context ) const
 {
-  return boundingRect( mPicture->boundingRect(), context );
+  return boundingRect( mPicture.boundingRect(), context );
 }
 
 
@@ -212,7 +202,7 @@ void QgsDrawSourceEffect::draw( QgsRenderContext &context )
   else
   {
     //rasterize source and apply modifications
-    QImage image = sourceAsImage( context )->copy();
+    QImage image = sourceAsImage( context ).copy();
     QgsImageOperation::multiplyOpacity( image, mOpacity, context.feedback() );
     const QgsScopedQPainterState painterState( painter );
     painter->setCompositionMode( mBlendMode );