Port and fix unit tests

python/core/layout/qgslayoutframe.sip

@@ -56,6 +56,11 @@ class QgsLayoutFrame: QgsLayoutItem
  :rtype: QgsLayoutMultiFrame
 %End
 
+    virtual QgsLayoutSize minimumSize() const;
+
+    virtual QgsLayoutSize fixedSize() const;
+
+
 
     QRectF extent() const;
 %Docstring

python/core/layout/qgslayoutitem.sip

@@ -276,7 +276,7 @@ class QgsLayoutItem : QgsLayoutObject, QGraphicsRectItem, QgsLayoutUndoObjectInt
  :rtype: ReferencePoint
 %End
 
-    QgsLayoutSize fixedSize() const;
+    virtual QgsLayoutSize fixedSize() const;
 %Docstring
  Returns the fixed size of the item, if applicable, or an empty size if item can be freely
  resized.

python/core/layout/qgslayoutitemattributetable.sip

@@ -29,9 +29,11 @@ class QgsLayoutItemAttributeTable: QgsLayoutTable
       RelationChildren
     };
 
-    QgsLayoutItemAttributeTable( QgsLayout *layout );
+    QgsLayoutItemAttributeTable( QgsLayout *layout /TransferThis/ );
 %Docstring
  Constructor for QgsLayoutItemAttributeTable, attached to the specified ``layout``.
+
+ Ownership is transferred to the layout.
 %End
 
     virtual int type() const;

python/core/layout/qgslayoutitemhtml.sip

@@ -27,9 +27,11 @@ class QgsLayoutItemHtml: QgsLayoutMultiFrame
       ManualHtml
     };
 
-    QgsLayoutItemHtml( QgsLayout *layout );
+    QgsLayoutItemHtml( QgsLayout *layout /TransferThis/ );
 %Docstring
  Constructor for QgsLayoutItemHtml, with the specified parent ``layout``.
+
+ Ownership is transferred to the layout.
 %End
 
     ~QgsLayoutItemHtml();

python/core/layout/qgslayoutitemtexttable.sip

@@ -21,9 +21,11 @@ class QgsLayoutItemTextTable : QgsLayoutTable
 %End
   public:
 
-    QgsLayoutItemTextTable( QgsLayout *layout );
+    QgsLayoutItemTextTable( QgsLayout *layout /TransferThis/ );
 %Docstring
  Constructor for QgsLayoutItemTextTable, for the specified ``layout``.
+
+ Ownership is transferred to the layout.
 %End
 
     virtual int type() const;

src/core/layout/qgslayoutframe.cpp

@@ -35,10 +35,8 @@ QgsLayoutFrame::QgsLayoutFrame( QgsLayout *layout, QgsLayoutMultiFrame *multiFra
       update();
     } );
 
-#if 0 //TODO
     //force recalculation of rect, so that multiframe specified sizes can be applied
-    setSceneRect( QRectF( pos().x(), pos().y(), rect().width(), rect().height() ) );
-#endif
+    refreshItemSize();
   }
 }
 
@@ -51,6 +49,29 @@ QgsLayoutMultiFrame *QgsLayoutFrame::multiFrame() const
 {
   return mMultiFrame;
 }
+
+QgsLayoutSize QgsLayoutFrame::minimumSize() const
+{
+  if ( !mMultiFrame )
+    return QgsLayoutSize();
+
+  //calculate index of frame
+  int frameIndex = mMultiFrame->frameIndex( const_cast< QgsLayoutFrame * >( this ) );
+  //check minimum size
+  return QgsLayoutSize( mMultiFrame->minFrameSize( frameIndex ), QgsUnitTypes::LayoutMillimeters );
+}
+
+QgsLayoutSize QgsLayoutFrame::fixedSize() const
+{
+  if ( !mMultiFrame )
+    return QgsLayoutSize();
+
+  //calculate index of frame
+  int frameIndex = mMultiFrame->frameIndex( const_cast< QgsLayoutFrame * >( this ) );
+  //check fixed size
+  return QgsLayoutSize( mMultiFrame->fixedFrameSize( frameIndex ), QgsUnitTypes::LayoutMillimeters );
+}
+
 #if 0// TODO - save/restore multiframe uuid!
 bool QgsLayoutFrame::writeXml( QDomElement &elem, QDomDocument &doc ) const
 {
@@ -168,37 +189,6 @@ QString QgsLayoutFrame::displayName() const
   return tr( "<Frame>" );
 }
 
