Skip to content

Commit

Permalink
Port raster_value to thread safe method, remove unused unsafe getRast…
Browse files Browse the repository at this point in the history
…erLayer/getMeshLayer methods
  • Loading branch information
nyalldawson committed Dec 13, 2022
1 parent 8e007c4 commit 246c5cb
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 63 deletions.
144 changes: 101 additions & 43 deletions src/core/expression/qgsexpressionfunction.cpp
Expand Up @@ -1657,52 +1657,68 @@ static QVariant fcnFeatureId( const QVariantList &, const QgsExpressionContext *

static QVariant fcnRasterValue( const QVariantList &values, const QgsExpressionContext *context, QgsExpression *parent, const QgsExpressionNodeFunction * )
{
QgsRasterLayer *layer = QgsExpressionUtils::getRasterLayer( values.at( 0 ), context, parent );
if ( !layer || !layer->dataProvider() )
{
parent->setEvalErrorString( QObject::tr( "Function `raster_value` requires a valid raster layer." ) );
return QVariant();
}

int bandNb = QgsExpressionUtils::getNativeIntValue( values.at( 1 ), parent );
if ( bandNb < 1 || bandNb > layer->bandCount() )
{
parent->setEvalErrorString( QObject::tr( "Function `raster_value` requires a valid raster band number." ) );
return QVariant();
}

QgsGeometry geom = QgsExpressionUtils::getGeometry( values.at( 2 ), parent );
if ( geom.isNull() || geom.type() != QgsWkbTypes::PointGeometry )
const int bandNb = QgsExpressionUtils::getNativeIntValue( values.at( 1 ), parent );
const QgsGeometry geom = QgsExpressionUtils::getGeometry( values.at( 2 ), parent );
bool foundLayer = false;
const QVariant res = QgsExpressionUtils::runMapLayerFunctionThreadSafe( values.at( 0 ), context, parent, [parent, bandNb, geom]( QgsMapLayer * mapLayer )
{
parent->setEvalErrorString( QObject::tr( "Function `raster_value` requires a valid point geometry." ) );
return QVariant();
}
QgsRasterLayer *layer = qobject_cast< QgsRasterLayer * >( mapLayer );
if ( !layer || !layer->dataProvider() )
{
parent->setEvalErrorString( QObject::tr( "Function `raster_value` requires a valid raster layer." ) );
return QVariant();
}

QgsPointXY point = geom.asPoint();
if ( geom.isMultipart() )
{
QgsMultiPointXY multiPoint = geom.asMultiPoint();
if ( multiPoint.count() == 1 )
if ( bandNb < 1 || bandNb > layer->bandCount() )
{
point = multiPoint[0];
parent->setEvalErrorString( QObject::tr( "Function `raster_value` requires a valid raster band number." ) );
return QVariant();
}
else

if ( geom.isNull() || geom.type() != QgsWkbTypes::PointGeometry )
{
// if the geometry contains more than one part, return an undefined value
parent->setEvalErrorString( QObject::tr( "Function `raster_value` requires a valid point geometry." ) );
return QVariant();
}
}

double value = layer->dataProvider()->sample( point, bandNb );
return std::isnan( value ) ? QVariant() : value;
QgsPointXY point = geom.asPoint();
if ( geom.isMultipart() )
{
QgsMultiPointXY multiPoint = geom.asMultiPoint();
if ( multiPoint.count() == 1 )
{
point = multiPoint[0];
}
else
{
// if the geometry contains more than one part, return an undefined value
return QVariant();
}
}

double value = layer->dataProvider()->sample( point, bandNb );
return std::isnan( value ) ? QVariant() : value;
},
foundLayer );

if ( !foundLayer )
{
parent->setEvalErrorString( QObject::tr( "Function `raster_value` requires a valid raster layer." ) );
return QVariant();
}
else
{
return res;
}
}

