Skip to content

Commit

Permalink
Made layers/labels visibility more consistent
Browse files Browse the repository at this point in the history
Some places forgot to make the max scale inclusive.
  • Loading branch information
Patrick Valsecchi committed Mar 9, 2016
1 parent e08130d commit 3efc73b
Show file tree
Hide file tree
Showing 18 changed files with 77 additions and 19 deletions.
6 changes: 6 additions & 0 deletions python/core/qgis.sip
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,12 @@ class QGis
/** Default highlight line/outline minimum width in mm.
* @note added in 2.3 */
static double DEFAULT_HIGHLIGHT_MIN_WIDTH_MM;

/** Fudge factor used to compare two scales. The code is often going from scale to scale
* denominator. So it looses precision and, when a limit is inclusive, can lead to errors.
* To avoid that, use this factor instead of using <= or >=.
* @note added in 2.15*/
static double SCALE_PRECISION;
};


Expand Down
3 changes: 3 additions & 0 deletions python/core/qgslabel.sip
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ class QgsLabel
void setScaleBasedVisibility( bool theVisibilityFlag );
bool scaleBasedVisibility() const;

/** Return true if the label is visible at the given scale */
bool isInScaleRange( double scale ) const;

private:
QgsLabel (); // pretend that constructor is private for now
QgsLabel( const QgsLabel& rh );
Expand Down
5 changes: 5 additions & 0 deletions python/core/qgsmaplayer.sip
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,11 @@ class QgsMapLayer : QObject
*/
QgsMapLayerStyleManager* styleManager() const;

/**
* @returns true if the layer is visible at the given scale.
*/
bool isInScaleRange( double scale ) const;

/** Returns the minimum scale denominator at which the layer is visible.
* Scale based visibility is only used if hasScaleBasedVisibility is true.
* @returns minimum scale denominator at which the layer will render
Expand Down
5 changes: 2 additions & 3 deletions src/core/dxf/qgsdxfexport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4109,11 +4109,10 @@ bool QgsDxfExport::layerIsScaleBasedVisible( const QgsMapLayer* layer ) const
if ( !layer )
return false;

if ( mSymbologyExport == QgsDxfExport::NoSymbology || !layer->hasScaleBasedVisibility() )
if ( mSymbologyExport == QgsDxfExport::NoSymbology )
return true;

return layer->minimumScale() < mSymbologyScaleDenominator &&
layer->maximumScale() > mSymbologyScaleDenominator;
return layer->isInScaleRange( mSymbologyScaleDenominator );
}

QString QgsDxfExport::layerName( const QString &id, const QgsFeature &f ) const
Expand Down
2 changes: 2 additions & 0 deletions src/core/qgis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ double QGis::DEFAULT_HIGHLIGHT_BUFFER_MM = 0.5;

double QGis::DEFAULT_HIGHLIGHT_MIN_WIDTH_MM = 1.0;

double QGis::SCALE_PRECISION = 0.9999999999;

// description strings for units
// Order must match enum indices
const char* QGis::qgisUnitTypes[] =
Expand Down
6 changes: 6 additions & 0 deletions src/core/qgis.h
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,12 @@ class CORE_EXPORT QGis
* @note added in 2.3 */
static double DEFAULT_HIGHLIGHT_MIN_WIDTH_MM;

/** Fudge factor used to compare two scales. The code is often going from scale to scale
* denominator. So it looses precision and, when a limit is inclusive, can lead to errors.
* To avoid that, use this factor instead of using <= or >=.
* @note added in 2.15*/
static double SCALE_PRECISION;

private:
// String representation of unit types (set in qgis.cpp)
static const char *qgisUnitTypes[];
Expand Down
6 changes: 6 additions & 0 deletions src/core/qgslabel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1414,3 +1414,9 @@ float QgsLabel::maxScale() const
{
return mMaxScale;
}

bool QgsLabel::isInScaleRange( double scale ) const
{
return !mScaleBasedVisibility ||
( mMinScale * QGis::SCALE_PRECISION < scale && scale * QGis::SCALE_PRECISION < mMaxScale );
}
3 changes: 3 additions & 0 deletions src/core/qgslabel.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ class CORE_EXPORT QgsLabel
void setScaleBasedVisibility( bool theVisibilityFlag );
bool scaleBasedVisibility() const;