-#if 0 //TODO
-void QgsLayoutFrame::setSceneRect( const QRectF &rectangle )
-{
-  QRectF fixedRect = rectangle;
-
-  if ( mMultiFrame )
-  {
-    //calculate index of frame
-    int frameIndex = mMultiFrame->frameIndex( this );
-
-    QSizeF fixedSize = mMultiFrame->fixedFrameSize( frameIndex );
-    if ( fixedSize.width() > 0 )
-    {
-      fixedRect.setWidth( fixedSize.width() );
-    }
-    if ( fixedSize.height() > 0 )
-    {
-      fixedRect.setHeight( fixedSize.height() );
-    }
-
-    //check minimum size
-    QSizeF minSize = mMultiFrame->minFrameSize( frameIndex );
-    fixedRect.setWidth( std::max( minSize.width(), fixedRect.width() ) );
-    fixedRect.setHeight( std::max( minSize.height(), fixedRect.height() ) );
-  }
-
-  QgsComposerItem::setSceneRect( fixedRect );
-}
-#endif
-
-
 void QgsLayoutFrame::draw( QgsRenderContext &context, const QStyleOptionGraphicsItem *itemStyle )
 {
   if ( mMultiFrame )

src/core/layout/qgslayoutframe.h

@@ -64,11 +64,10 @@ class CORE_EXPORT QgsLayoutFrame: public QgsLayoutItem
      */
     QgsLayoutMultiFrame *multiFrame() const;
 
-#if 0 //TODO
-    //Overridden to handle fixed frame sizes set by multi frame
-    void setSceneRect( const QRectF &rectangle ) override;
+    QgsLayoutSize minimumSize() const override;
+    QgsLayoutSize fixedSize() const override;
 
-    void paint( QPainter *painter, const QStyleOptionGraphicsItem *itemStyle, QWidget *pWidget ) override;
+#if 0 //TODO
     void beginItemCommand( const QString &text ) override;
     void endItemCommand() override;
     bool writeXml( QDomElement &elem, QDomDocument &doc ) const override;

src/core/layout/qgslayoutitem.cpp

@@ -1200,8 +1200,15 @@ QSizeF QgsLayoutItem::applyFixedSize( const QSizeF &targetSize )
   {
     return targetSize;
   }
+
+  QSizeF size = targetSize;
   QSizeF fixedSizeLayoutUnits = mLayout->convertToLayoutUnits( fixedSize() );
-  return targetSize.expandedTo( fixedSizeLayoutUnits );
+  if ( fixedSizeLayoutUnits.width() > 0 )
+    size.setWidth( fixedSizeLayoutUnits.width() );
+  if ( fixedSizeLayoutUnits.height() > 0 )
+    size.setHeight( fixedSizeLayoutUnits.height() );
+
+  return size;
 }
 
 void QgsLayoutItem::refreshItemRotation( QPointF *origin )

src/core/layout/qgslayoutitem.h

@@ -299,7 +299,7 @@ class CORE_EXPORT QgsLayoutItem : public QgsLayoutObject, public QGraphicsRectIt
      * \see setFixedSize()
      * \see minimumSize()
     */
-    QgsLayoutSize fixedSize() const { return mFixedSize; }
+    virtual QgsLayoutSize fixedSize() const { return mFixedSize; }
 
     /**
      * Returns the minimum allowed size of the item, if applicable, or an empty size if item can be freely

src/core/layout/qgslayoutitemattributetable.cpp

@@ -679,8 +679,11 @@ void QgsLayoutItemAttributeTable::setWrapString( const QString &wrapString )
   emit changed();
 }
 
-bool QgsLayoutItemAttributeTable::writePropertiesToElement( QDomElement &tableElem, QDomDocument &, const QgsReadWriteContext & ) const
+bool QgsLayoutItemAttributeTable::writePropertiesToElement( QDomElement &tableElem, QDomDocument &doc, const QgsReadWriteContext &context ) const
 {
+  if ( !QgsLayoutTable::writePropertiesToElement( tableElem, doc, context ) )
+    return false;
+
   tableElem.setAttribute( QStringLiteral( "source" ), QString::number( static_cast< int >( mSource ) ) );
   tableElem.setAttribute( QStringLiteral( "relationId" ), mRelationId );
   tableElem.setAttribute( QStringLiteral( "showUniqueRowsOnly" ), mShowUniqueRowsOnly );
@@ -706,7 +709,7 @@ bool QgsLayoutItemAttributeTable::writePropertiesToElement( QDomElement &tableEl
   return true;
 }
 
-bool QgsLayoutItemAttributeTable::readPropertiesFromElement( const QDomElement &itemElem, const QDomDocument &, const QgsReadWriteContext & )
+bool QgsLayoutItemAttributeTable::readPropertiesFromElement( const QDomElement &itemElem, const QDomDocument &doc, const QgsReadWriteContext &context )
 {
   QgsVectorLayer *prevLayer = sourceLayer();
   if ( prevLayer )
@@ -715,6 +718,9 @@ bool QgsLayoutItemAttributeTable::readPropertiesFromElement( const QDomElement &
     disconnect( prevLayer, &QgsVectorLayer::layerModified, this, &QgsLayoutTable::refreshAttributes );
   }
 
+  if ( !QgsLayoutTable::readPropertiesFromElement( itemElem, doc, context ) )
+    return false;
+
   mSource = QgsLayoutItemAttributeTable::ContentSource( itemElem.attribute( QStringLiteral( "source" ), QStringLiteral( "0" ) ).toInt() );
   mRelationId = itemElem.attribute( QStringLiteral( "relationId" ), QLatin1String( "" ) );
 

src/core/layout/qgslayoutitemattributetable.h

@@ -50,8 +50,10 @@ class CORE_EXPORT QgsLayoutItemAttributeTable: public QgsLayoutTable
 
     /**
      * Constructor for QgsLayoutItemAttributeTable, attached to the specified \a layout.
+     *
+     * Ownership is transferred to the layout.
      */
