Navigation Menu

Skip to content

Commit

Permalink
Fix ownership issue with layout guides
Browse files Browse the repository at this point in the history
  • Loading branch information
nyalldawson committed Aug 7, 2017
1 parent 94362fe commit 3fd2e09
Show file tree
Hide file tree
Showing 9 changed files with 171 additions and 99 deletions.
2 changes: 2 additions & 0 deletions python/core/layout/qgslayout.sip
Expand Up @@ -39,6 +39,8 @@ class QgsLayout : QGraphicsScene, QgsExpressionContextGenerator
called on the new layout.
%End

~QgsLayout();

void initializeDefaults();
%Docstring
Initializes an empty layout, e.g. by adding a default page to the layout. This should be called after creating
Expand Down
30 changes: 18 additions & 12 deletions python/core/layout/qgslayoutguidecollection.sip
Expand Up @@ -26,7 +26,7 @@ class QgsLayoutGuide : QObject
Vertical,
};

QgsLayoutGuide( Orientation orientation, const QgsLayoutMeasurement &position );
QgsLayoutGuide( Orientation orientation, const QgsLayoutMeasurement &position, QgsLayoutItemPage *page );
%Docstring
Constructor for a new guide with the specified ``orientation`` and
initial ``position``.
Expand All @@ -36,6 +36,8 @@ class QgsLayoutGuide : QObject
the corresponding layout for you.
%End

~QgsLayoutGuide();

QgsLayout *layout() const;
%Docstring
Returns the layout the guide belongs to.
Expand Down Expand Up @@ -82,21 +84,17 @@ class QgsLayoutGuide : QObject
.. seealso:: position()
%End

int page() const;
QgsLayoutItemPage *page();
%Docstring
Returns the page number of the guide.

Page numbering begins at 0.
Returns the page the guide is contained within.

.. seealso:: setPage()
:rtype: int
:rtype: QgsLayoutItemPage
%End

void setPage( int page );
void setPage( QgsLayoutItemPage *page );
%Docstring
Sets the ``page`` number of the guide.

Page numbering begins at 0.
Sets the ``page`` the guide is contained within.

.. seealso:: page()
%End
Expand Down Expand Up @@ -155,9 +153,10 @@ class QgsLayoutGuideCollection : QAbstractTableModel
LayoutPositionRole,
};

QgsLayoutGuideCollection( QgsLayout *layout );
QgsLayoutGuideCollection( QgsLayout *layout, QgsLayoutPageCollection *pageCollection );
%Docstring
Constructor for QgsLayoutGuideCollection belonging to the specified layout.
Constructor for QgsLayoutGuideCollection belonging to the specified layout,
and linked to the specified ``pageCollection``.
%End
~QgsLayoutGuideCollection();

Expand Down Expand Up @@ -210,9 +209,16 @@ class QgsLayoutGuideCollection : QAbstractTableModel
Returns the list of guides contained in the collection with the specified
``orientation`` and on a matching ``page``.
If ``page`` is -1, guides from all pages will be returned.
.. seealso:: guidesOnPage()
:rtype: list of QgsLayoutGuide
%End

QList< QgsLayoutGuide * > guidesOnPage( int page );
%Docstring
Returns the list of guides contained on a matching ``page``.
.. seealso:: guides()
:rtype: list of QgsLayoutGuide
%End