static QVariant fcnRasterAttributes( const QVariantList &values, const QgsExpressionContext *context, QgsExpression *parent, const QgsExpressionNodeFunction * )
{
const int bandNb = QgsExpressionUtils::getNativeIntValue( values.at( 1 ), parent );
const double value = QgsExpressionUtils::getDoubleValue( values.at( 2 ), parent );

return QgsExpressionUtils::runMapLayerFunctionThreadSafe( values.at( 0 ), context, parent, [parent, bandNb, value]( QgsMapLayer * mapLayer )-> QVariant
bool foundLayer = false;
const QVariant res = QgsExpressionUtils::runMapLayerFunctionThreadSafe( values.at( 0 ), context, parent, [parent, bandNb, value]( QgsMapLayer * mapLayer )-> QVariant
{
QgsRasterLayer *layer = qobject_cast< QgsRasterLayer *>( mapLayer );
if ( !layer || !layer->dataProvider() )
Expand Down Expand Up @@ -1747,7 +1763,17 @@ static QVariant fcnRasterAttributes( const QVariantList &values, const QgsExpres
}

return result;
} );
}, foundLayer );

if ( !foundLayer )
{
parent->setEvalErrorString( QObject::tr( "Function `raster_attributes` requires a valid raster layer." ) );
return QVariant();
}
else
{
return res;
}
}

