Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix incorrect annotation scaling when exporting layouts
Previously, annotation size and position always used pixel units. This
did not work well when exporting layouts, resulting in tiny annotations
(it also caused issues when moving projects between hidpi/non hidpi
displays).

Instead, use millimeters for annotation size and position so that the
appearance is consistent across displays and works correctly in layout
exports.

Add lots of unit tests covering this.

Fixes #18373
  • Loading branch information
nyalldawson committed Apr 30, 2019
1 parent 8f02861 commit 74307d6
Show file tree
Hide file tree
Showing 43 changed files with 456 additions and 143 deletions.
62 changes: 54 additions & 8 deletions python/core/auto_generated/annotations/qgsannotation.sip.in
Expand Up @@ -143,34 +143,80 @@ the relative percentage for the position compared to the map width and height.
.. seealso:: :py:func:`relativePosition`
%End

void setFrameOffsetFromReferencePoint( QPointF offset );
void setFrameOffsetFromReferencePoint( QPointF offset ) /Deprecated/;
%Docstring
Sets the annotation's frame's offset from the mapPosition() reference point.
Sets the annotation's frame's offset (in pixels) from the mapPosition() reference point.

.. seealso:: :py:func:`frameOffsetFromReferencePoint`

.. deprecated:: use setFrameOffsetFromReferencePointMm() instead
%End

QPointF frameOffsetFromReferencePoint() const;
QPointF frameOffsetFromReferencePoint() const /Deprecated/;
%Docstring
Returns the annotation's frame's offset from the mapPosition() reference point.
Returns the annotation's frame's offset (in pixels) from the mapPosition() reference point.

.. seealso:: :py:func:`setFrameOffsetFromReferencePoint`

.. deprecated:: use frameOffsetFromReferencePointMm() instead
%End

void setFrameOffsetFromReferencePointMm( QPointF offset );
%Docstring
Sets the annotation's frame's offset (in millimeters) from the mapPosition() reference point.

.. seealso:: :py:func:`frameOffsetFromReferencePointMm`

.. versionadded:: 3.4.8
%End

void setFrameSize( QSizeF size );
QPointF frameOffsetFromReferencePointMm() const;
%Docstring
Sets the size of the annotation's frame (the main area in which
Returns the annotation's frame's offset (in millimeters) from the mapPosition() reference point.

.. seealso:: :py:func:`setFrameOffsetFromReferencePointMm`

.. versionadded:: 3.4.8
%End

void setFrameSize( QSizeF size ) /Deprecated/;
%Docstring
Sets the size (in pixels) of the annotation's frame (the main area in which
the annotation's content is drawn).

.. seealso:: :py:func:`frameSize`

.. deprecated:: use setFrameSizeMm() instead
%End

QSizeF frameSize() const;
QSizeF frameSize() const /Deprecated/;
%Docstring
Returns the size of the annotation's frame (the main area in which
Returns the size (in pixels) of the annotation's frame (the main area in which
the annotation's content is drawn).

.. seealso:: :py:func:`setFrameSize`

.. deprecated:: use frameSizeMm() instead
%End

void setFrameSizeMm( QSizeF size );
%Docstring
Sets the size (in millimeters) of the annotation's frame (the main area in which
the annotation's content is drawn).

.. seealso:: :py:func:`frameSizeMm`

.. versionadded:: 3.4.8
%End

QSizeF frameSizeMm() const;
%Docstring
Returns the size (in millimeters) of the annotation's frame (the main area in which
the annotation's content is drawn).

.. seealso:: :py:func:`setFrameSizeMm`

.. versionadded:: 3.4.8
%End

