Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix bad handling of non-square svg images in svg marker symbol layers
  • Loading branch information
github-actions[bot] authored and nyalldawson committed Aug 20, 2020
1 parent d8281f3 commit ff2807e
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/core/symbology/qgsmarkersymbollayer.cpp
Expand Up @@ -1906,6 +1906,7 @@ void QgsSvgMarkerSymbolLayer::resolvePaths( QgsStringMap &properties, const QgsP

void QgsSvgMarkerSymbolLayer::setPath( const QString &path )
{
mDefaultAspectRatio = 0;
mPath = path;
QColor defaultFillColor, defaultStrokeColor;
double strokeWidth, fillOpacity, strokeOpacity;
Expand Down
24 changes: 24 additions & 0 deletions src/core/symbology/qgssvgcache.cpp
Expand Up @@ -348,7 +348,31 @@ double QgsSvgCache::calcSizeScaleFactor( QgsSvgCacheEntry *entry, const QDomElem

//could not find valid viewbox attribute
if ( viewBox.isEmpty() )
{
// trying looking for width/height and use them as a fallback
if ( docElem.tagName() == QLatin1String( "svg" ) && docElem.hasAttribute( QStringLiteral( "width" ) ) )
{
const QString widthString = docElem.attribute( QStringLiteral( "width" ) );
const QRegularExpression measureRegEx( QStringLiteral( "([\\d\\.]+).*?$" ) );
const QRegularExpressionMatch widthMatch = measureRegEx.match( widthString );
if ( widthMatch.hasMatch() )
{
double width = widthMatch.captured( 1 ).toDouble();
const QString heightString = docElem.attribute( QStringLiteral( "height" ) );

const QRegularExpressionMatch heightMatch = measureRegEx.match( heightString );
if ( heightMatch.hasMatch() )
{
double height = heightMatch.captured( 1 ).toDouble();
viewboxSize = QSizeF( width, height );
return width / entry->size;
}
}
}

return 1.0;
}


//width should be 3rd element in a 4 part space delimited string
QStringList parts = viewBox.split( ' ' );
Expand Down
2 changes: 1 addition & 1 deletion src/gui/symbology/qgssymbollayerwidget.cpp
Expand Up @@ -2405,7 +2405,7 @@ void QgsSvgMarkerSymbolLayerWidget::setGuiForSvg( const QgsSvgMarkerSymbolLayer
spinHeight->blockSignals( true );
if ( preservedAspectRatio )
{
spinHeight->setValue( layer->size() );
spinHeight->setValue( layer->size() * layer->defaultAspectRatio() );
}
else
{
Expand Down
17 changes: 17 additions & 0 deletions tests/src/core/testqgssvgcache.cpp
Expand Up @@ -55,6 +55,7 @@ class TestQgsSvgCache : public QObject
void base64();
void replaceParams();
void aspectRatio();
void noViewBox();

};

Expand Down Expand Up @@ -345,6 +346,22 @@ void TestQgsSvgCache::aspectRatio()
QVERIFY( imageCheck( QStringLiteral( "svgcache_aspect_ratio" ), img, 30 ) );
}

void TestQgsSvgCache::noViewBox()
{
// if a source SVG has no viewbox but it does have width/height, use that as a backup so that
// we can correctly determine the svg's aspect ratio
const QString originalImage = TEST_DATA_DIR + QStringLiteral( "/svg/no_viewbox.svg" );
QgsSvgCache cache;
double size = 12;
const QColor fill = QColor( 0, 0, 0 );
const QColor stroke = QColor( 0, 0, 0 );
double strokeWidth = 1;
double widthScaleFactor = 1;
QSizeF viewBoxSize = cache.svgViewboxSize( originalImage, size, fill, stroke, strokeWidth, widthScaleFactor );
QGSCOMPARENEAR( viewBoxSize.width(), 1.329267, 0.0001 );
QGSCOMPARENEAR( viewBoxSize.height(), 6.358467, 0.0001 );
}

bool TestQgsSvgCache::imageCheck( const QString &testName, QImage &image, int mismatchCount )
{
//draw background
Expand Down
28 changes: 28 additions & 0 deletions tests/src/core/testqgssvgmarker.cpp
Expand Up @@ -62,6 +62,7 @@ class TestQgsSvgMarkerSymbol : public QObject
void dynamicSizeWithAspectRatio();
void dynamicWidthWithAspectRatio();
void dynamicAspectRatio();
void resetDefaultAspectRatio();

private:
bool mTestHasError = false ;
Expand Down Expand Up @@ -252,6 +253,33 @@ void TestQgsSvgMarkerSymbol::dynamicAspectRatio()
QVERIFY( result );
}

void TestQgsSvgMarkerSymbol::resetDefaultAspectRatio()
{
// default aspect ratio must be updated as SVG path is changed
QString svgPath = QgsSymbolLayerUtils::svgSymbolNameToPath( QStringLiteral( "/amenity/amenity_bench.svg" ), QgsPathResolver() );
QgsSvgMarkerSymbolLayer layer( svgPath );
QCOMPARE( layer.defaultAspectRatio(), 1.0 );
QVERIFY( layer.preservedAspectRatio() );

// different aspect ratio
layer.setPath( mTestDataDir + "test_symbol_svg.svg" );
QGSCOMPARENEAR( layer.defaultAspectRatio(), 1.58258242005, 0.0001 );
QVERIFY( layer.preservedAspectRatio() );
layer.setPath( svgPath );
QCOMPARE( layer.defaultAspectRatio(), 1.0 );
QVERIFY( layer.preservedAspectRatio() );

layer.setFixedAspectRatio( 0.5 );
QCOMPARE( layer.defaultAspectRatio(), 1.0 );
QCOMPARE( layer.fixedAspectRatio(), 0.5 );
QVERIFY( !layer.preservedAspectRatio() );

layer.setPath( mTestDataDir + "test_symbol_svg.svg" );
QGSCOMPARENEAR( layer.defaultAspectRatio(), 1.58258242005, 0.0001 );
QCOMPARE( layer.fixedAspectRatio(), 0.5 );
QVERIFY( !layer.preservedAspectRatio() );
}

//
// Private helper functions not called directly by CTest
//
Expand Down
49 changes: 49 additions & 0 deletions tests/testdata/svg/no_viewbox.svg
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 ff2807e

Please sign in to comment.