static QVariant fcnFeature( const QVariantList &, const QgsExpressionContext *context, QgsExpression *, const QgsExpressionNodeFunction * )
Expand Down Expand Up @@ -5877,10 +5903,11 @@ static QVariant fcnTransformGeometry( const QVariantList &values, const QgsExpre

static QVariant fcnGetFeatureById( const QVariantList &values, const QgsExpressionContext *context, QgsExpression *parent, const QgsExpressionNodeFunction * )
{
std::unique_ptr<QgsVectorLayerFeatureSource> featureSource = QgsExpressionUtils::getFeatureSource( values.at( 0 ), context, parent );
bool foundLayer = false;
std::unique_ptr<QgsVectorLayerFeatureSource> featureSource = QgsExpressionUtils::getFeatureSource( values.at( 0 ), context, parent, foundLayer );

//no layer found
if ( !featureSource )
if ( !featureSource || !foundLayer )
{
return QVariant();
}
Expand All @@ -5906,11 +5933,11 @@ static QVariant fcnGetFeatureById( const QVariantList &values, const QgsExpressi
static QVariant fcnGetFeature( const QVariantList &values, const QgsExpressionContext *context, QgsExpression *parent, const QgsExpressionNodeFunction * )
{
//arguments: 1. layer id / name, 2. key attribute, 3. eq value

std::unique_ptr<QgsVectorLayerFeatureSource> featureSource = QgsExpressionUtils::getFeatureSource( values.at( 0 ), context, parent );
bool foundLayer = false;
std::unique_ptr<QgsVectorLayerFeatureSource> featureSource = QgsExpressionUtils::getFeatureSource( values.at( 0 ), context, parent, foundLayer );

//no layer found
if ( !featureSource )
if ( !featureSource || !foundLayer )
{
return QVariant();
}
Expand Down Expand Up @@ -6052,7 +6079,8 @@ static QVariant fcnGetLayerProperty( const QVariantList &values, const QgsExpres
{
const QString layerProperty = QgsExpressionUtils::getStringValue( values.at( 1 ), parent );

return QgsExpressionUtils::runMapLayerFunctionThreadSafe( values.at( 0 ), context, parent, [layerProperty]( QgsMapLayer * layer )-> QVariant
bool foundLayer = false;
const QVariant res = QgsExpressionUtils::runMapLayerFunctionThreadSafe( values.at( 0 ), context, parent, [layerProperty]( QgsMapLayer * layer )-> QVariant
{
if ( !layer )
return QVariant();
Expand Down Expand Up @@ -6151,14 +6179,21 @@ static QVariant fcnGetLayerProperty( const QVariantList &values, const QgsExpres
}

return QVariant();
} );
}, foundLayer );

if ( !foundLayer )
return QVariant();
else
return res;
}

static QVariant fcnDecodeUri( const QVariantList &values, const QgsExpressionContext *context, QgsExpression *parent, const QgsExpressionNodeFunction * )
{
const QString uriPart = values.at( 1 ).toString();

return QgsExpressionUtils::runMapLayerFunctionThreadSafe( values.at( 0 ), context, parent, [parent, uriPart]( QgsMapLayer * layer )-> QVariant
bool foundLayer = false;

const QVariant res = QgsExpressionUtils::runMapLayerFunctionThreadSafe( values.at( 0 ), context, parent, [parent, uriPart]( QgsMapLayer * layer )-> QVariant
{
if ( !layer->dataProvider() )
{
Expand All @@ -6176,15 +6211,26 @@ static QVariant fcnDecodeUri( const QVariantList &values, const QgsExpressionCon
{
return decodedUri;
}
} );
}, foundLayer );

if ( !foundLayer )
{
parent->setEvalErrorString( QObject::tr( "Function `decode_uri` requires a valid layer." ) );
return QVariant();
}
else
{
return res;
}
}

static QVariant fcnGetRasterBandStat( const QVariantList &values, const QgsExpressionContext *context, QgsExpression *parent, const QgsExpressionNodeFunction * )
{
const int band = QgsExpressionUtils::getNativeIntValue( values.at( 1 ), parent );
const QString layerProperty = QgsExpressionUtils::getStringValue( values.at( 2 ), parent );

return QgsExpressionUtils::runMapLayerFunctionThreadSafe( values.at( 0 ), context, parent, [parent, band, layerProperty]( QgsMapLayer * layer )-> QVariant
bool foundLayer = false;
const QVariant res = QgsExpressionUtils::runMapLayerFunctionThreadSafe( values.at( 0 ), context, parent, [parent, band, layerProperty]( QgsMapLayer * layer )-> QVariant
{
QgsRasterLayer *rl = qobject_cast< QgsRasterLayer * >( layer );
if ( !rl )
Expand Down Expand Up @@ -6233,7 +6279,19 @@ static QVariant fcnGetRasterBandStat( const QVariantList &values, const QgsExpre
return stats.sum;
}
return QVariant();
} );
}, foundLayer );

if ( !foundLayer )
{
#if 0 // for consistency with other functions we should raise an error here, but for compatibility with old projects we don't
parent->setEvalErrorString( QObject::tr( "Function `raster_statistic` requires a valid raster layer." ) );
#endif
return QVariant();
}
else
{
return res;
}
}

static QVariant fcnArray( const QVariantList &values, const QgsExpressionContext *, QgsExpression *, const QgsExpressionNodeFunction * )
Expand Down
21 changes: 14 additions & 7 deletions src/core/expression/qgsexpressionutils.cpp
Expand Up @@ -93,7 +93,7 @@ QgsMapLayer *QgsExpressionUtils::getMapLayer( const QVariant &value, const QgsEx
return ml;
}

void QgsExpressionUtils::executeLambdaForMapLayer( const QVariant &value, const QgsExpressionContext *context, QgsExpression *expression, const std::function<void ( QgsMapLayer * )> &function )
void QgsExpressionUtils::executeLambdaForMapLayer( const QVariant &value, const QgsExpressionContext *context, QgsExpression *expression, const std::function<void ( QgsMapLayer * )> &function, bool &foundLayer )
{
// First check if we already received a layer pointer
QPointer< QgsMapLayer > ml = value.value< QgsWeakMapLayerPointer >().data();
Expand All @@ -110,10 +110,11 @@ void QgsExpressionUtils::executeLambdaForMapLayer( const QVariant &value, const
if ( ml )
{
QPointer< QgsMapLayer > layerPointer( ml );
auto runFunction = [ layerPointer, &function ]
auto runFunction = [ layerPointer, &function, &foundLayer ]
{
if ( QgsMapLayer *layer = layerPointer.data() )
{
foundLayer = true;
function( layer );
}
};
Expand All @@ -130,12 +131,17 @@ void QgsExpressionUtils::executeLambdaForMapLayer( const QVariant &value, const
}

// if no layer stores, then this is only for layers in project and therefore associated with the main thread
auto runFunction = [ value, context, expression, &function ]
auto runFunction = [ value, context, expression, &function, &foundLayer ]
{
if ( QgsMapLayer *layer = getMapLayer( value, context, expression ) )
{
foundLayer = true;
function( layer );
}
else
{
foundLayer = false;
}
};

// Make sure we only deal with the layer on the thread where it lives.
Expand All @@ -146,20 +152,21 @@ void QgsExpressionUtils::executeLambdaForMapLayer( const QVariant &value, const
QMetaObject::invokeMethod( QgsProject::instance(), runFunction, Qt::BlockingQueuedConnection );
}

QVariant QgsExpressionUtils::runMapLayerFunctionThreadSafe( const QVariant &value, const QgsExpressionContext *context, QgsExpression *expression, const std::function<QVariant( QgsMapLayer * )> &function )
QVariant QgsExpressionUtils::runMapLayerFunctionThreadSafe( const QVariant &value, const QgsExpressionContext *context, QgsExpression *expression, const std::function<QVariant( QgsMapLayer * )> &function, bool &foundLayer )
{
QVariant res;
foundLayer = false;

executeLambdaForMapLayer( value, context, expression, [&res, function]( QgsMapLayer * layer )
{
if ( layer )
res = function( layer );
} );
}, foundLayer );

return res;
}

std::unique_ptr<QgsVectorLayerFeatureSource> QgsExpressionUtils::getFeatureSource( const QVariant &value, const QgsExpressionContext *context, QgsExpression *e )
std::unique_ptr<QgsVectorLayerFeatureSource> QgsExpressionUtils::getFeatureSource( const QVariant &value, const QgsExpressionContext *context, QgsExpression *e, bool &foundLayer )
{
std::unique_ptr<QgsVectorLayerFeatureSource> featureSource;

Expand All @@ -169,7 +176,7 @@ std::unique_ptr<QgsVectorLayerFeatureSource> QgsExpressionUtils::getFeatureSourc
{
featureSource.reset( new QgsVectorLayerFeatureSource( vl ) );
}
} );
}, foundLayer );

return featureSource;
}
Expand Down
16 changes: 3 additions & 13 deletions src/core/expression/qgsexpressionutils.h
Expand Up @@ -359,35 +359,25 @@ class CORE_EXPORT QgsExpressionUtils
*
* \since QGIS 3.30
*/
static void executeLambdaForMapLayer( const QVariant &value, const QgsExpressionContext *context, QgsExpression *expression, const std::function< void( QgsMapLayer * )> &function );
static void executeLambdaForMapLayer( const QVariant &value, const QgsExpressionContext *context, QgsExpression *expression, const std::function< void( QgsMapLayer * )> &function, bool &foundLayer );

/**
* Evaluates a \a value to a map layer, then runs a \a function on the layer in a thread safe way before returning the result of the function.
*
* \since QGIS 3.30
*/
static QVariant runMapLayerFunctionThreadSafe( const QVariant &value, const QgsExpressionContext *context, QgsExpression *expression, const std::function<QVariant( QgsMapLayer * ) > &function );
static QVariant runMapLayerFunctionThreadSafe( const QVariant &value, const QgsExpressionContext *context, QgsExpression *expression, const std::function<QVariant( QgsMapLayer * ) > &function, bool &foundLayer );

/**
* Gets a vector layer feature source for a \a value which corresponds to a vector layer, in a thread-safe way.
*/
static std::unique_ptr<QgsVectorLayerFeatureSource> getFeatureSource( const QVariant &value, const QgsExpressionContext *context, QgsExpression *e );
static std::unique_ptr<QgsVectorLayerFeatureSource> getFeatureSource( const QVariant &value, const QgsExpressionContext *context, QgsExpression *e, bool &foundLayer );

static QgsVectorLayer *getVectorLayer( const QVariant &value, const QgsExpressionContext *context, QgsExpression *e )
{
return qobject_cast<QgsVectorLayer *>( getMapLayer( value, context, e ) );
}

static QgsRasterLayer *getRasterLayer( const QVariant &value, const QgsExpressionContext *context, QgsExpression *e )
{
return qobject_cast<QgsRasterLayer *>( getMapLayer( value, context, e ) );
}

static QgsMeshLayer *getMeshLayer( const QVariant &value, const QgsExpressionContext *context, QgsExpression *e )
{
return qobject_cast<QgsMeshLayer *>( getMapLayer( value, context, e ) );
}

/**
* Tries to convert a \a value to a file path.
*
Expand Down

0 comments on commit 246c5cb

Please sign in to comment.