bool visible() const;
%Docstring
Expand Down
6 changes: 2 additions & 4 deletions src/app/layout/qgslayoutguidewidget.cpp
Expand Up @@ -60,15 +60,13 @@ QgsLayoutGuideWidget::QgsLayoutGuideWidget( QWidget *parent, QgsLayout *layout,

void QgsLayoutGuideWidget::addHorizontalGuide()
{
std::unique_ptr< QgsLayoutGuide > newGuide( new QgsLayoutGuide( QgsLayoutGuide::Horizontal, QgsLayoutMeasurement( 0 ) ) );
newGuide->setPage( mPage );
std::unique_ptr< QgsLayoutGuide > newGuide( new QgsLayoutGuide( QgsLayoutGuide::Horizontal, QgsLayoutMeasurement( 0 ), mLayout->pageCollection()->page( mPage ) ) );
mLayout->guides().addGuide( newGuide.release() );
}

void QgsLayoutGuideWidget::addVerticalGuide()
{
std::unique_ptr< QgsLayoutGuide > newGuide( new QgsLayoutGuide( QgsLayoutGuide::Vertical, QgsLayoutMeasurement( 0 ) ) );
newGuide->setPage( mPage );
std::unique_ptr< QgsLayoutGuide > newGuide( new QgsLayoutGuide( QgsLayoutGuide::Vertical, QgsLayoutMeasurement( 0 ), mLayout->pageCollection()->page( mPage ) ) );
mLayout->guides().addGuide( newGuide.release() );
}

Expand Down
8 changes: 7 additions & 1 deletion src/core/layout/qgslayout.cpp
Expand Up @@ -22,13 +22,19 @@ QgsLayout::QgsLayout( QgsProject *project )
: QGraphicsScene()
, mProject( project )
, mSnapper( QgsLayoutSnapper( this ) )
, mGuideCollection( new QgsLayoutGuideCollection( this ) )
, mPageCollection( new QgsLayoutPageCollection( this ) )
, mGuideCollection( new QgsLayoutGuideCollection( this, mPageCollection.get() ) )
{
// just to make sure - this should be the default, but maybe it'll change in some future Qt version...
setBackgroundBrush( Qt::NoBrush );
}

QgsLayout::~QgsLayout()
{
// delete guide collection FIRST, since it depends on the page collection
mGuideCollection.reset();
}

void QgsLayout::initializeDefaults()
{
// default to a A4 landscape page
Expand Down
5 changes: 4 additions & 1 deletion src/core/layout/qgslayout.h
Expand Up @@ -59,6 +59,8 @@ class CORE_EXPORT QgsLayout : public QGraphicsScene, public QgsExpressionContext
*/
QgsLayout( QgsProject *project );

~QgsLayout();

/**
* Initializes an empty layout, e.g. by adding a default page to the layout. This should be called after creating
* a new layout.
Expand Down Expand Up @@ -309,10 +311,11 @@ class CORE_EXPORT QgsLayout : public QGraphicsScene, public QgsExpressionContext
QgsLayoutContext mContext;
QgsLayoutSnapper mSnapper;
QgsLayoutGridSettings mGridSettings;
std::unique_ptr< QgsLayoutGuideCollection > mGuideCollection;

std::unique_ptr< QgsLayoutPageCollection > mPageCollection;

std::unique_ptr< QgsLayoutGuideCollection > mGuideCollection;

};

#endif //QGSLAYOUT_H
Expand Down
102 changes: 73 additions & 29 deletions src/core/layout/qgslayoutguidecollection.cpp
Expand Up @@ -23,20 +23,20 @@
// QgsLayoutGuide
//

QgsLayoutGuide::QgsLayoutGuide( Orientation orientation, const QgsLayoutMeasurement &position )
QgsLayoutGuide::QgsLayoutGuide( Orientation orientation, const QgsLayoutMeasurement &position, QgsLayoutItemPage *page )
: QObject( nullptr )
, mOrientation( orientation )
, mPosition( position )
, mLineItem( new QGraphicsLineItem() )
, mPage( page )
{}

QgsLayoutGuide::~QgsLayoutGuide()
{
mLineItem->hide();
mLineItem->setZValue( QgsLayout::ZGuide );
QPen linePen( Qt::DotLine );
linePen.setColor( Qt::red );
// use a pen width of 0, since this activates a cosmetic pen
// which doesn't scale with the composer and keeps a constant size
linePen.setWidthF( 0 );
mLineItem->setPen( linePen );
if ( mLayout && mLineItem )
{
mLayout->removeItem( mLineItem );
delete mLineItem;
}
}

QgsLayoutMeasurement QgsLayoutGuide::position() const
Expand All @@ -51,56 +51,58 @@ void QgsLayoutGuide::setPosition( const QgsLayoutMeasurement &position )
emit positionChanged();
}

int QgsLayoutGuide::page() const
QgsLayoutItemPage *QgsLayoutGuide::page()
{
return mPage;
}

void QgsLayoutGuide::setPage( int page )
void QgsLayoutGuide::setPage( QgsLayoutItemPage *page )
{
mPage = page;
update();
}

void QgsLayoutGuide::update()
{
if ( !mLayout )
if ( !mLayout || !mLineItem )
return;

// first find matching page
if ( mPage >= mLayout->pageCollection()->pageCount() )
if ( !mPage )
{
mLineItem->hide();
return;
}

QgsLayoutItemPage *page = mLayout->pageCollection()->page( mPage );
mLineItem->setParentItem( page );
if ( mLineItem->parentItem() != mPage )
{
mLineItem->setParentItem( mPage );
}
double layoutPos = mLayout->convertToLayoutUnits( mPosition );
bool showGuide = mLayout->guides().visible();
switch ( mOrientation )
{
case Horizontal:
if ( layoutPos > page->rect().height() )
if ( layoutPos > mPage->rect().height() )
{
mLineItem->hide();
}
else
{
mLineItem->setLine( 0, layoutPos, page->rect().width(), layoutPos );
mLineItem->setLine( 0, layoutPos, mPage->rect().width(), layoutPos );
mLineItem->setVisible( showGuide );
}

break;

case Vertical:
if ( layoutPos > page->rect().width() )
if ( layoutPos > mPage->rect().width() )
{
mLineItem->hide();
}
else
{
mLineItem->setLine( layoutPos, 0, layoutPos, page->rect().height() );
mLineItem->setLine( layoutPos, 0, layoutPos, mPage->rect().height() );
mLineItem->setVisible( showGuide );
}

Expand All @@ -110,11 +112,14 @@ void QgsLayoutGuide::update()

QGraphicsLineItem *QgsLayoutGuide::item()
{
return mLineItem.get();
return mLineItem;
}

double QgsLayoutGuide::layoutPosition() const
{
if ( !mLineItem )
return -999;

switch ( mOrientation )
{
case Horizontal:
Expand All @@ -128,6 +133,9 @@ double QgsLayoutGuide::layoutPosition() const

void QgsLayoutGuide::setLayoutPosition( double position )
{
if ( !mLayout )
return;

double p = 0;
switch ( mOrientation )
{
Expand All @@ -152,7 +160,21 @@ QgsLayout *QgsLayoutGuide::layout() const
void QgsLayoutGuide::setLayout( QgsLayout *layout )
{
mLayout = layout;
mLayout->addItem( mLineItem.get() );

if ( !mLineItem )
{
mLineItem = new QGraphicsLineItem();
mLineItem->hide();
mLineItem->setZValue( QgsLayout::ZGuide );
QPen linePen( Qt::DotLine );
linePen.setColor( Qt::red );
// use a pen width of 0, since this activates a cosmetic pen
// which doesn't scale with the composer and keeps a constant size
linePen.setWidthF( 0 );
mLineItem->setPen( linePen );
}

mLayout->addItem( mLineItem );
update();
}

Expand All @@ -167,12 +189,15 @@ QgsLayoutGuide::Orientation QgsLayoutGuide::orientation() const
// QgsLayoutGuideCollection
//

QgsLayoutGuideCollection::QgsLayoutGuideCollection( QgsLayout *layout )
QgsLayoutGuideCollection::QgsLayoutGuideCollection( QgsLayout *layout, QgsLayoutPageCollection *pageCollection )
: QAbstractTableModel( layout )
, mLayout( layout )
, mPageCollection( pageCollection )
{
QFont f;
mHeaderSize = QFontMetrics( f ).width( "XX" );

connect( mPageCollection, &QgsLayoutPageCollection::pageAboutToBeRemoved, this, &QgsLayoutGuideCollection::pageAboutToBeRemoved );
}

QgsLayoutGuideCollection::~QgsLayoutGuideCollection()
Expand Down Expand Up @@ -223,7 +248,7 @@ QVariant QgsLayoutGuideCollection::data( const QModelIndex &index, int role ) co
return guide->position().units();

case PageRole:
return guide->page();
return mPageCollection->pageNumber( guide->page() );

case LayoutPositionRole:
return guide->layoutPosition();
Expand Down Expand Up @@ -358,23 +383,23 @@ void QgsLayoutGuideCollection::clear()

void QgsLayoutGuideCollection::applyGuidesToAllOtherPages( int sourcePage )
{
QgsLayoutItemPage *page = mPageCollection->page( sourcePage );
// remove other page's guides
Q_FOREACH ( QgsLayoutGuide *guide, mGuides )
{
if ( guide->page() != sourcePage )
if ( guide->page() != page )
removeGuide( guide );
}

// remaining guides belong to source page - clone them to other pages
Q_FOREACH ( QgsLayoutGuide *guide, mGuides )
{
for ( int p = 0; p < mLayout->pageCollection()->pageCount(); ++p )
for ( int p = 0; p < mPageCollection->pageCount(); ++p )
{
if ( p == sourcePage )
continue;

std::unique_ptr< QgsLayoutGuide> newGuide( new QgsLayoutGuide( guide->orientation(), guide->position() ) );
newGuide->setPage( p );
std::unique_ptr< QgsLayoutGuide> newGuide( new QgsLayoutGuide( guide->orientation(), guide->position(), mPageCollection->page( p ) ) );
newGuide->setLayout( mLayout );
if ( newGuide->item()->isVisible() )
{
Expand All @@ -399,7 +424,18 @@ QList<QgsLayoutGuide *> QgsLayoutGuideCollection::guides( QgsLayoutGuide::Orient
Q_FOREACH ( QgsLayoutGuide *guide, mGuides )
{
if ( guide->orientation() == orientation && guide->item()->isVisible() &&
( page < 0 || page == guide->page() ) )
( page < 0 || mPageCollection->page( page ) == guide->page() ) )
res << guide;
}
return res;
}

QList<QgsLayoutGuide *> QgsLayoutGuideCollection::guidesOnPage( int page )
{
QList<QgsLayoutGuide *> res;
Q_FOREACH ( QgsLayoutGuide *guide, mGuides )
{
if ( mPageCollection->page( page ) == guide->page() )
res << guide;
}
return res;
Expand All @@ -416,6 +452,14 @@ void QgsLayoutGuideCollection::setVisible( bool visible )
update();
}

void QgsLayoutGuideCollection::pageAboutToBeRemoved( int pageNumber )
{
Q_FOREACH ( QgsLayoutGuide *guide, guidesOnPage( pageNumber ) )
{
removeGuide( guide );
}
}



//
Expand Down

0 comments on commit 3fd2e09

Please sign in to comment.