/** Return true if the label is visible at the given scale */
bool isInScaleRange( double scale ) const;

private:
/** Does the actual rendering of a label at the given point */
void renderLabel( QgsRenderContext &renderContext, QgsPoint point,
Expand Down
2 changes: 1 addition & 1 deletion src/core/qgsmaphittest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ void QgsMapHitTest::run()

if ( !mOnlyExpressions )
{
if ( vl->hasScaleBasedVisibility() && ( mSettings.scale() < vl->minimumScale() || mSettings.scale() > vl->maximumScale() ) )
if ( !vl->isInScaleRange( mSettings.scale() ) )
{
mHitTest[vl] = SymbolV2Set(); // no symbols -> will not be shown
mHitTestRuleKey[vl] = SymbolV2Set();
Expand Down
4 changes: 4 additions & 0 deletions src/core/qgsmaplayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,10 @@ void QgsMapLayer::connectNotify( const char * signal )
} // QgsMapLayer::connectNotify
#endif

bool QgsMapLayer::isInScaleRange( double scale ) const
{
return !mScaleBasedVisibility || ( mMinScale * QGis::SCALE_PRECISION < scale && scale < mMaxScale );
}

void QgsMapLayer::toggleScaleBasedVisibility( bool theVisibilityFlag )
{
Expand Down
5 changes: 5 additions & 0 deletions src/core/qgsmaplayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,11 @@ class CORE_EXPORT QgsMapLayer : public QObject
*/
QgsMapLayerStyleManager* styleManager() const;

/**
* @returns true if the layer is visible at the given scale.
*/
bool isInScaleRange( double scale ) const;

/** Returns the minimum scale denominator at which the layer is visible.
* Scale based visibility is only used if hasScaleBasedVisibility is true.
* @returns minimum scale denominator at which the layer will render
Expand Down
4 changes: 2 additions & 2 deletions src/core/qgsmaprenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ void QgsMapRenderer::render( QPainter* painter, double* forceWidthScale )
mypContextPainter->setCompositionMode( ml->blendMode() );
}

if ( !ml->hasScaleBasedVisibility() || ( ml->minimumScale() <= mScale && mScale < ml->maximumScale() ) || mOverview )
if ( ml->isInScaleRange( mScale ) || mOverview )
{
connect( ml, SIGNAL( drawingProgress( int, int ) ), this, SLOT( onDrawingProgress( int, int ) ) );

Expand Down Expand Up @@ -584,7 +584,7 @@ void QgsMapRenderer::render( QPainter* painter, double* forceWidthScale )
{
// only make labels if the layer is visible
// after scale dep viewing settings are checked
if ( !ml->hasScaleBasedVisibility() || ( ml->minimumScale() < mScale && mScale < ml->maximumScale() ) )
if ( ml->isInScaleRange( mScale ) )
{
bool split = false;

Expand Down
2 changes: 1 addition & 1 deletion src/core/qgsmaprenderercustompainterjob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ void QgsMapRendererJob::drawOldLabeling( const QgsMapSettings& settings, QgsRend

// only make labels if the layer is visible
// after scale dep viewing settings are checked
if ( ml->hasScaleBasedVisibility() && ( settings.scale() < ml->minimumScale() || settings.scale() > ml->maximumScale() ) )
if ( !ml->isInScaleRange( settings.scale() ) )
continue;

const QgsCoordinateTransform* ct = nullptr;
Expand Down
2 changes: 1 addition & 1 deletion src/core/qgsmaprendererjob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ LayerRenderJobs QgsMapRendererJob::prepareJobs( QPainter* painter, QgsPalLabelin
.arg( ml->blendMode() )
);

if ( ml->hasScaleBasedVisibility() && ( mSettings.scale() < ml->minimumScale() || mSettings.scale() > ml->maximumScale() ) ) //|| mOverview )
if ( !ml->isInScaleRange( mSettings.scale() ) ) //|| mOverview )
{
QgsDebugMsg( "Layer not rendered because it is not within the defined visibility scale range" );
continue;
Expand Down
4 changes: 1 addition & 3 deletions src/core/qgsvectorlayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,7 @@ void QgsVectorLayer::drawLabels( QgsRenderContext& rendererContext )
QgsDebugMsg( "Starting draw of labels: " + id() );

if ( mRendererV2 && mLabelOn && mLabel &&
( !mLabel->scaleBasedVisibility() ||
( mLabel->minScale() <= rendererContext.rendererScale() &&
rendererContext.rendererScale() <= mLabel->maxScale() ) ) )
mLabel->isInScaleRange( rendererContext.rendererScale() ) )
{
QgsAttributeList attributes;
Q_FOREACH ( const QString& attrName, mRendererV2->usedAttributes() )
Expand Down
4 changes: 1 addition & 3 deletions src/gui/attributetable/qgsattributetablefiltermodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,7 @@ void QgsAttributeTableFilterModel::generateListOfVisibleFeatures()
}

const QgsMapSettings& ms = mCanvas->mapSettings();
if ( layer()->hasScaleBasedVisibility() &&
( layer()->minimumScale() > ms.scale() ||
layer()->maximumScale() <= ms.scale() ) )
if ( !layer()->isInScaleRange( ms.scale() ) )
{
QgsDebugMsg( "Out of scale limits" );
}
Expand Down
4 changes: 1 addition & 3 deletions src/gui/qgsmaptoolidentify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,7 @@ bool QgsMapToolIdentify::identifyVectorLayer( QList<IdentifyResult> *results, Qg
if ( !layer || !layer->hasGeometryType() )
return false;

if ( layer->hasScaleBasedVisibility() &&
( layer->minimumScale() > mCanvas->mapSettings().scale() ||
layer->maximumScale() <= mCanvas->mapSettings().scale() ) )
if ( !layer->isInScaleRange( mCanvas->mapSettings().scale() ) )
{
QgsDebugMsg( "Out of scale limits" );
return false;
Expand Down
29 changes: 27 additions & 2 deletions tests/src/core/testqgsmaplayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,15 @@ class TestQgsMapLayer : public QObject
private slots:
void initTestCase();// will be called before the first testfunction is executed.
void cleanupTestCase();// will be called after the last testfunction was executed.
void init() {} // will be called before each testfunction is executed.
void cleanup() {} // will be called after every testfunction.
void init(); // will be called before each testfunction is executed.
void cleanup(); // will be called after every testfunction.

void isValid();

void setBlendMode();

void isInScaleRange();

private:
QgsMapLayer * mpLayer;
};
Expand All @@ -75,6 +78,10 @@ void TestQgsMapLayer::initTestCase()
QgsApplication::initQgis();
QgsApplication::showSettings();

}

void TestQgsMapLayer::init()
{
//create some objects that will be used in all tests...
//create a map layer that will be used in all tests...
QString myFileName( TEST_DATA_DIR ); //defined in CmakeLists.txt
Expand All @@ -84,6 +91,11 @@ void TestQgsMapLayer::initTestCase()
myMapFileInfo.completeBaseName(), "ogr" );
}

void TestQgsMapLayer::cleanup()
{
delete mpLayer;
}

void TestQgsMapLayer::cleanupTestCase()
{
QgsApplication::exitQgis();
Expand All @@ -107,5 +119,18 @@ void TestQgsMapLayer::setBlendMode()
QCOMPARE( mpLayer->blendMode(), QPainter::CompositionMode_Screen );
}

void TestQgsMapLayer::isInScaleRange()
{
mpLayer->setMinimumScale( 2500.0f );
mpLayer->setMaximumScale( 5000.0f );
mpLayer->setScaleBasedVisibility( true );
QCOMPARE( mpLayer->isInScaleRange( 3000.0f ), true ); // In the middle
QCOMPARE( mpLayer->isInScaleRange( 1000.0f ), false ); // Too low
QCOMPARE( mpLayer->isInScaleRange( 6000.0f ), false ); // Too high
QCOMPARE( mpLayer->isInScaleRange( 5000.0f ), false ); // Max is not inclusive
QCOMPARE( mpLayer->isInScaleRange( 2500.0f ), true ); // Min is inclusive
QCOMPARE( mpLayer->isInScaleRange( 1.0f / (( float )1.0 / 2500.0 ) ), true ); // Min is inclusive even with conversion errors
}

QTEST_MAIN( TestQgsMapLayer )
#include "testqgsmaplayer.moc"

0 comments on commit 3efc73b

Please sign in to comment.