Skip to content

Commit

Permalink
[symbology] Don't force rasterized output when exporting point patter…
Browse files Browse the repository at this point in the history 
…n fills

This avoids the force conversion to a raster based pattern which currently occurs
when exporting maps/layouts to a vector format (e.g. PDF). The raster pattern
results in considerable quality loss, and the tiling edges of the raster brush
can sometimes be seen in outputs.

Additionally, fixes render corrupt when marker subsymbols have data defined properties
which affect the marker shape, such as data defined rotation or sizes

Refs #16100 (still needs fixing for line fill symbols)
  • Loading branch information
@nyalldawson
nyalldawson committed May 28, 2020
1 parent b0e71a8 commit 23396b7
Show file tree
Hide file tree
Showing 9 changed files with 256 additions and 9 deletions.
3 changes: 2 additions & 1 deletion python/core/auto_generated/symbology/qgsfillsymbollayer.sip.in
View file
Expand Up @@ -1615,9 +1615,10 @@ Caller takes ownership of the returned symbol layer.

virtual void startRender( QgsSymbolRenderContext &context );


virtual void stopRender( QgsSymbolRenderContext &context );

virtual void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context );


virtual QgsStringMap properties() const;

Expand Down
149 changes: 146 additions & 3 deletions src/core/symbology/qgsfillsymbollayer.cpp
View file
Expand Up @@ -3276,7 +3276,19 @@ void QgsPointPatternFillSymbolLayer::applyPattern( const QgsSymbolRenderContext

void QgsPointPatternFillSymbolLayer::startRender( QgsSymbolRenderContext &context )
{
applyPattern( context, mBrush, mDistanceX, mDistanceY, mDisplacementX, mDisplacementY, mOffsetX, mOffsetY );
// if we are using a vector based output, we need to render points as vectors
// (OR if the marker has data defined symbology, in which case we need to evaluate this point-by-point)
mRenderUsingMarkers = context.renderContext().forceVectorOutput() || mMarkerSymbol->hasDataDefinedProperties();

if ( mRenderUsingMarkers )
{
mMarkerSymbol->startRender( context.renderContext() );
}
else
{
// optimised render for screen only, use image based brush
applyPattern( context, mBrush, mDistanceX, mDistanceY, mDisplacementX, mDisplacementY, mOffsetX, mOffsetY );
}

if ( mStroke )
{
Expand All @@ -3286,12 +3298,142 @@ void QgsPointPatternFillSymbolLayer::startRender( QgsSymbolRenderContext &contex

void QgsPointPatternFillSymbolLayer::stopRender( QgsSymbolRenderContext &context )
{
if ( mRenderUsingMarkers )
{
mMarkerSymbol->stopRender( context.renderContext() );
}

if ( mStroke )
{
mStroke->stopRender( context.renderContext() );
}
}

void QgsPointPatternFillSymbolLayer::renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context )
{
if ( !mRenderUsingMarkers )
{
// use image based brush for speed
QgsImageFillSymbolLayer::renderPolygon( points, rings, context );
return;
}

// vector based output - so draw dot by dot!
QPainter *p = context.renderContext().painter();
if ( !p )
{
return;
}

double distanceX = mDistanceX;
if ( mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyDistanceX ) )
{
context.setOriginalValueVariable( mDistanceX );
distanceX = mDataDefinedProperties.valueAsDouble( QgsSymbolLayer::PropertyDistanceX, context.renderContext().expressionContext(), mDistanceX );
}
const double width = context.renderContext().convertToPainterUnits( distanceX, mDistanceXUnit, mDistanceXMapUnitScale );

double distanceY = mDistanceY;
if ( mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyDistanceY ) )
{
context.setOriginalValueVariable( mDistanceY );
distanceY = mDataDefinedProperties.valueAsDouble( QgsSymbolLayer::PropertyDistanceY, context.renderContext().expressionContext(), mDistanceY );
}
const double height = context.renderContext().convertToPainterUnits( distanceY, mDistanceYUnit, mDistanceYMapUnitScale );

double offsetX = mOffsetX;
if ( mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyOffsetX ) )
{
context.setOriginalValueVariable( mOffsetX );
offsetX = mDataDefinedProperties.valueAsDouble( QgsSymbolLayer::PropertyOffsetX, context.renderContext().expressionContext(), mOffsetX );
}
const double widthOffset = std::fmod( context.renderContext().convertToPainterUnits( offsetX, mOffsetXUnit, mOffsetXMapUnitScale ), width );

double offsetY = mOffsetY;
if ( mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyOffsetY ) )
{
context.setOriginalValueVariable( mOffsetY );
offsetY = mDataDefinedProperties.valueAsDouble( QgsSymbolLayer::PropertyOffsetY, context.renderContext().expressionContext(), mOffsetY );
}
const double heightOffset = std::fmod( context.renderContext().convertToPainterUnits( offsetY, mOffsetYUnit, mOffsetYMapUnitScale ), height );