void setContentsMargin( const QgsMargins &margins );
Expand Down
46 changes: 23 additions & 23 deletions src/app/qgsmaptoolannotation.cpp
Expand Up @@ -50,26 +50,19 @@ QDialog *QgsMapToolAnnotation::createItemEditor( QgsMapCanvasAnnotationItem *ite

QgsAnnotation *annotation = item->annotation();

QgsTextAnnotation *tItem = dynamic_cast<QgsTextAnnotation *>( annotation );
if ( tItem )
if ( qobject_cast<QgsTextAnnotation *>( annotation ) )
{
return new QgsTextAnnotationDialog( item );
}

QgsFormAnnotation *fItem = dynamic_cast<QgsFormAnnotation *>( annotation );
if ( fItem )
else if ( qobject_cast<QgsFormAnnotation *>( annotation ) )
{
return new QgsFormAnnotationDialog( item );
}

QgsHtmlAnnotation *hItem = dynamic_cast<QgsHtmlAnnotation *>( annotation );
if ( hItem )
else if ( qobject_cast<QgsHtmlAnnotation *>( annotation ) )
{
return new QgsHtmlAnnotationDialog( item );
}

QgsSvgAnnotation *sItem = dynamic_cast<QgsSvgAnnotation *>( annotation );
if ( sItem )
else if ( qobject_cast<QgsSvgAnnotation *>( annotation ) )
{
return new QgsSvgAnnotationDialog( item );
}
Expand Down Expand Up @@ -124,7 +117,7 @@ void QgsMapToolAnnotation::canvasPressEvent( QgsMapMouseEvent *e )
annotation->setMapPositionCrs( mCanvas->mapSettings().destinationCrs() );
annotation->setRelativePosition( QPointF( e->pos().x() / mCanvas->width(),
e->pos().y() / mCanvas->height() ) );
annotation->setFrameSize( QSizeF( 200, 100 ) );
annotation->setFrameSizeMm( QSizeF( 50, 25 ) );

QgsProject::instance()->annotationManager()->addAnnotation( annotation );

Expand Down Expand Up @@ -192,7 +185,11 @@ void QgsMapToolAnnotation::canvasMoveEvent( QgsMapMouseEvent *e )
QPointF newCanvasPos = item->pos() + ( e->pos() - mLastMousePosition );
if ( annotation->hasFixedMapPosition() )
{
annotation->setFrameOffsetFromReferencePoint( annotation->frameOffsetFromReferencePoint() + ( e->pos() - mLastMousePosition ) );
const double pixelToMmScale = 25.4 / mCanvas->logicalDpiX();
const double deltaX = pixelToMmScale * ( e->pos().x() - mLastMousePosition.x() );
const double deltaY = pixelToMmScale * ( e->pos().y() - mLastMousePosition.y() );
annotation->setFrameOffsetFromReferencePointMm( QPointF( annotation->frameOffsetFromReferencePointMm().x() + deltaX,
annotation->frameOffsetFromReferencePointMm().y() + deltaY ) );
annotation->setRelativePosition( QPointF( newCanvasPos.x() / mCanvas->width(),
newCanvasPos.y() / mCanvas->height() ) );
}
Expand All @@ -209,9 +206,12 @@ void QgsMapToolAnnotation::canvasMoveEvent( QgsMapMouseEvent *e )
else if ( mCurrentMoveAction != QgsMapCanvasAnnotationItem::NoAction )
{
//handle the frame resize actions
QSizeF size = annotation->frameSize();
double xmin = annotation->frameOffsetFromReferencePoint().x();
double ymin = annotation->frameOffsetFromReferencePoint().y();

const double pixelToMmScale = 25.4 / mCanvas->logicalDpiX();

QSizeF size = annotation->frameSizeMm();
double xmin = annotation->frameOffsetFromReferencePointMm().x();
double ymin = annotation->frameOffsetFromReferencePointMm().y();
double xmax = xmin + size.width();
double ymax = ymin + size.height();
double relPosX = annotation->relativePosition().x();
Expand All @@ -221,27 +221,27 @@ void QgsMapToolAnnotation::canvasMoveEvent( QgsMapMouseEvent *e )
mCurrentMoveAction == QgsMapCanvasAnnotationItem::ResizeFrameRightDown ||
mCurrentMoveAction == QgsMapCanvasAnnotationItem::ResizeFrameRightUp )
{
xmax += e->pos().x() - mLastMousePosition.x();
xmax += pixelToMmScale * ( e->pos().x() - mLastMousePosition.x() );
}
if ( mCurrentMoveAction == QgsMapCanvasAnnotationItem::ResizeFrameLeft ||
mCurrentMoveAction == QgsMapCanvasAnnotationItem::ResizeFrameLeftDown ||
mCurrentMoveAction == QgsMapCanvasAnnotationItem::ResizeFrameLeftUp )
{
xmin += e->pos().x() - mLastMousePosition.x();
xmin += pixelToMmScale * ( e->pos().x() - mLastMousePosition.x() );
relPosX = ( relPosX * mCanvas->width() + e->pos().x() - mLastMousePosition.x() ) / static_cast<double>( mCanvas->width() );
}
if ( mCurrentMoveAction == QgsMapCanvasAnnotationItem::ResizeFrameUp ||
mCurrentMoveAction == QgsMapCanvasAnnotationItem::ResizeFrameLeftUp ||
mCurrentMoveAction == QgsMapCanvasAnnotationItem::ResizeFrameRightUp )
{
ymin += e->pos().y() - mLastMousePosition.y();
ymin += pixelToMmScale * ( e->pos().y() - mLastMousePosition.y() );
relPosY = ( relPosY * mCanvas->height() + e->pos().y() - mLastMousePosition.y() ) / static_cast<double>( mCanvas->height() );
}
if ( mCurrentMoveAction == QgsMapCanvasAnnotationItem::ResizeFrameDown ||
mCurrentMoveAction == QgsMapCanvasAnnotationItem::ResizeFrameLeftDown ||
mCurrentMoveAction == QgsMapCanvasAnnotationItem::ResizeFrameRightDown )
{
ymax += e->pos().y() - mLastMousePosition.y();
ymax += pixelToMmScale * ( e->pos().y() - mLastMousePosition.y() );
}

//switch min / max if necessary
Expand All @@ -259,8 +259,8 @@ void QgsMapToolAnnotation::canvasMoveEvent( QgsMapMouseEvent *e )
ymin = tmp;
}

annotation->setFrameOffsetFromReferencePoint( QPointF( xmin, ymin ) );
annotation->setFrameSize( QSizeF( xmax - xmin, ymax - ymin ) );
annotation->setFrameOffsetFromReferencePointMm( QPointF( xmin, ymin ) );
annotation->setFrameSizeMm( QSizeF( xmax - xmin, ymax - ymin ) );
annotation->setRelativePosition( QPointF( relPosX, relPosY ) );
item->update();
QgsProject::instance()->setDirty( true );
Expand Down Expand Up @@ -349,7 +349,7 @@ void QgsMapToolAnnotation::toggleTextItemVisibilities()
QList<QgsMapCanvasAnnotationItem *> itemList = annotationItems();
Q_FOREACH ( QgsMapCanvasAnnotationItem *item, itemList )
{
QgsTextAnnotation *textItem = dynamic_cast<QgsTextAnnotation *>( item->annotation() );
QgsTextAnnotation *textItem = qobject_cast<QgsTextAnnotation *>( item->annotation() );
if ( textItem )
{
textItem->setVisible( !textItem->isVisible() );
Expand Down

0 comments on commit 74307d6

Please sign in to comment.