[layouts] Fix some linked map items not correctly restored when

loading projects. Also remove some outdated TODOs Fixes #17892, #17891
qgis · Jan 22, 2018 · 20f50fb · 20f50fb
1 parent f69c1cf
commit 20f50fb
Show file tree

Hide file tree

Showing 21 changed files with 142 additions and 107 deletions.
diff --git a/python/core/layout/qgslayoutframe.sip.in b/python/core/layout/qgslayoutframe.sip.in
@@ -62,7 +62,6 @@ Returns the parent multiframe for the frame.
     virtual QgsLayoutSize fixedSize() const;
 
 
-
     QRectF extent() const;
 %Docstring
 Returns the visible portion of the multi frame's content which

diff --git a/python/core/layout/qgslayoutitemattributetable.sip.in b/python/core/layout/qgslayoutitemattributetable.sip.in
@@ -292,6 +292,8 @@ be replaced by a line break.
 
     virtual QgsExpressionContext createExpressionContext() const;
 
+    virtual void finalizeRestoreFromXml();
+
 
   protected:
 

diff --git a/python/core/layout/qgslayoutitemmapitem.sip.in b/python/core/layout/qgslayoutitemmapitem.sip.in
@@ -48,6 +48,17 @@ Sets the map item state from a DOM document, where ``element`` is the DOM
 node corresponding to a 'LayoutMapGrid' tag.
 
 .. seealso:: :py:func:`writeXml`
+%End
+
+    virtual void finalizeRestoreFromXml();
+%Docstring
+Called after all pending items have been restored from XML. Map items can use
+this method to run steps which must take place after all items have been restored to the layout,
+such as connecting to signals emitted by other items, which may not have existed in the layout
+at the time readXml() was called. E.g. an overview can use this to connect to its linked
+map item after restoration from XML.
+
+.. seealso:: :py:func:`readXml`
 %End
 
     void setMap( QgsLayoutItemMap *map );
@@ -156,6 +167,17 @@ Sets the item stack's state from a DOM document, where ``element`` is a DOM node
 Returns true if read was successful.
 
 .. seealso:: :py:func:`writeXml`
+%End
+
+    virtual void finalizeRestoreFromXml();
+%Docstring
+Called after all pending items have been restored from XML. Map item stacks can use
+this method to run steps which must take place after all items have been restored to the layout,
+such as connecting to signals emitted by other items, which may not have existed in the layout
+at the time readXml() was called. E.g. an overview can use this to connect to its linked
+map item after restoration from XML.
+
+.. seealso:: :py:func:`readXml`
 %End
 
     void drawItems( QPainter *painter );

diff --git a/python/core/layout/qgslayoutitemmapoverview.sip.in b/python/core/layout/qgslayoutitemmapoverview.sip.in
@@ -138,6 +138,8 @@ Constructor for QgsLayoutItemMapOverview.
 
     virtual bool readXml( const QDomElement &itemElem, const QDomDocument &doc, const QgsReadWriteContext &context );
 
+    virtual void finalizeRestoreFromXml();
+
     virtual bool usesAdvancedEffects() const;
 
 

diff --git a/src/core/layout/qgslayoutframe.cpp b/src/core/layout/qgslayoutframe.cpp
@@ -209,21 +209,3 @@ bool QgsLayoutFrame::readPropertiesFromElement( const QDomElement &itemElem, con
   mMultiFrame = mLayout->multiFrameByUuid( mMultiFrameUuid );
   return true;
 }
-
-#if 0 //TODO
-void QgsLayoutFrame::beginItemCommand( const QString &text )
-{
-  if ( mComposition )
-  {
-    mComposition->beginMultiFrameCommand( multiFrame(), text );
-  }
-}
-
-void QgsLayoutFrame::endItemCommand()
-{
-  if ( mComposition )
-  {
-    mComposition->endMultiFrameCommand();
-  }
-}
-#endif
diff --git a/src/core/layout/qgslayoutframe.h b/src/core/layout/qgslayoutframe.h
@@ -68,11 +68,6 @@ class CORE_EXPORT QgsLayoutFrame: public QgsLayoutItem
     QgsLayoutSize minimumSize() const override;
     QgsLayoutSize fixedSize() const override;
 