double displacementX = mDisplacementX;
if ( mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyDisplacementX ) )
{
context.setOriginalValueVariable( mDisplacementX );
displacementX = mDataDefinedProperties.valueAsDouble( QgsSymbolLayer::PropertyDisplacementX, context.renderContext().expressionContext(), mDisplacementX );
}
const double displacementPixelX = context.renderContext().convertToPainterUnits( displacementX, mDisplacementXUnit, mDisplacementXMapUnitScale );

double displacementY = mDisplacementY;
if ( mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyDisplacementY ) )
{
context.setOriginalValueVariable( mDisplacementY );
displacementY = mDataDefinedProperties.valueAsDouble( QgsSymbolLayer::PropertyDisplacementY, context.renderContext().expressionContext(), mDisplacementY );
}
const double displacementPixelY = context.renderContext().convertToPainterUnits( displacementY, mDisplacementYUnit, mDisplacementYMapUnitScale );

p->setPen( QPen( Qt::NoPen ) );

if ( context.selected() )
{
QColor selColor = context.renderContext().selectionColor();
p->setBrush( QBrush( selColor ) );
_renderPolygon( p, points, rings, context );
}

p->save();

QPainterPath path;
path.addPolygon( points );
if ( rings )
{
for ( const QPolygonF &ring : *rings )
{
path.addPolygon( ring );
}
}
p->setClipPath( path, Qt::IntersectClip );

const double left = points.boundingRect().left();
const double top = points.boundingRect().top();
const double right = points.boundingRect().right();
const double bottom = points.boundingRect().bottom();

bool alternateColumn = false;
for ( double currentX = ( std::floor( left / width ) - 2 ) * width; currentX <= right + 2 * width; currentX += width, alternateColumn = !alternateColumn )
{
bool alternateRow = false;
const double columnX = currentX + widthOffset;
for ( double currentY = ( std::floor( top / height ) - 2 ) * height; currentY <= bottom + 2 * height; currentY += height, alternateRow = !alternateRow )
{
double y = currentY + heightOffset;
double x = columnX;
if ( alternateRow )
x += displacementPixelX;

if ( !alternateColumn )
y -= displacementPixelY;

mMarkerSymbol->renderPoint( QPointF( x, y ), context.feature(), context.renderContext() );
}
}

p->restore();

if ( mStroke )
{
mStroke->renderPolyline( points, context.feature(), context.renderContext(), -1, SELECT_FILL_BORDER && context.selected() );
if ( rings )
{
for ( auto ringIt = rings->constBegin(); ringIt != rings->constEnd(); ++ringIt )
{
mStroke->renderPolyline( *ringIt, context.feature(), context.renderContext(), -1, SELECT_FILL_BORDER && context.selected() );
}
}
}
}

