[composer] Setting the composition for a composer view correctly creates item widgets and sets up connections for composer to composition (fix #10038)

python/gui/qgscomposerview.sip

@@ -89,8 +89,11 @@ class QgsComposerView : QGraphicsView
     QgsComposerView::Tool currentTool() const;
     void setCurrentTool( QgsComposerView::Tool t );
 
-    /**Sets composition (derived from QGraphicsScene)*/
+    /**Sets the composition for the view. If the composition is being set manually and not by a QgsComposer, then this must
+     * be set BEFORE adding any items to the composition.
+    */
     void setComposition( QgsComposition* c /KeepReference/ );
+    
     /**Returns the composition or 0 in case of error*/
     QgsComposition* composition();
 
@@ -158,4 +161,7 @@ class QgsComposerView : QGraphicsView
     void composerViewShow( QgsComposerView* );
     /**Emitted before composerview is hidden*/
     void composerViewHide( QgsComposerView* );
+    
+    /**Emitted when the composition is set for the view*/
+    void compositionSet( QgsComposition* );    
 };

src/app/composer/qgscomposer.cpp

@@ -487,10 +487,15 @@ QgsComposer::QgsComposer( QgisApp *qgis, const QString& title )
   }
 
   restoreGridSettings();
-  connectSlots();
+  connectViewSlots();
+  connectCompositionSlots();
+  connectOtherSlots();
 
   mComposition->setParent( mView );
   mView->setComposition( mComposition );
+  //this connection is set up after setting the view's composition, as we don't want setComposition called
+  //for new composers
+  connect( mView, SIGNAL( compositionSet( QgsComposition* ) ), this, SLOT( setComposition( QgsComposition* ) ) );
 
   int minDockWidth( 335 );
 
@@ -515,11 +520,7 @@ QgsComposer::QgsComposer( QgisApp *qgis, const QString& title )
   mUndoDock->setFeatures( QDockWidget::DockWidgetMovable | QDockWidget::DockWidgetClosable );
   mAtlasDock->setFeatures( QDockWidget::DockWidgetMovable | QDockWidget::DockWidgetClosable );
 
-  QgsCompositionWidget* compositionWidget = new QgsCompositionWidget( mGeneralDock, mComposition );
-  connect( mComposition, SIGNAL( paperSizeChanged() ), compositionWidget, SLOT( displayCompositionWidthHeight() ) );
-  connect( this, SIGNAL( printAsRasterChanged( bool ) ), compositionWidget, SLOT( setPrintAsRasterCheckBox( bool ) ) );
-  connect( compositionWidget, SIGNAL( pageOrientationChanged( QString ) ), this, SLOT( setPrinterPageOrientation( QString ) ) );
-  mGeneralDock->setWidget( compositionWidget );
+  createCompositionWidget();
 
   //undo widget
   mUndoView = new QUndoView( mComposition->undoStack(), this );
@@ -658,12 +659,29 @@ void QgsComposer::setIconSizes( int size )
   }
 }
 