-#if 0 //TODO
-    void beginItemCommand( const QString &text ) override;
-    void endItemCommand() override;
-#endif
-
     /**
      * Returns the visible portion of the multi frame's content which
      * is shown in this frame, in layout units.

diff --git a/src/core/layout/qgslayoutitem.cpp b/src/core/layout/qgslayoutitem.cpp
@@ -699,10 +699,6 @@ bool QgsLayoutItem::readXml( const QDomElement &element, const QDomDocument &doc
     bool alphaOk = false;
     int penRed, penGreen, penBlue, penAlpha;
 
-#if 0 // TODO, old style
-    double penWidth;
-    penWidth = element.attribute( QStringLiteral( "outlineWidth" ) ).toDouble( &widthOk );
-#endif
     penRed = frameColorElem.attribute( QStringLiteral( "red" ) ).toDouble( &redOk );
     penGreen = frameColorElem.attribute( QStringLiteral( "green" ) ).toDouble( &greenOk );
     penBlue = frameColorElem.attribute( QStringLiteral( "blue" ) ).toDouble( &blueOk );

diff --git a/src/core/layout/qgslayoutitemattributetable.cpp b/src/core/layout/qgslayoutitemattributetable.cpp
@@ -538,6 +538,19 @@ QgsExpressionContext QgsLayoutItemAttributeTable::createExpressionContext() cons
   return context;
 }
 
+void QgsLayoutItemAttributeTable::finalizeRestoreFromXml()
+{
+  if ( !mMap && !mMapUuid.isEmpty() && mLayout )
+  {
+    mMap = qobject_cast< QgsLayoutItemMap *>( mLayout->itemByUuid( mMapUuid, true ) );
+    if ( mMap )
+    {
+      //if we have found a valid map item, listen out to extent changes on it and refresh the table
+      connect( mMap, &QgsLayoutItemMap::extentChanged, this, &QgsLayoutTable::refreshAttributes );
+    }
+  }
+}
+
 QVariant QgsLayoutItemAttributeTable::replaceWrapChar( const QVariant &variant ) const
 {
   //avoid converting variants to string if not required (try to maintain original type for sorting)
@@ -683,21 +696,13 @@ bool QgsLayoutItemAttributeTable::readPropertiesFromElement( const QDomElement &
   mWrapString = itemElem.attribute( QStringLiteral( "wrapString" ) );
 
   //map
-  QString mapUuid = itemElem.attribute( QStringLiteral( "mapUuid" ) );
-  if ( !mLayout || mapUuid.isEmpty() )
-  {
-    mMap = nullptr;
-  }
-  else
-  {
-    mMap = qobject_cast< QgsLayoutItemMap *>( mLayout->itemByUuid( mapUuid, true ) );
-  }
-
+  mMapUuid = itemElem.attribute( QStringLiteral( "mapUuid" ) );
   if ( mMap )
   {
-    //if we have found a valid map item, listen out to extent changes on it and refresh the table
-    connect( mMap, &QgsLayoutItemMap::extentChanged, this, &QgsLayoutTable::refreshAttributes );
+    disconnect( mMap, &QgsLayoutItemMap::extentChanged, this, &QgsLayoutTable::refreshAttributes );
+    mMap = nullptr;
   }
+  // setting new mMap occurs in finalizeRestoreFromXml
 
   //vector layer
   QString layerId = itemElem.attribute( QStringLiteral( "vectorLayer" ) );

diff --git a/src/core/layout/qgslayoutitemattributetable.h b/src/core/layout/qgslayoutitemattributetable.h
@@ -290,6 +290,7 @@ class CORE_EXPORT QgsLayoutItemAttributeTable: public QgsLayoutTable
     bool getTableContents( QgsLayoutTableContents &contents ) override SIP_SKIP;
 
     QgsExpressionContext createExpressionContext() const override;
+    void finalizeRestoreFromXml() override;
 
   protected:
 
@@ -310,6 +311,7 @@ class CORE_EXPORT QgsLayoutItemAttributeTable: public QgsLayoutTable
 
     //! Associated map (used to display the visible features)
     QgsLayoutItemMap *mMap = nullptr;
+    QString mMapUuid;
 
     //! Maximum number of features that is displayed
     int mMaximumNumberOfFeatures = 30;

diff --git a/src/core/layout/qgslayoutitemlegend.cpp b/src/core/layout/qgslayoutitemlegend.cpp
@@ -126,12 +126,6 @@ void QgsLayoutItemLegend::paint( QPainter *painter, const QStyleOptionGraphicsIt
 
 void QgsLayoutItemLegend::finalizeRestoreFromXml()
 {
-#if 0 //TODO
-  if ( mMapId != -1 && mMapUuid.isEmpty() )
-  {
-    setMap( mComposition->getComposerMapById( itemElem.attribute( QStringLiteral( "map" ) ).toInt() ) );
-  }
-#endif
   if ( !mMapUuid.isEmpty() )
   {
     setLinkedMap( qobject_cast< QgsLayoutItemMap * >( mLayout->itemByUuid( mMapUuid, true ) ) );
@@ -211,7 +205,6 @@ void QgsLayoutItemLegend::setCustomLayerTree( QgsLayerTree *rootGroup )
   mCustomLayerTree.reset( rootGroup );
 }
 
-
 void QgsLayoutItemLegend::setAutoUpdateModel( bool autoUpdate )
 {
   if ( autoUpdate == autoUpdateModel() )
@@ -584,16 +577,14 @@ bool QgsLayoutItemLegend::readPropertiesFromElement( const QDomElement &itemElem
   //composer map
   mLegendFilterByMap = itemElem.attribute( QStringLiteral( "legendFilterByMap" ), QStringLiteral( "0" ) ).toInt();
 
-  mMapId = -1;
   mMapUuid.clear();
-  if ( !itemElem.attribute( QStringLiteral( "map" ) ).isEmpty() )
-  {
-    mMapId = itemElem.attribute( QStringLiteral( "map" ) ).toInt();
-  }
   if ( !itemElem.attribute( QStringLiteral( "map_uuid" ) ).isEmpty() )
   {
     mMapUuid = itemElem.attribute( QStringLiteral( "map_uuid" ) );
   }
+  // disconnect current map
+  setupMapConnections( mMap, false );
+  mMap = nullptr;
 
   mFilterOutAtlas = itemElem.attribute( QStringLiteral( "legendFilterByAtlas" ), QStringLiteral( "0" ) ).toInt();
 
@@ -638,24 +629,40 @@ QString QgsLayoutItemLegend::displayName() const
   }
 }
 
+
+void QgsLayoutItemLegend::setupMapConnections( QgsLayoutItemMap *map, bool connectSlots )
+{
+  if ( !map )
+    return;
+
+  if ( !connectSlots )
+  {
+    disconnect( map, &QObject::destroyed, this, &QgsLayoutItemLegend::invalidateCurrentMap );
+    disconnect( map, &QgsLayoutObject::changed, this, &QgsLayoutItemLegend::updateFilterByMapAndRedraw );
+    disconnect( map, &QgsLayoutItemMap::extentChanged, this, &QgsLayoutItemLegend::updateFilterByMapAndRedraw );
+    disconnect( map, &QgsLayoutItemMap::layerStyleOverridesChanged, this, &QgsLayoutItemLegend::mapLayerStyleOverridesChanged );
+  }
+  else
+  {
+    connect( map, &QObject::destroyed, this, &QgsLayoutItemLegend::invalidateCurrentMap );
+    connect( map, &QgsLayoutObject::changed, this, &QgsLayoutItemLegend::updateFilterByMapAndRedraw );
+    connect( map, &QgsLayoutItemMap::extentChanged, this, &QgsLayoutItemLegend::updateFilterByMapAndRedraw );
+    connect( map, &QgsLayoutItemMap::layerStyleOverridesChanged, this, &QgsLayoutItemLegend::mapLayerStyleOverridesChanged );
+  }
+}
+
 void QgsLayoutItemLegend::setLinkedMap( QgsLayoutItemMap *map )
 {
   if ( mMap )
   {
-    disconnect( mMap, &QObject::destroyed, this, &QgsLayoutItemLegend::invalidateCurrentMap );
-    disconnect( mMap, &QgsLayoutObject::changed, this, &QgsLayoutItemLegend::updateFilterByMapAndRedraw );
-    disconnect( mMap, &QgsLayoutItemMap::extentChanged, this, &QgsLayoutItemLegend::updateFilterByMapAndRedraw );
-    disconnect( mMap, &QgsLayoutItemMap::layerStyleOverridesChanged, this, &QgsLayoutItemLegend::mapLayerStyleOverridesChanged );
+    setupMapConnections( mMap, false );
   }
 
   mMap = map;
 
-  if ( map )
+  if ( mMap )
   {
-    connect( map, &QObject::destroyed, this, &QgsLayoutItemLegend::invalidateCurrentMap );
-    connect( map, &QgsLayoutObject::changed, this, &QgsLayoutItemLegend::updateFilterByMapAndRedraw );
-    connect( map, &QgsLayoutItemMap::extentChanged, this, &QgsLayoutItemLegend::updateFilterByMapAndRedraw );
-    connect( map, &QgsLayoutItemMap::layerStyleOverridesChanged, this, &QgsLayoutItemLegend::mapLayerStyleOverridesChanged );
+    setupMapConnections( mMap, true );
   }
 
   updateFilterByMap();

diff --git a/src/core/layout/qgslayoutitemlegend.h b/src/core/layout/qgslayoutitemlegend.h
@@ -472,6 +472,8 @@ class CORE_EXPORT QgsLayoutItemLegend : public QgsLayoutItem
     //! use new custom layer tree and update model. if new root is null pointer, will use project's tree
     void setCustomLayerTree( QgsLayerTree *rootGroup );
 
+    void setupMapConnections( QgsLayoutItemMap *map, bool connect = true );
+
     std::unique_ptr< QgsLegendModel > mLegendModel;
     std::unique_ptr< QgsLayerTreeGroup > mCustomLayerTree;
 
@@ -481,7 +483,6 @@ class CORE_EXPORT QgsLayoutItemLegend : public QgsLayoutItem
     int mColumnCount = 1;
 
     QString mMapUuid;
-    int mMapId = -1;
     QgsLayoutItemMap *mMap = nullptr;
 
     bool mLegendFilterByMap = false;

diff --git a/src/core/layout/qgslayoutitemmap.cpp b/src/core/layout/qgslayoutitemmap.cpp
@@ -1110,11 +1110,8 @@ void QgsLayoutItemMap::finalizeRestoreFromXml()
 {
   assignFreeId();
 
-  const QList<QgsLayoutItemMapOverview * > allOverviews = overviews()->asList();
-  for ( QgsLayoutItemMapOverview *overview : allOverviews )
-  {
-    overview->connectSignals();
-  }
+  mOverviewStack->finalizeRestoreFromXml();
+  mGridStack->finalizeRestoreFromXml();
 }
 
 void QgsLayoutItemMap::setMoveContentPreviewOffset( double xOffset, double yOffset )

diff --git a/src/core/layout/qgslayoutitemmapitem.cpp b/src/core/layout/qgslayoutitemmapitem.cpp
@@ -47,6 +47,10 @@ bool QgsLayoutItemMapItem::readXml( const QDomElement &itemElem, const QDomDocum
   return true;
 }
 
+void QgsLayoutItemMapItem::finalizeRestoreFromXml()
+{
+}
+
 void QgsLayoutItemMapItem::setMap( QgsLayoutItemMap *map )
 {
   mMap = map;
@@ -195,6 +199,14 @@ bool QgsLayoutItemMapItemStack::writeXml( QDomElement &elem, QDomDocument &doc,
   return true;
 }
 
+void QgsLayoutItemMapItemStack::finalizeRestoreFromXml()
+{
+  for ( QgsLayoutItemMapItem *item : mItems )
+  {
+    item->finalizeRestoreFromXml();
+  }
+}
+
 void QgsLayoutItemMapItemStack::drawItems( QPainter *painter )
 {
   if ( !painter )

diff --git a/src/core/layout/qgslayoutitemmapitem.h b/src/core/layout/qgslayoutitemmapitem.h
@@ -61,6 +61,16 @@ class CORE_EXPORT QgsLayoutItemMapItem : public QgsLayoutObject
      */
     virtual bool readXml( const QDomElement &element, const QDomDocument &doc, const QgsReadWriteContext &context );
 