QgsStringMap QgsPointPatternFillSymbolLayer::properties() const
{
QgsStringMap map;
Expand Down Expand Up @@ -3395,6 +3537,7 @@ void QgsPointPatternFillSymbolLayer::applyDataDefinedSettings( QgsSymbolRenderCo
{
if ( !mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyDistanceX ) && !mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyDistanceY )
&& !mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyDisplacementX ) && !mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyDisplacementY )
&& !mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyOffsetX ) && !mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyOffsetY )
&& ( !mMarkerSymbol || !mMarkerSymbol->hasDataDefinedProperties() ) )
{
return;
Expand Down Expand Up @@ -3427,13 +3570,13 @@ void QgsPointPatternFillSymbolLayer::applyDataDefinedSettings( QgsSymbolRenderCo
double offsetX = mOffsetX;
if ( mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyOffsetX ) )
{
context.setOriginalValueVariable( mDisplacementX );
context.setOriginalValueVariable( mOffsetX );
offsetX = mDataDefinedProperties.valueAsDouble( QgsSymbolLayer::PropertyOffsetX, context.renderContext().expressionContext(), mOffsetX );
}
double offsetY = mOffsetY;
if ( mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyOffsetY ) )
{
context.setOriginalValueVariable( mDisplacementY );
context.setOriginalValueVariable( mOffsetY );
offsetY = mDataDefinedProperties.valueAsDouble( QgsSymbolLayer::PropertyOffsetY, context.renderContext().expressionContext(), mOffsetY );
}
applyPattern( context, mBrush, distanceX, distanceY, displacementX, displacementY, offsetX, offsetY );
Expand Down
4 changes: 3 additions & 1 deletion src/core/symbology/qgsfillsymbollayer.h
View file
Expand Up @@ -1478,8 +1478,8 @@ class CORE_EXPORT QgsPointPatternFillSymbolLayer: public QgsImageFillSymbolLayer
QString layerType() const override;

void startRender( QgsSymbolRenderContext &context ) override;

void stopRender( QgsSymbolRenderContext &context ) override;
void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;

QgsStringMap properties() const override;

Expand Down Expand Up @@ -1720,6 +1720,8 @@ class CORE_EXPORT QgsPointPatternFillSymbolLayer: public QgsImageFillSymbolLayer

void applyPattern( const QgsSymbolRenderContext &context, QBrush &brush, double distanceX, double distanceY,
double displacementX, double displacementY, double offsetX, double offsetY );

bool mRenderUsingMarkers = false;
};