-void QgsComposer::connectSlots()
+void QgsComposer::connectViewSlots()
 {
+  if ( !mView )
+  {
+    return;
+  }
+
   connect( mView, SIGNAL( selectedItemChanged( QgsComposerItem* ) ), this, SLOT( showItemOptions( QgsComposerItem* ) ) );
   connect( mView, SIGNAL( itemRemoved( QgsComposerItem* ) ), this, SLOT( deleteItem( QgsComposerItem* ) ) );
   connect( mView, SIGNAL( actionFinished() ), this, SLOT( setSelectionTool() ) );
 
+  //listen out for position updates from the QgsComposerView
+  connect( mView, SIGNAL( cursorPosChanged( QPointF ) ), this, SLOT( updateStatusCursorPos( QPointF ) ) );
+  connect( mView, SIGNAL( zoomLevelChanged() ), this, SLOT( updateStatusZoom() ) );
+}
+
+void QgsComposer::connectCompositionSlots()
+{
+  if ( !mComposition )
+  {
+    return;
+  }
+
   connect( mComposition, SIGNAL( selectedItemChanged( QgsComposerItem* ) ), this, SLOT( showItemOptions( QgsComposerItem* ) ) );
   connect( mComposition, SIGNAL( composerArrowAdded( QgsComposerArrow* ) ), this, SLOT( addComposerArrow( QgsComposerArrow* ) ) );
   connect( mComposition, SIGNAL( composerHtmlFrameAdded( QgsComposerHtml*, QgsComposerFrame* ) ), this, SLOT( addComposerHtmlFrame( QgsComposerHtml*, QgsComposerFrame* ) ) );
@@ -680,19 +698,21 @@ void QgsComposer::connectSlots()
   connect( mComposition, SIGNAL( nPagesChanged() ), mHorizontalRuler, SLOT( update() ) );
   connect( mComposition, SIGNAL( nPagesChanged() ), mVerticalRuler, SLOT( update() ) );
 
-  //listen out for position updates from the QgsComposerView
-  connect( mView, SIGNAL( cursorPosChanged( QPointF ) ), this, SLOT( updateStatusCursorPos( QPointF ) ) );
+  //listen out to status bar updates from the atlas
+  QgsAtlasComposition* atlasMap = &mComposition->atlasComposition();
+  connect( atlasMap, SIGNAL( statusMsgChanged( QString ) ), this, SLOT( updateStatusAtlasMsg( QString ) ) );
+
+  //listen out to status bar updates from the composition
+  connect( mComposition, SIGNAL( statusMsgChanged( QString ) ), this, SLOT( updateStatusCompositionMsg( QString ) ) );
+}
+
+void QgsComposer::connectOtherSlots()
+{
   //also listen out for position updates from the horizontal/vertical rulers
   connect( mHorizontalRuler, SIGNAL( cursorPosChanged( QPointF ) ), this, SLOT( updateStatusCursorPos( QPointF ) ) );
   connect( mVerticalRuler, SIGNAL( cursorPosChanged( QPointF ) ), this, SLOT( updateStatusCursorPos( QPointF ) ) );
   //listen out for zoom updates
   connect( this, SIGNAL( zoomLevelChanged() ), this, SLOT( updateStatusZoom() ) );
-  connect( mView, SIGNAL( zoomLevelChanged() ), this, SLOT( updateStatusZoom() ) );
-  //listen out to status bar updates from the composition
-  connect( mComposition, SIGNAL( statusMsgChanged( QString ) ), this, SLOT( updateStatusCompositionMsg( QString ) ) );
-  //listen out to status bar updates from the atlas
-  QgsAtlasComposition* atlasMap = &mComposition->atlasComposition();
-  connect( atlasMap, SIGNAL( statusMsgChanged( QString ) ), this, SLOT( updateStatusAtlasMsg( QString ) ) );
 }
 
 void QgsComposer::open( void )
@@ -1200,6 +1220,41 @@ void QgsComposer::activateDeuteranopePreview()
   mView->setPreviewModeEnabled( true );
 }
 
+void QgsComposer::setComposition( QgsComposition* composition )
+{
+  if ( !composition )
+  {
+    return;
+  }
+
+  //delete composition widget
+  QgsCompositionWidget* oldCompositionWidget = qobject_cast<QgsCompositionWidget *>( mGeneralDock->widget() );
+  delete oldCompositionWidget;
+
+  deleteItemWidgets();
+
+  mComposition = composition;
+
+  connectCompositionSlots();
+  createCompositionWidget();
+  restoreGridSettings();
+  setupUndoView();
+
+  //setup atlas composition widget
+  QgsAtlasCompositionWidget* oldAtlasWidget = qobject_cast<QgsAtlasCompositionWidget *>( mAtlasDock->widget() );
+  delete oldAtlasWidget;
+  mAtlasDock->setWidget( new QgsAtlasCompositionWidget( mAtlasDock, mComposition ) );
+
+  //set state of atlas controls
+  QgsAtlasComposition* atlasMap = &mComposition->atlasComposition();
+  toggleAtlasControls( atlasMap->enabled() );
+  connect( atlasMap, SIGNAL( toggled( bool ) ), this, SLOT( toggleAtlasControls( bool ) ) );
+  connect( atlasMap, SIGNAL( coverageLayerChanged( QgsVectorLayer* ) ), this, SLOT( updateAtlasMapLayerAction( QgsVectorLayer * ) ) );
+
+  //default printer page setup
+  setPrinterPageDefaults();
+}
+
 void QgsComposer::on_mActionExportAtlasAsPDF_triggered()
 {
   QgsComposition::AtlasMode previousMode = mComposition->atlasMode();
@@ -2811,6 +2866,20 @@ void QgsComposer::readXML( const QDomDocument& doc )
   cleanupAfterTemplateRead();
 }
 