+    /**
+     * Called after all pending items have been restored from XML. Map items can use
+     * this method to run steps which must take place after all items have been restored to the layout,
+     * such as connecting to signals emitted by other items, which may not have existed in the layout
+     * at the time readXml() was called. E.g. an overview can use this to connect to its linked
+     * map item after restoration from XML.
+     * \see readXml()
+     */
+    virtual void finalizeRestoreFromXml();
+
     /**
      * Sets the corresponding layout \a map for the item.
      * \see map()
@@ -165,6 +175,16 @@ class CORE_EXPORT QgsLayoutItemMapItemStack
      */
     virtual bool readXml( const QDomElement &element, const QDomDocument &doc, const QgsReadWriteContext &context ) = 0;
 
+    /**
+     * Called after all pending items have been restored from XML. Map item stacks can use
+     * this method to run steps which must take place after all items have been restored to the layout,
+     * such as connecting to signals emitted by other items, which may not have existed in the layout
+     * at the time readXml() was called. E.g. an overview can use this to connect to its linked
+     * map item after restoration from XML.
+     * \see readXml()
+     */
+    virtual void finalizeRestoreFromXml();
+
     /**
      * Draws the items from the stack on a specified \a painter.
      */

diff --git a/src/core/layout/qgslayoutitemmapoverview.cpp b/src/core/layout/qgslayoutitemmapoverview.cpp
@@ -178,9 +178,9 @@ bool QgsLayoutItemMapOverview::readXml( const QDomElement &itemElem, const QDomD
 
   bool ok = QgsLayoutItemMapItem::readXml( itemElem, doc, context );
 
-#if 0 //TODO
-  setFrameMapUuid( itemElem.attribute( QStringLiteral( "frameMap" ), QStringLiteral( "-1" ) ).toInt() );
-#endif
+  mFrameMapUuid = itemElem.attribute( QStringLiteral( "frameMap" ) );
+  setLinkedMap( nullptr );
+
   mBlendMode = QgsPainting::getCompositionMode( static_cast< QgsPainting::BlendMode >( itemElem.attribute( QStringLiteral( "blendMode" ), QStringLiteral( "0" ) ).toUInt() ) );
   mInverted = ( itemElem.attribute( QStringLiteral( "inverted" ), QStringLiteral( "0" ) ) != QLatin1String( "0" ) );
   mCentered = ( itemElem.attribute( QStringLiteral( "centered" ), QStringLiteral( "0" ) ) != QLatin1String( "0" ) );
@@ -193,6 +193,14 @@ bool QgsLayoutItemMapOverview::readXml( const QDomElement &itemElem, const QDomD
   return ok;
 }
 
+void QgsLayoutItemMapOverview::finalizeRestoreFromXml()
+{
+  if ( !mFrameMapUuid.isEmpty() )
+  {
+    setLinkedMap( qobject_cast< QgsLayoutItemMap * >( mLayout->itemByUuid( mFrameMapUuid, true ) ) );
+  }
+}
+
 bool QgsLayoutItemMapOverview::usesAdvancedEffects() const
 {
   return mBlendMode != QPainter::CompositionMode_SourceOver;

diff --git a/src/core/layout/qgslayoutitemmapoverview.h b/src/core/layout/qgslayoutitemmapoverview.h
@@ -135,6 +135,7 @@ class CORE_EXPORT QgsLayoutItemMapOverview : public QgsLayoutItemMapItem
     void draw( QPainter *painter ) override;
     bool writeXml( QDomElement &elem, QDomDocument &doc, const QgsReadWriteContext &context ) const override;
     bool readXml( const QDomElement &itemElem, const QDomDocument &doc, const QgsReadWriteContext &context ) override;
+    void finalizeRestoreFromXml() override;
     bool usesAdvancedEffects() const override;
 
     /**
@@ -224,6 +225,7 @@ class CORE_EXPORT QgsLayoutItemMapOverview : public QgsLayoutItemMapItem
 
     QgsLayoutItemMapOverview() = delete;
 
+    QString mFrameMapUuid;
     QPointer< QgsLayoutItemMap > mFrameMap;
 
     //! Drawing style for overview farme