Fix annotation position when maps are rotated, remove hacks

Adds a new interface class QgsAnnotation, and allows for removal
of a bunch of hacks in QgsComposerMap without breaking 2.x API

python/core/core.sip

@@ -18,6 +18,7 @@
 
 %Include qgis.sip
 
+%Include qgsannotation.sip
 %Include qgsapplication.sip
 %Include qgsaction.sip
 %Include qgsactionmanager.sip

python/core/qgsannotation.sip

@@ -0,0 +1,58 @@
+/** \ingroup core
+ * \class QgsAnnotation
+ * \note added in QGIS 3.0
+ *
+ *  An interface for annotation type items.
+ */
+
+class QgsAnnotation
+{
+%TypeHeaderCode
+#include <qgsannotation.h>
+%End
+  public:
+
+    //! Returns true if the annotation should be shown.
+    virtual bool showItem() const = 0;
+
+    /** Returns true if the annotation is attached to a fixed map position, or
+     * false if the annotation uses a position relative to the current map
+     * extent.
+     * @see mapPosition()
+     * @see relativePositon()
+     */
+    //TODO QGIS 3 - rename to hasFixedMapPosition()
+    virtual bool mapPositionFixed() const = 0;
+
+    /** Returns the map position of the annotation, if it is attached to a fixed map
+     * position.
+     * @see mapPositionFixed()
+     * @see mapPositionCrs()
+     */
+    virtual QgsPoint mapPosition() const;
+
+    /** Returns the CRS of the map position, or an invalid CRS if the annotation does
+     * not have a fixed map position.
+    */
+    virtual QgsCoordinateReferenceSystem mapPositionCrs() const;
+
+    /** Returns the relative position of the annotation, if it is not attached to a fixed map
+     * position. The coordinates in the return point should be between 0 and 1, and represent
+     * the relative percentage for the position compared to the map width and height.
+     * @see mapPositionFixed()
+     */
+    virtual QPointF relativePosition() const;
+
+    /** Returns a scaling factor which should be applied to painters before rendering
+     * the item.
+     */
+    virtual double scaleFactor() const = 0;
+
+    //! deprecated - do not use
+    // TODO QGIS 3.0 - remove
+    virtual void setItemData( int role, const QVariant& value ) = 0;
+
+    //! Paint the annotation to a destination painter
+    virtual void paint( QPainter * painter, const QStyleOptionGraphicsItem * option, QWidget * widget = nullptr ) = 0;
+
+};

python/gui/qgsannotationitem.sip

@@ -8,7 +8,7 @@
 #include "qgstextannotationitem.h"
 %End
 