+void QgsComposer::createCompositionWidget()
+{
+  if ( !mComposition )
+  {
+    return;
+  }
+
+  QgsCompositionWidget* compositionWidget = new QgsCompositionWidget( mGeneralDock, mComposition );
+  connect( mComposition, SIGNAL( paperSizeChanged() ), compositionWidget, SLOT( displayCompositionWidthHeight() ) );
+  connect( this, SIGNAL( printAsRasterChanged( bool ) ), compositionWidget, SLOT( setPrintAsRasterCheckBox( bool ) ) );
+  connect( compositionWidget, SIGNAL( pageOrientationChanged( QString ) ), this, SLOT( setPrinterPageOrientation( QString ) ) );
+  mGeneralDock->setWidget( compositionWidget );
+}
+
 void QgsComposer::readXML( const QDomElement& composerElem, const QDomDocument& doc, bool fromTemplate )
 {
   // Set title only if reading from project file
@@ -2837,14 +2906,9 @@ void QgsComposer::readXML( const QDomElement& composerElem, const QDomDocument&
     mComposition->readXML( compositionElem, doc );
   }
 
-  connectSlots();
-
-  //create compositionwidget
-  QgsCompositionWidget* compositionWidget = new QgsCompositionWidget( mGeneralDock, mComposition );
-  connect( mComposition, SIGNAL( paperSizeChanged() ), compositionWidget, SLOT( displayCompositionWidthHeight() ) );
-  connect( this, SIGNAL( printAsRasterChanged( bool ) ), compositionWidget, SLOT( setPrintAsRasterCheckBox( bool ) ) );
-  connect( compositionWidget, SIGNAL( pageOrientationChanged( QString ) ), this, SLOT( setPrinterPageOrientation( QString ) ) );
-  mGeneralDock->setWidget( compositionWidget );
+  connectViewSlots();
+  connectCompositionSlots();
+  createCompositionWidget();
 
   //read and restore all the items
   if ( mComposition )
@@ -2876,20 +2940,12 @@ void QgsComposer::readXML( const QDomElement& composerElem, const QDomDocument&
   //make sure z values are consistent
   mComposition->refreshZList();
 
+  //disconnect from view's compositionSet signal, since that will be emitted automatically
+  disconnect( mView, SIGNAL( compositionSet( QgsComposition* ) ), this, SLOT( setComposition( QgsComposition* ) ) );
   mView->setComposition( mComposition );
+  connect( mView, SIGNAL( compositionSet( QgsComposition* ) ), this, SLOT( setComposition( QgsComposition* ) ) );
 
-  if ( mUndoView )
-  {
-    //init undo/redo buttons
-    mActionUndo->setEnabled( false );
-    mActionRedo->setEnabled( false );
-    if ( mComposition->undoStack() )
-    {
-      mUndoView->setStack( mComposition->undoStack() );
-      connect( mComposition->undoStack(), SIGNAL( canUndoChanged( bool ) ), mActionUndo, SLOT( setEnabled( bool ) ) );
-      connect( mComposition->undoStack(), SIGNAL( canRedoChanged( bool ) ), mActionRedo, SLOT( setEnabled( bool ) ) );
-    }
-  }
+  setupUndoView();
 
   // atlas properties reading
   QDomNodeList atlasNodeList = composerElem.elementsByTagName( "Atlas" );
@@ -2903,19 +2959,9 @@ void QgsComposer::readXML( const QDomElement& composerElem, const QDomDocument&
 
   //set state of atlas controls
   QgsAtlasComposition* atlasMap = &mComposition->atlasComposition();
-  mActionAtlasPreview->setEnabled( atlasMap->enabled() );
-  mActionAtlasPreview->setChecked( false );
-  mActionAtlasFirst->setEnabled( false );
-  mActionAtlasLast->setEnabled( false );
-  mActionAtlasNext->setEnabled( false );
-  mActionAtlasPrev->setEnabled( false );
-  mActionPrintAtlas->setEnabled( atlasMap->enabled() );
-  mActionExportAtlasAsImage->setEnabled( atlasMap->enabled() );
-  mActionExportAtlasAsSVG->setEnabled( atlasMap->enabled() );
-  mActionExportAtlasAsPDF->setEnabled( atlasMap->enabled() );
+  toggleAtlasControls( atlasMap->enabled() );
   connect( atlasMap, SIGNAL( toggled( bool ) ), this, SLOT( toggleAtlasControls( bool ) ) );
   connect( atlasMap, SIGNAL( coverageLayerChanged( QgsVectorLayer* ) ), this, SLOT( updateAtlasMapLayerAction( QgsVectorLayer * ) ) );
-  updateAtlasMapLayerAction( atlasMap->enabled() );
 
   //default printer page setup
   setPrinterPageDefaults();
@@ -2923,6 +2969,24 @@ void QgsComposer::readXML( const QDomElement& composerElem, const QDomDocument&
   setSelectionTool();
 }
 
+void QgsComposer::setupUndoView()
+{
+  if ( !mUndoView || !mComposition )
+  {
+    return;
+  }
+
+  //init undo/redo buttons
+  mActionUndo->setEnabled( false );
+  mActionRedo->setEnabled( false );
+  if ( mComposition->undoStack() )
+  {
+    mUndoView->setStack( mComposition->undoStack() );
+    connect( mComposition->undoStack(), SIGNAL( canUndoChanged( bool ) ), mActionUndo, SLOT( setEnabled( bool ) ) );
+    connect( mComposition->undoStack(), SIGNAL( canRedoChanged( bool ) ), mActionRedo, SLOT( setEnabled( bool ) ) );
+  }
+}
+
 void QgsComposer::restoreGridSettings()
 {
   //restore grid settings

src/app/composer/qgscomposer.h

@@ -429,8 +429,20 @@ class QgsComposer: public QMainWindow, private Ui::QgsComposerBase
 
   private:
 
-    /**Establishes the signal slot connection for the class*/
-    void connectSlots();
+    /**Establishes the signal slot connections from the QgsComposerView to the composer*/
+    void connectViewSlots();
+
+    /**Establishes the signal slot connections from the QgsComposition to the composer*/
+    void connectCompositionSlots();
+
+    /**Establishes other signal slot connections for the composer*/
+    void connectOtherSlots();
+
+    /**Creates the composition widget*/
+    void createCompositionWidget();
+
+    /**Sets up the compositions undo/redo connections*/
+    void setupUndoView();
 
     //! True if a composer map contains a WMS layer
     bool containsWMSLayer() const;
@@ -572,7 +584,6 @@ class QgsComposer: public QMainWindow, private Ui::QgsComposerBase
 
     QgsMapLayerAction* mAtlasFeatureAction;
 
-
   signals:
     void printAsRasterChanged( bool state );
 
@@ -618,6 +629,9 @@ class QgsComposer: public QMainWindow, private Ui::QgsComposerBase
     void activateProtanopePreview();
     void activateDeuteranopePreview();
 
+    //! Sets the composition for the composer window
+    void setComposition( QgsComposition* composition );
+
 };
 
 #endif

src/app/qgisappinterface.h

@@ -177,8 +177,10 @@ class APP_EXPORT QgisAppInterface : public QgisInterface
 
     QgsMessageBar * messageBar();
 
+    // ### QGIS 3: return QgsComposer*, not QgsComposerView*
     QList<QgsComposerView*> activeComposers();
 
+    // ### QGIS 3: return QgsComposer*, not QgsComposerView*
     /** Create a new composer
      * @param title window title for new composer (one will be generated if empty)
      * @return pointer to composer's view
@@ -186,6 +188,7 @@ class APP_EXPORT QgisAppInterface : public QgisInterface
      */
     QgsComposerView* createNewComposer( QString title = QString( "" ) );
 
+    // ### QGIS 3: return QgsComposer*, not QgsComposerView*
     /** Duplicate an existing parent composer from composer view
      * @param composerView pointer to existing composer view
      * @param title window title for duplicated composer (one will be generated if empty)

src/gui/qgscomposerview.cpp

@@ -1574,6 +1574,9 @@ void QgsComposerView::setComposition( QgsComposition* c )
   {
     mVerticalRuler->setComposition( c );
   }
+
+  //emit compositionSet, so that composer windows can update for the new composition
+  emit compositionSet( c );
 }
 
 QgsComposition* QgsComposerView::composition()

src/gui/qgscomposerview.h

@@ -120,8 +120,11 @@ class GUI_EXPORT QgsComposerView: public QGraphicsView
     QgsComposerView::Tool currentTool() const {return mCurrentTool;}
     void setCurrentTool( QgsComposerView::Tool t );
 
-    /**Sets composition (derived from QGraphicsScene)*/
+    /**Sets the composition for the view. If the composition is being set manually and not by a QgsComposer, then this must
+     * be set BEFORE adding any items to the composition.
+    */
     void setComposition( QgsComposition* c );
+
     /**Returns the composition or 0 in case of error*/
     QgsComposition* composition();
 
@@ -259,6 +262,9 @@ class GUI_EXPORT QgsComposerView: public QGraphicsView
     void composerViewShow( QgsComposerView* );
     /**Emitted before composerview is hidden*/
     void composerViewHide( QgsComposerView* );
+
+    /**Emitted when the composition is set for the view*/
+    void compositionSet( QgsComposition* );
 };
 
 #endif