Fix is_layer_visible to allow direct layer objects and handle removed layers

Author: nyalldawson

python/core/auto_generated/qgsexpressioncontext.sip.in

@@ -1075,6 +1075,7 @@ Creates a new scope which contains variables and functions relating to provider
 Registers all known core functions provided by QgsExpressionContextScope objects.
 %End
 
+      public:
 };
 
 /************************************************************************

src/core/qgsexpressioncontext.cpp

@@ -32,6 +32,7 @@
 #include "qgslayout.h"
 #include "qgslayoutpagecollection.h"
 #include "qgslayoutreportcontext.h"
+#include "qgsexpressionutils.h"
 
 #include <QSettings>
 #include <QDir>
@@ -712,44 +713,6 @@ class GetLayoutItemVariables : public QgsScopedExpressionFunction
 
 };
 
-class GetLayerVisibility : public QgsScopedExpressionFunction
-{
-  public:
-    GetLayerVisibility( const QList<QgsMapLayer *> &layers )
-      : QgsScopedExpressionFunction( QStringLiteral( "is_layer_visible" ), QgsExpressionFunction::ParameterList() << QgsExpressionFunction::Parameter( QStringLiteral( "id" ) ), QStringLiteral( "General" ) )
-      , mLayers( layers )
-    {}
-
-    QVariant func( const QVariantList &values, const QgsExpressionContext *, QgsExpression *, const QgsExpressionNodeFunction * ) override
-    {
-      if ( mLayers.isEmpty() )
-      {
-        return QVariant( false );
-      }
-
-      QgsMapLayer *layer = _qgis_findLayer( mLayers, values.at( 0 ).toString() );
-      if ( layer )
-      {
-        return QVariant( true );
-      }
-      else
-      {
-        return QVariant( false );
-      }
-    }
-
-    QgsScopedExpressionFunction *clone() const override
-    {
-      return new GetLayerVisibility( mLayers );
-    }
-
-  private:
-
-    const QList<QgsMapLayer *> mLayers;
-
-};
-
-
 class GetCurrentFormFieldValue : public QgsScopedExpressionFunction
 {
   public:
@@ -1340,3 +1303,35 @@ bool QgsScopedExpressionFunction::isStatic( const QgsExpressionNodeFunction *nod
 {
   return allParamsStatic( node, parent, context );
 }
+
+//
+// GetLayerVisibility
+//
+
+QgsExpressionContextUtils::GetLayerVisibility::GetLayerVisibility( const QList<QgsMapLayer *> &layers )
+  : QgsScopedExpressionFunction( QStringLiteral( "is_layer_visible" ), QgsExpressionFunction::ParameterList() << QgsExpressionFunction::Parameter( QStringLiteral( "id" ) ), QStringLiteral( "General" ) )
+  , mLayers( _qgis_listRawToQPointer( layers ) )
+{}
+
+QVariant QgsExpressionContextUtils::GetLayerVisibility::func( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * )
+{
+  if ( mLayers.isEmpty() )
+  {
+    return false;
+  }
+
+  QgsMapLayer *layer = QgsExpressionUtils::getMapLayer( values.at( 0 ), parent );
+  if ( layer )
+  {
+    return mLayers.contains( layer );
+  }
+  else
+  {
+    return false;
+  }
+}
+
+QgsScopedExpressionFunction *QgsExpressionContextUtils::GetLayerVisibility::clone() const
+{
+  return new GetLayerVisibility( _qgis_listQPointerToRaw( mLayers ) );
+}

src/core/qgsexpressioncontext.h

@@ -988,6 +988,23 @@ class CORE_EXPORT QgsExpressionContextUtils
      */
     static void registerContextFunctions();
 
+  private:
+
+    class GetLayerVisibility : public QgsScopedExpressionFunction
+    {
+      public:
+        GetLayerVisibility( const QList<QgsMapLayer *> &layers );
+        QVariant func( const QVariantList &values, const QgsExpressionContext *, QgsExpression *, const QgsExpressionNodeFunction * ) override;
+        QgsScopedExpressionFunction *clone() const override;
+
+      private:
+
+        const QList< QPointer< QgsMapLayer > > mLayers;
+
+    };
+
+    friend class QgsLayoutItemMap; // needs access to GetLayerVisibility
+
 };
 
 #endif // QGSEXPRESSIONCONTEXT_H

tests/src/core/testqgsmapsettings.cpp

@@ -176,6 +176,8 @@ void TestQgsMapSettings::testIsLayerVisible()
   QList<QgsMapLayer *> layers;
   layers << vlA << vlB;
 
+  QgsProject::instance()->addMapLayers( layers );
+
   QgsMapSettings ms;
   ms.setLayers( layers );
   QgsExpressionContext context;
@@ -186,6 +188,11 @@ void TestQgsMapSettings::testIsLayerVisible()
   QVariant r = e.evaluate( &context );
   QCOMPARE( r.toBool(), true );
 
+  // test checking for visible layer by direct map layer object
+  QgsExpression e4( QStringLiteral( "is_layer_visible(array_get( @map_layers, 0 ) )" ) );
+  r = e4.evaluate( &context );
+  QCOMPARE( r.toBool(), true );
+
   // test checking for visible layer by name
   QgsExpression e2( QStringLiteral( "is_layer_visible( '%1' )" ).arg( vlB-> name() ) );
   r = e2.evaluate( &context );
@@ -196,8 +203,16 @@ void TestQgsMapSettings::testIsLayerVisible()
   r = e3.evaluate( &context );
   QCOMPARE( r.toBool(), false );
 
-  delete vlA;
-  delete vlB;
+  QgsProject::instance()->removeMapLayer( vlA );
+  r = e.evaluate( &context );
+  QCOMPARE( r.toBool(), false ); // layer is deleted
+  r = e2.evaluate( &context );
+  QCOMPARE( r.toBool(), true ); // layer still exists
+
+  QgsProject::instance()->removeMapLayer( vlB );
+  r = e2.evaluate( &context );
+  QCOMPARE( r.toBool(), false ); // layer is deleted
+
 }
 
 void TestQgsMapSettings::testMapLayerListUtils()