-class QgsAnnotationItem: QgsMapCanvasItem
+class QgsAnnotationItem: QgsMapCanvasItem, QgsAnnotation
 {
 %TypeHeaderCode
 #include <qgsannotationitem.h>
@@ -66,6 +66,12 @@ class QgsAnnotationItem: QgsMapCanvasItem
     virtual void setMapPosition( const QgsPoint& pos );
     QgsPoint mapPosition() const;
 
+    virtual QPointF relativePosition() const;
+
+    virtual double scaleFactor() const;
+
+    virtual bool showItem() const;
+
     /** Sets the CRS of the map position.
       @param crs the CRS to set */
     virtual void setMapPositionCrs( const QgsCoordinateReferenceSystem& crs );
@@ -97,18 +103,28 @@ class QgsAnnotationItem: QgsMapCanvasItem
     void _writeXml( QDomDocument& doc, QDomElement& itemElem ) const;
     void _readXml( const QDomDocument& doc, const QDomElement& annotationElem );
 
+    virtual void setItemData( int role, const QVariant& value );
+    virtual void paint( QPainter * painter, const QStyleOptionGraphicsItem * option, QWidget * widget = nullptr );
+    void paint( QPainter* painter );
+
   protected:
     void updateBoundingRect();
     /** Check where to attach the balloon connection between frame and map point*/
     void updateBalloon();
 
-    void drawFrame( QPainter* p );
-    void drawMarkerSymbol( QPainter* p );
-    void drawSelectionBoxes( QPainter* p );
+    //! Draws the annotation frame to a destination painter
+    void drawFrame( QPainter* p ) const;
+
+    //! Draws the map position marker symbol to a destination painter
+    void drawMarkerSymbol( QPainter* p ) const;
+
+    //! Draws selection handles around the item
+    void drawSelectionBoxes( QPainter* p ) const;
+
     /** Returns frame width in painter units*/
     //double scaledFrameWidth( QPainter* p) const;
     /** Gets the frame line (0 is the top line, 1 right, 2 bottom, 3 left)*/
-    QLineF segment( int index );
+    QLineF segment( int index ) const;
     /** Returns a point on the line from startPoint to directionPoint that is a certain distance away from the starting point*/
     QPointF pointOnLineWithDistance( QPointF startPoint, QPointF directionPoint, double distance ) const;
     /** Returns the symbol size scaled in (mapcanvas) pixels. Used for the counding rect calculation*/

src/core/CMakeLists.txt

@@ -603,6 +603,7 @@ SET(QGIS_CORE_HDRS
   qgis.h
   qgsaction.h
   qgsaggregatecalculator.h
+  qgsannotation.h
   qgsattributetableconfig.h
   qgsattributeaction.h
   qgscachedfeatureiterator.h

src/core/composer/qgscomposermap.cpp

@@ -35,6 +35,7 @@
 #include "qgspallabeling.h"
 #include "qgsexpression.h"
 #include "qgsvisibilitypresetcollection.h"
+#include "qgsannotation.h"
 
 #include "qgssymbollayerv2utils.h" //for pointOnLineWithDistance
 
@@ -2365,101 +2366,70 @@ void QgsComposerMap::drawCanvasItems( QPainter* painter, const QStyleOptionGraph
   for ( int i = itemList.size() - 1; i >= 0; --i )
   {
     currentItem = itemList.at( i );
-    //don't draw mapcanvasmap (has z value -10)
-    if ( !currentItem || currentItem->data( 0 ).toString() != "AnnotationItem" )
+
+    const QgsAnnotation* annotation = dynamic_cast< const QgsAnnotation* >( currentItem );
+    if ( !annotation )
     {
       continue;
     }
-    drawCanvasItem( currentItem, painter, itemStyle );
+    drawCanvasItem( annotation, painter, itemStyle );
   }
 }
 
-void QgsComposerMap::drawCanvasItem( QGraphicsItem* item, QPainter* painter, const QStyleOptionGraphicsItem* itemStyle )
+void QgsComposerMap::drawCanvasItem( const QgsAnnotation* annotation, QPainter* painter, const QStyleOptionGraphicsItem* itemStyle )
 {
-  if ( !item || !mMapCanvas || !item->isVisible() )
+  if ( !annotation || !annotation->showItem() )
   {
     return;
   }
 
   painter->save();
   painter->setRenderHint( QPainter::Antialiasing );
 
-  //determine scale factor according to graphics view dpi
-  double scaleFactor = 1.0 / mMapCanvas->logicalDpiX() * 25.4;
+  double scaleFactor = annotation->scaleFactor();
 
   double itemX, itemY;
-  QGraphicsItem* parent = item->parentItem();
-  if ( !parent )
+  if ( annotation->mapPositionFixed() )
   {
-    // having no parent is a hack to indicate that annotation has a fixed position
-    QPointF mapPos = composerMapPosForItem( item );
+    QPointF mapPos = composerMapPosForItem( annotation );
     itemX = mapPos.x();
     itemY = mapPos.y();
   }
-  else //place item relative to the parent item
+  else
   {
-    // having a parent is a hack to indicate annotation has relative position
-    QPointF itemScenePos = item->scenePos();
-    QPointF parentScenePos = parent->scenePos();
-
-    QPointF mapPos = composerMapPosForItem( parent );
-
-    itemX = mapPos.x() + ( itemScenePos.x() - parentScenePos.x() ) * scaleFactor;
-    itemY = mapPos.y() + ( itemScenePos.y() - parentScenePos.y() ) * scaleFactor;
+    itemX = annotation->relativePosition().x() * rect().width();
+    itemY = annotation->relativePosition().y() * rect().height();
   }
-  painter->translate( itemX, itemY );
 
+  painter->translate( itemX, itemY );
   painter->scale( scaleFactor, scaleFactor );
 
   //a little trick to let the item know that the paint request comes from the composer
-  item->setData( 1, "composer" );
-  item->paint( painter, itemStyle, nullptr );
-  item->setData( 1, "" );
+  const_cast< QgsAnnotation* >( annotation )->setItemData( 1, "composer" );
+  const_cast< QgsAnnotation* >( annotation )->paint( painter, itemStyle, nullptr );
+  const_cast< QgsAnnotation* >( annotation )->setItemData( 1, "" );
+
   painter->restore();
 }
 
-QPointF QgsComposerMap::composerMapPosForItem( const QGraphicsItem* item ) const
+QPointF QgsComposerMap::composerMapPosForItem( const QgsAnnotation* annotation ) const
 {
-  if ( !item )
+  if ( !annotation )
     return QPointF( 0, 0 );
 
   double mapX = 0.0;
   double mapY = 0.0;
-  if ( item->parentItem() && !mMapCanvas )
-  {
-    // having a parentItem is a hack used to indicate annotation has relative position
-    return QPointF( 0, 0 );
-  }
-  else if ( item->parentItem() )
-  {
-    // TODO this is totally broken for rotated maps
-    if ( currentMapExtent()->height() <= 0 || currentMapExtent()->width() <= 0 || mMapCanvas->width() <= 0 || mMapCanvas->height() <= 0 )
-    {
-      return QPointF( 0, 0 );
-    }
 
-    QRectF graphicsSceneRect = mMapCanvas->sceneRect();
-    QPointF itemScenePos = item->scenePos();
-    QgsRectangle mapRendererExtent = mComposition->mapSettings().visibleExtent();
+  mapX = annotation->mapPosition().x();
+  mapY = annotation->mapPosition().y();
+  QgsCoordinateReferenceSystem crs = annotation->mapPositionCrs();
 
-    mapX = itemScenePos.x() / graphicsSceneRect.width() * mapRendererExtent.width() + mapRendererExtent.xMinimum();
-    mapY = mapRendererExtent.yMaximum() - itemScenePos.y() / graphicsSceneRect.height() * mapRendererExtent.height();
-  }
-  else
+  if ( crs != mComposition->mapSettings().destinationCrs() )
   {
-    //fixed position, use a hack where the position is encoded in item's custom data
-    //(this whole annotation handling is a hack and needs to be rewritten)
-    mapX = item->data( 2 ).toDouble();
-    mapY = item->data( 3 ).toDouble();
-    long crsid = item->data( 4 ).toLongLong();
-
-    if ( crsid != mComposition->mapSettings().destinationCrs().srsid() )
-    {
-      //need to reproject
-      QgsCoordinateTransform t( QgsCoordinateReferenceSystem::fromSrsId( crsid ), mComposition->mapSettings().destinationCrs() );
-      double z = 0.0;
-      t.transformInPlace( mapX, mapY, z );
-    }
+    //need to reproject
+    QgsCoordinateTransform t( crs, mComposition->mapSettings().destinationCrs() );
+    double z = 0.0;
+    t.transformInPlace( mapX, mapY, z );
   }
 
   return mapToItemCoords( QPointF( mapX, mapY ) );

src/core/composer/qgscomposermap.h

@@ -36,6 +36,7 @@ class QPainter;
 class QgsFillSymbolV2;
 class QgsLineSymbolV2;
 class QgsVectorLayer;
+class QgsAnnotation;
 
 /** \ingroup core
  *  \class QgsComposerMap
@@ -968,8 +969,8 @@ class CORE_EXPORT QgsComposerMap : public QgsComposerItem
     void transformShift( double& xShift, double& yShift ) const;
 
     void drawCanvasItems( QPainter* painter, const QStyleOptionGraphicsItem* itemStyle );
-    void drawCanvasItem( QGraphicsItem* item, QPainter* painter, const QStyleOptionGraphicsItem* itemStyle );
-    QPointF composerMapPosForItem( const QGraphicsItem* item ) const;
+    void drawCanvasItem( const QgsAnnotation* item, QPainter* painter, const QStyleOptionGraphicsItem* itemStyle );
+    QPointF composerMapPosForItem( const QgsAnnotation* item ) const;
 
     enum PartType
     {

src/core/qgsannotation.h

@@ -0,0 +1,84 @@
+/***************************************************************************
+                             qgsannotation.h
+                             ---------------
+    begin                : July 2016
+    copyright            : (C) 2016 by Nyall Dawson
+    email                : nyall dot dawson at gmail dot com
+ ***************************************************************************/
+
+/***************************************************************************
+ *                                                                         *
+ *   This program is free software; you can redistribute it and/or modify  *
+ *   it under the terms of the GNU General Public License as published by  *
+ *   the Free Software Foundation; either version 2 of the License, or     *
+ *   (at your option) any later version.                                   *
+ *                                                                         *
+ ***************************************************************************/
+
+#ifndef QGSANNOTATION_H
+#define QGSANNOTATION_H
+
+#include "qgspoint.h"
+#include "qgscoordinatereferencesystem.h"
+
+class QPainter;
+class QStyleOptionGraphicsItem;
+
+/** \ingroup core
+ * \class QgsAnnotation
+ * \note added in QGIS 3.0
+ *
+ *  An interface for annotation type items.
+ */
+
+class CORE_EXPORT QgsAnnotation
+{
+  public:
+
+    //! Returns true if the annotation should be shown.
+    virtual bool showItem() const = 0;
+
+    /** Returns true if the annotation is attached to a fixed map position, or
+     * false if the annotation uses a position relative to the current map
+     * extent.
+     * @see mapPosition()
+     * @see relativePositon()
+     */
+    //TODO QGIS 3 - rename to hasFixedMapPosition()
+    virtual bool mapPositionFixed() const = 0;
+
+    /** Returns the map position of the annotation, if it is attached to a fixed map
+     * position.
+     * @see mapPositionFixed()
+     * @see mapPositionCrs()
+     */
+    virtual QgsPoint mapPosition() const { return QgsPoint(); }
+
+    /** Returns the CRS of the map position, or an invalid CRS if the annotation does
+     * not have a fixed map position.
+    */
+    virtual QgsCoordinateReferenceSystem mapPositionCrs() const { return QgsCoordinateReferenceSystem(); }
+
+    /** Returns the relative position of the annotation, if it is not attached to a fixed map
+     * position. The coordinates in the return point should be between 0 and 1, and represent
+     * the relative percentage for the position compared to the map width and height.
+     * @see mapPositionFixed()
+     */
+    virtual QPointF relativePosition() const { return QPointF(); }
+
+    /** Returns a scaling factor which should be applied to painters before rendering
+     * the item.
+     */
+    virtual double scaleFactor() const = 0;
+
+    //! deprecated - do not use
+    // TODO QGIS 3.0 - remove
+    virtual void setItemData( int role, const QVariant& value ) = 0;
+
+    //! Paint the annotation to a destination painter
+    virtual void paint( QPainter * painter, const QStyleOptionGraphicsItem * option, QWidget * widget = nullptr ) = 0;
+
+
+};
+
+#endif // QGSANNOTATION_H