-    QgsLayoutItemAttributeTable( QgsLayout *layout );
+    QgsLayoutItemAttributeTable( QgsLayout *layout SIP_TRANSFERTHIS );
 
     int type() const override;
     QString stringType() const override;

src/core/layout/qgslayoutitemhtml.h

@@ -48,8 +48,10 @@ class CORE_EXPORT QgsLayoutItemHtml: public QgsLayoutMultiFrame
 
     /**
      * Constructor for QgsLayoutItemHtml, with the specified parent \a layout.
+     *
+     * Ownership is transferred to the layout.
      */
-    QgsLayoutItemHtml( QgsLayout *layout );
+    QgsLayoutItemHtml( QgsLayout *layout SIP_TRANSFERTHIS );
 
     ~QgsLayoutItemHtml();
 

src/core/layout/qgslayoutitemtexttable.h

@@ -36,8 +36,10 @@ class CORE_EXPORT QgsLayoutItemTextTable : public QgsLayoutTable
 
     /**
      * Constructor for QgsLayoutItemTextTable, for the specified \a layout.
+     *
+     * Ownership is transferred to the layout.
      */
-    QgsLayoutItemTextTable( QgsLayout *layout );
+    QgsLayoutItemTextTable( QgsLayout *layout SIP_TRANSFERTHIS );
 
     int type() const override;
     QString stringType() const override;

src/core/layout/qgslayoutmultiframe.cpp

@@ -220,7 +220,6 @@ void QgsLayoutMultiFrame::recalculateFrameSizes()
 
 void QgsLayoutMultiFrame::recalculateFrameRects()
 {
-#if 0 //TODO
   if ( mFrameItems.empty() )
   {
     //no frames, nothing to do
@@ -230,10 +229,8 @@ void QgsLayoutMultiFrame::recalculateFrameRects()
   const QList< QgsLayoutFrame * > frames = mFrameItems;
   for ( QgsLayoutFrame *frame : frames )
   {
-    frame->setSceneRect( QRectF( frame->scenePos().x(), frame->scenePos().y(),
-                                 frame->rect().width(), frame->rect().height() ) );
+    frame->refreshItemSize();
   }
-#endif
 }
 
 QgsLayoutFrame *QgsLayoutMultiFrame::createNewFrame( QgsLayoutFrame *currentFrame, QPointF pos, QSizeF size )

tests/src/core/CMakeLists.txt

@@ -147,6 +147,7 @@ SET(TESTS
  testqgslayoutpicture.cpp
  testqgslayoutscalebar.cpp
  testqgslayoutshapes.cpp
+ testqgslayouttable.cpp
  testqgslayoutunits.cpp
  testqgslayoututils.cpp
  testqgslegendrenderer.cpp

tests/src/core/testqgslayoutitem.cpp

@@ -1074,6 +1074,11 @@ void TestQgsLayoutItem::fixedSize()
   QGSCOMPARENEAR( item->rect().width(), 2.0 * 25.4, 4 * DBL_EPSILON );
   QGSCOMPARENEAR( item->rect().height(), 4.0 * 25.4, 4 * DBL_EPSILON );
 
+  item->attemptResize( QgsLayoutSize( 7.0, 8.0, QgsUnitTypes::LayoutInches ) );
+  //check size matches fixed item size converted to mm
+  QGSCOMPARENEAR( item->rect().width(), 2.0 * 25.4, 4 * DBL_EPSILON );
+  QGSCOMPARENEAR( item->rect().height(), 4.0 * 25.4, 4 * DBL_EPSILON );
+
   //check that setting a fixed size applies this size immediately
   item->updateFixedSize( QgsLayoutSize( 150, 250, QgsUnitTypes::LayoutMillimeters ) );
   QGSCOMPARENEAR( item->rect().width(), 150.0, 4 * DBL_EPSILON );
@@ -1119,8 +1124,8 @@ void TestQgsLayoutItem::minSize()
   //try to resize to less than minimum size
   fixedMinItem->attemptResize( QgsLayoutSize( 1.0, 0.5, QgsUnitTypes::LayoutPoints ) );
   //check size matches fixed item size, not minimum size (converted to mm)
-  QGSCOMPARENEAR( fixedMinItem->rect().width(), 50.0, 4 * DBL_EPSILON );
-  QGSCOMPARENEAR( fixedMinItem->rect().height(), 90.0, 4 * DBL_EPSILON );
+  QGSCOMPARENEAR( fixedMinItem->rect().width(), 20.0, 4 * DBL_EPSILON );
+  QGSCOMPARENEAR( fixedMinItem->rect().height(), 40.0, 4 * DBL_EPSILON );
 }
 
 void TestQgsLayoutItem::move()