/**
Expand Down
2 changes: 2 additions & 0 deletions src/core/symbology/qgssymbollayer.cpp
View file
Expand Up @@ -84,6 +84,8 @@ void QgsSymbolLayer::initPropertyDefinitions()
{ QgsSymbolLayer::PropertyDistanceY, QgsPropertyDefinition( "distanceY", QObject::tr( "Vertical distance between markers" ), QgsPropertyDefinition::DoublePositive, origin )},
{ QgsSymbolLayer::PropertyDisplacementX, QgsPropertyDefinition( "displacementX", QObject::tr( "Horizontal displacement between rows" ), QgsPropertyDefinition::DoublePositive, origin )},
{ QgsSymbolLayer::PropertyDisplacementY, QgsPropertyDefinition( "displacementY", QObject::tr( "Vertical displacement between columns" ), QgsPropertyDefinition::DoublePositive, origin )},
{ QgsSymbolLayer::PropertyOffsetX, QgsPropertyDefinition( "offsetX", QObject::tr( "Horizontal offset" ), QgsPropertyDefinition::Double, origin )},
{ QgsSymbolLayer::PropertyOffsetY, QgsPropertyDefinition( "offsetY", QObject::tr( "Vertical offset" ), QgsPropertyDefinition::Double, origin )},
{ QgsSymbolLayer::PropertyOpacity, QgsPropertyDefinition( "alpha", QObject::tr( "Opacity" ), QgsPropertyDefinition::Opacity, origin )},
{ QgsSymbolLayer::PropertyCustomDash, QgsPropertyDefinition( "customDash", QgsPropertyDefinition::DataTypeString, QObject::tr( "Custom dash pattern" ), QObject::tr( "[<b><dash>;<space></b>] e.g. '8;2;1;2'" ), origin )},
{ QgsSymbolLayer::PropertyCapStyle, QgsPropertyDefinition( "capStyle", QObject::tr( "Line cap style" ), QgsPropertyDefinition::CapStyle, origin )},
Expand Down
107 changes: 103 additions & 4 deletions tests/src/core/testqgspointpatternfillsymbol.cpp
View file
Expand Up @@ -20,6 +20,7 @@
#include <QFileInfo>
#include <QDir>
#include <QDesktopServices>
#include <QSvgGenerator>

//qgis includes...
#include <qgsmaplayer.h>
Expand All @@ -35,6 +36,7 @@

//qgis test includes
#include "qgsrenderchecker.h"
#include "qgsmaprenderercustompainterjob.h"

/**
* \ingroup UnitTests
Expand All @@ -54,9 +56,12 @@ class TestQgsPointPatternFillSymbol : public QObject
void cleanup() {} // will be called after every testfunction.

void pointPatternFillSymbol();
void pointPatternFillSymbolVector();
void offsettedPointPatternFillSymbol();
void offsettedPointPatternFillSymbolVector();
void dataDefinedSubSymbol();
void zeroSpacedPointPatternFillSymbol();
void zeroSpacedPointPatternFillSymbolVector();

private:
bool mTestHasError = false ;
Expand Down Expand Up @@ -107,10 +112,6 @@ void TestQgsPointPatternFillSymbol::initTestCase()
mSymbolRenderer = new QgsSingleSymbolRenderer( mFillSymbol );
mpPolysLayer->setRenderer( mSymbolRenderer );

// We only need maprender instead of mapcanvas
// since maprender does not require a qui
// and is more light weight
//
mMapSettings.setLayers( QList<QgsMapLayer *>() << mpPolysLayer );
mReport += QLatin1String( "<h1>Point Pattern Fill Tests</h1>\n" );

Expand Down Expand Up @@ -144,6 +145,52 @@ void TestQgsPointPatternFillSymbol::pointPatternFillSymbol()
QVERIFY( imageCheck( "symbol_pointfill" ) );
}

void TestQgsPointPatternFillSymbol::pointPatternFillSymbolVector()
{
mReport += QLatin1String( "<h2>Point pattern fill symbol renderer test</h2>\n" );

QgsStringMap properties;
properties.insert( QStringLiteral( "color" ), QStringLiteral( "0,0,0,255" ) );
properties.insert( QStringLiteral( "outline_color" ), QStringLiteral( "#000000" ) );
properties.insert( QStringLiteral( "name" ), QStringLiteral( "circle" ) );
properties.insert( QStringLiteral( "size" ), QStringLiteral( "5.0" ) );
QgsMarkerSymbol *pointSymbol = QgsMarkerSymbol::createSimple( properties );

mPointPatternFill->setSubSymbol( pointSymbol );
mMapSettings.setFlag( QgsMapSettings::ForceVectorOutput, true );
bool res = imageCheck( "symbol_pointfill_vector" );
mMapSettings.setFlag( QgsMapSettings::ForceVectorOutput, false );
QVERIFY( res );

// also confirm that output is indeed vector!
QSvgGenerator generator;
generator.setResolution( mMapSettings.outputDpi() );
generator.setSize( QSize( 100, 100 ) );
generator.setViewBox( QRect( 0, 0, 100, 100 ) );
QBuffer buffer;
generator.setOutputDevice( &buffer );
QPainter p;
p.begin( &generator );

mMapSettings.setFlag( QgsMapSettings::ForceVectorOutput, true );
mMapSettings.setOutputSize( QSize( 100, 100 ) );
mMapSettings.setExtent( mpPolysLayer->extent() );
mMapSettings.setOutputDpi( 96 );

properties.insert( QStringLiteral( "color" ), QStringLiteral( "255,0,0,255" ) );
pointSymbol = QgsMarkerSymbol::createSimple( properties );
mPointPatternFill->setSubSymbol( pointSymbol );

QgsMapRendererCustomPainterJob job( mMapSettings, &p );
job.start();
job.waitForFinished();
p.end();
mMapSettings.setFlag( QgsMapSettings::ForceVectorOutput, false );

QByteArray ba = buffer.data();
QVERIFY( ba.contains( "fill=\"#ff0000\"" ) );
}

void TestQgsPointPatternFillSymbol::offsettedPointPatternFillSymbol()
{
mReport += QLatin1String( "<h2>Offsetted point pattern fill symbol renderer test</h2>\n" );
Expand Down Expand Up @@ -171,6 +218,35 @@ void TestQgsPointPatternFillSymbol::offsettedPointPatternFillSymbol()
mPointPatternFill->setOffsetY( 0 );
}

void TestQgsPointPatternFillSymbol::offsettedPointPatternFillSymbolVector()
{
mReport += QLatin1String( "<h2>Offsetted point pattern fill symbol renderer test</h2>\n" );

QgsStringMap properties;
properties.insert( QStringLiteral( "color" ), QStringLiteral( "0,0,0,255" ) );
properties.insert( QStringLiteral( "outline_color" ), QStringLiteral( "#000000" ) );
properties.insert( QStringLiteral( "name" ), QStringLiteral( "circle" ) );
properties.insert( QStringLiteral( "size" ), QStringLiteral( "5.0" ) );
QgsMarkerSymbol *pointSymbol = QgsMarkerSymbol::createSimple( properties );

mPointPatternFill->setSubSymbol( pointSymbol );
mPointPatternFill->setDistanceX( 15 );
mPointPatternFill->setDistanceY( 15 );
mPointPatternFill->setOffsetX( 4 );
mPointPatternFill->setOffsetY( 4 );
QVERIFY( imageCheck( "symbol_pointfill_offset" ) );

// With offset values greater than the pattern size (i.e. distance * 2 ), offsets values are modulos of offset against distance
mPointPatternFill->setOffsetX( 19 );
mPointPatternFill->setOffsetY( 19 );
mMapSettings.setFlag( QgsMapSettings::ForceVectorOutput, true );
bool res = imageCheck( "symbol_pointfill_offset_vector" );
mMapSettings.setFlag( QgsMapSettings::ForceVectorOutput, false );
mPointPatternFill->setOffsetX( 0 );
mPointPatternFill->setOffsetY( 0 );
QVERIFY( res );
}

void TestQgsPointPatternFillSymbol::dataDefinedSubSymbol()
{
mReport += QLatin1String( "<h2>Point pattern symbol data defined sub symbol test</h2>\n" );
Expand All @@ -182,6 +258,7 @@ void TestQgsPointPatternFillSymbol::dataDefinedSubSymbol()
properties.insert( QStringLiteral( "size" ), QStringLiteral( "5.0" ) );
QgsMarkerSymbol *pointSymbol = QgsMarkerSymbol::createSimple( properties );
pointSymbol->symbolLayer( 0 )->setDataDefinedProperty( QgsSymbolLayer::PropertyFillColor, QgsProperty::fromExpression( QStringLiteral( "if(\"Name\" ='Lake','#ff0000','#ff00ff')" ) ) );
pointSymbol->symbolLayer( 0 )->setDataDefinedProperty( QgsSymbolLayer::PropertySize, QgsProperty::fromExpression( QStringLiteral( "if(\"Name\" ='Lake',5,10)" ) ) );
mPointPatternFill->setSubSymbol( pointSymbol );
QVERIFY( imageCheck( "datadefined_subsymbol" ) );
}
Expand All @@ -205,6 +282,28 @@ void TestQgsPointPatternFillSymbol::zeroSpacedPointPatternFillSymbol()
QVERIFY( imageCheck( "pointfill_zero_space" ) );
}

void TestQgsPointPatternFillSymbol::zeroSpacedPointPatternFillSymbolVector()
{
mReport += QLatin1String( "<h2>Zero distance point pattern fill symbol renderer test</h2>\n" );

QgsStringMap properties;
properties.insert( QStringLiteral( "color" ), QStringLiteral( "0,0,0,255" ) );
properties.insert( QStringLiteral( "outline_color" ), QStringLiteral( "#000000" ) );
properties.insert( QStringLiteral( "name" ), QStringLiteral( "circle" ) );
properties.insert( QStringLiteral( "size" ), QStringLiteral( "5.0" ) );
QgsMarkerSymbol *pointSymbol = QgsMarkerSymbol::createSimple( properties );

mPointPatternFill->setSubSymbol( pointSymbol );
mPointPatternFill->setDistanceX( 0 );
mPointPatternFill->setDistanceY( 15 );
mPointPatternFill->setOffsetX( 4 );
mPointPatternFill->setOffsetY( 4 );
mMapSettings.setFlag( QgsMapSettings::ForceVectorOutput, true );
bool res = imageCheck( "pointfill_zero_space" );
mMapSettings.setFlag( QgsMapSettings::ForceVectorOutput, false );
QVERIFY( res );
}

//
// Private helper functions not called directly by CTest
//
Expand Down
Binary file modified ...ntpatternfill/expected_datadefined_subsymbol/expected_datadefined_subsymbol.png
View file
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file removed ...ternfill/expected_datadefined_subsymbol/expected_datadefined_subsymbol_mask.png
View file
Binary file not shown.
Binary file added ...cted_symbol_pointfill_offset_vector/expected_symbol_pointfill_offset_vector.png
View file
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...tternfill/expected_symbol_pointfill_vector/expected_symbol_pointfill_vector.png
View file
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 23396b7

Please sign in to comment.