Skip to content
Permalink
Browse files
Add support for map rotation (hub #9330) 
Includes widget to show and set map rotation.
Handle rotation in vector and raster renderers.
Ensure correct behavior of panning and zooming actions.

Drop compile-time defines for ARM and ANDROID, leaving only
the qreal based function to transform in place.

Update expected test results after eye comparison.
  • Loading branch information
Sandro Santilli committed Dec 7, 2014
1 parent b774853 commit ce8a9ba
Show file tree
Hide file tree
Showing 28 changed files with 486 additions and 125 deletions.
55 src/app/qgisapp.cpp
View file
@@ -24,6 +24,7 @@
#include <QApplication>
#include <QBitmap>
#include <QCheckBox>
#include <QSpinBox>
#include <QClipboard>
#include <QColor>
#include <QCursor>
@@ -1732,6 +1733,37 @@ void QgisApp::createStatusBar()
statusBar()->addPermanentWidget( mScaleEdit, 0 );
connect( mScaleEdit, SIGNAL( scaleChanged() ), this, SLOT( userScale() ) );

// add a widget to show/set current rotation
mRotationLabel = new QLabel( QString(), statusBar() );
mRotationLabel->setObjectName( "mRotationLabel" );
mRotationLabel->setFont( myFont );
mRotationLabel->setMinimumWidth( 10 );
mRotationLabel->setMaximumHeight( 20 );
mRotationLabel->setMargin( 3 );
mRotationLabel->setAlignment( Qt::AlignCenter );
mRotationLabel->setFrameStyle( QFrame::NoFrame );
mRotationLabel->setText( tr( "Rotation:" ) );
mRotationLabel->setToolTip( tr( "Current clockwise map rotation in degrees" ) );
statusBar()->addPermanentWidget( mRotationLabel, 0 );

mRotationEdit = new QSpinBox( statusBar() );
mRotationEdit->setObjectName( "mRotationEdit" );
mRotationEdit->setMaximumWidth( 100 );
mRotationEdit->setMaximumHeight( 20 );
mRotationEdit->setRange(-180, 180);
mRotationEdit->setWrapping(true);
mRotationEdit->setSingleStep(5.0);
mRotationEdit->setFont( myFont );
mRotationEdit->setWhatsThis( tr( "Shows the current map clockwise rotation "
"in degrees. It also allows editing to set "
"the rotation") );
mRotationEdit->setToolTip( tr( "Current clockwise map rotation in degrees" ) );
statusBar()->addPermanentWidget( mRotationEdit, 0 );
connect( mRotationEdit, SIGNAL( valueChanged(int) ), this, SLOT( userRotation() ) );

showRotation();


// render suppression status bar widget
mRenderSuppressionCBox = new QCheckBox( tr( "Render" ), statusBar() );
mRenderSuppressionCBox->setObjectName( "mRenderSuppressionCBox" );
@@ -1975,6 +2007,8 @@ void QgisApp::setupConnections()
this, SLOT( showExtents() ) );
connect( mMapCanvas, SIGNAL( scaleChanged( double ) ),
this, SLOT( showScale( double ) ) );
connect( mMapCanvas, SIGNAL( rotationChanged( double ) ),
this, SLOT( showRotation() ) );
connect( mMapCanvas, SIGNAL( scaleChanged( double ) ),
this, SLOT( updateMouseCoordinatePrecision() ) );
connect( mMapCanvas, SIGNAL( mapToolSet( QgsMapTool *, QgsMapTool * ) ),
@@ -6857,14 +6891,14 @@ void QgisApp::userCenter()
if ( !yOk )
return;

QgsRectangle r = mMapCanvas->extent();
mMapCanvas->setCenter( QgsPoint( x, y ) );
mMapCanvas->refresh();
}

mMapCanvas->setExtent(
QgsRectangle(
x - r.width() / 2.0, y - r.height() / 2.0,
x + r.width() / 2.0, y + r.height() / 2.0
)
);
void QgisApp::userRotation()
{
double degrees = mRotationEdit->value();
mMapCanvas->setRotation(degrees);
mMapCanvas->refresh();
}

@@ -8752,6 +8786,13 @@ void QgisApp::showExtents()
}
} // QgisApp::showExtents

void QgisApp::showRotation()
{
// update the statusbar with the current rotation.
double myrotation = mMapCanvas->rotation();
mRotationEdit->setValue( myrotation );
} // QgisApp::showRotation


void QgisApp::updateMouseCoordinatePrecision()
{
11 src/app/qgisapp.h
View file
@@ -30,6 +30,7 @@ class QProgressBar;
class QPushButton;
class QRect;
class QSettings;
class QSpinBox;
class QSplashScreen;
class QStringList;
class QToolButton;
@@ -687,6 +688,9 @@ class APP_EXPORT QgisApp : public QMainWindow, private Ui::MainWindow
void userScale();
//! Slot to handle user center input;
void userCenter();
//! Slot to handle user rotation input;
//! @note added in 2.8
void userRotation();
//! Remove a layer from the map and legend
void removeLayer();
/** Duplicate map layer(s) in legend */
@@ -1021,6 +1025,7 @@ class APP_EXPORT QgisApp : public QMainWindow, private Ui::MainWindow
void showProgress( int theProgress, int theTotalSteps );
void extentsViewToggled( bool theFlag );
void showExtents();
void showRotation();
void showStatusMessage( QString theMessage );
void displayMapToolMessage( QString message, QgsMessageBar::MessageLevel level = QgsMessageBar::INFO );
void removeMapToolMessage();
@@ -1430,6 +1435,12 @@ class APP_EXPORT QgisApp : public QMainWindow, private Ui::MainWindow
QLineEdit * mCoordsEdit;
//! The validator for the mCoordsEdit
QValidator * mCoordsEditValidator;
//! Widget that will live on the statusbar to display "Rotation"
QLabel * mRotationLabel;
//! Widget that will live in the statusbar to display and edit rotation
QSpinBox * mRotationEdit;
//! The validator for the mCoordsEdit
QValidator * mRotationEditValidator;
//! Widget that will live in the statusbar to show progress of operations
QProgressBar * mProgressBar;
//! Widget used to suppress rendering
13 src/core/qgsmaprenderer.cpp
View file
@@ -42,6 +42,7 @@
QgsMapRenderer::QgsMapRenderer()
{
mScale = 1.0;
mRotation = 0.0;
mScaleCalculator = new QgsScaleCalculator;
mDistArea = new QgsDistanceArea;

@@ -121,6 +122,18 @@ bool QgsMapRenderer::setExtent( const QgsRectangle& extent )
return true;
}

void QgsMapRenderer::setRotation( double rotation )
{
mRotation = rotation;
// TODO: adjust something ?

emit rotationChanged( rotation );
}

double QgsMapRenderer::rotation( ) const
{
return mRotation;
}


void QgsMapRenderer::setOutputSize( QSize size, int dpi )
16 src/core/qgsmaprenderer.h
View file
@@ -184,6 +184,15 @@ class CORE_EXPORT QgsMapRenderer : public QObject
//! returns current extent
QgsRectangle extent() const;

//! sets rotation
//! value in clockwise degrees
//! @note added in 2.8
void setRotation( double degrees );

//! returns current rotation in clockwise degrees
//! @note added in 2.8
double rotation() const;

const QgsMapToPixel* coordinateTransform() { return &( mRenderContext.mapToPixel() ); }

//! Scale denominator
@@ -353,6 +362,10 @@ class CORE_EXPORT QgsMapRenderer : public QObject
//! @note added in 2.4
void extentsChanged();

//! emitted when the current rotation gets changed
//! @note added in 2.8
void rotationChanged( double );

//! Notifies higher level components to show the datum transform dialog and add a QgsLayerCoordinateTransformInfo for that layer
void datumTransformInfoRequested( const QgsMapLayer* ml, const QString& srcAuthId, const QString& destAuthId ) const;

@@ -376,6 +389,9 @@ class CORE_EXPORT QgsMapRenderer : public QObject
//! Map scale denominator at its current zoom level
double mScale;

//! Map rotation
double mRotation;

//! scale calculator
QgsScaleCalculator * mScaleCalculator;

49 src/core/qgsmapsettings.cpp
View file
@@ -34,6 +34,7 @@ QgsMapSettings::QgsMapSettings()
: mDpi( qt_defaultDpiX() ) // DPI that will be used by default for QImage instances
, mSize( QSize( 0, 0 ) )
, mExtent()
, mRotation( 0.0 )
, mProjectionsEnabled( false )
, mDestCRS( GEOCRS_ID, QgsCoordinateReferenceSystem::InternalCrsId ) // WGS 84
, mDatumTransformStore( mDestCRS )
@@ -61,6 +62,21 @@ void QgsMapSettings::setExtent( const QgsRectangle& extent )
updateDerived();
}

double QgsMapSettings::rotation() const
{
return mRotation;
}

void QgsMapSettings::setRotation( double degrees )
{
if ( mRotation == degrees ) return;

mRotation = degrees;

// TODO: update extent while keeping scale ?
updateDerived();
}


void QgsMapSettings::updateDerived()
{
@@ -142,14 +158,30 @@ void QgsMapSettings::updateDerived()
mScale = mScaleCalculator.calculate( mVisibleExtent, mSize.width() );

mMapToPixel = QgsMapToPixel( mapUnitsPerPixel(), outputSize().height(), visibleExtent().yMinimum(), visibleExtent().xMinimum() );
mMapToPixel.setMapRotation( mRotation, visibleExtent().center().x(), visibleExtent().center().y() );

#if 1 // set visible extent taking rotation in consideration
if ( mRotation ) {
QgsPoint p1 = mMapToPixel.toMapCoordinates( QPoint(0,0) );
QgsPoint p2 = mMapToPixel.toMapCoordinates( QPoint(0,myHeight) );
QgsPoint p3 = mMapToPixel.toMapCoordinates( QPoint(myWidth,0) );
QgsPoint p4 = mMapToPixel.toMapCoordinates( QPoint(myWidth,myHeight) );

This comment has been minimized.

Sign in to view

Copy link
@wonder-sk

wonder-sk Dec 8, 2014

Member

These should probably read myWidth-1 and myHeight-1 ?
dxmin = std::min(p1.x(), std::min(p2.x(), std::min(p3.x(), p4.x())));
dymin = std::min(p1.y(), std::min(p2.y(), std::min(p3.y(), p4.y())));
dxmax = std::max(p1.x(), std::max(p2.x(), std::max(p3.x(), p4.x())));
dymax = std::max(p1.y(), std::max(p2.y(), std::max(p3.y(), p4.y())));
mVisibleExtent.set( dxmin, dymin, dxmax, dymax );
}
#endif

QgsDebugMsg( QString( "Map units per pixel (x,y) : %1, %2" ).arg( qgsDoubleToString( mapUnitsPerPixelX ) ).arg( qgsDoubleToString( mapUnitsPerPixelY ) ) );
QgsDebugMsg( QString( "Pixmap dimensions (x,y) : %1, %2" ).arg( qgsDoubleToString( myWidth ) ).arg( qgsDoubleToString( myHeight ) ) );
QgsDebugMsg( QString( "Pixmap dimensions (x,y) : %1, %2" ).arg( qgsDoubleToString( mSize.width() ) ).arg( qgsDoubleToString( mSize.height() ) ) );
QgsDebugMsg( QString( "Extent dimensions (x,y) : %1, %2" ).arg( qgsDoubleToString( mExtent.width() ) ).arg( qgsDoubleToString( mExtent.height() ) ) );
QgsDebugMsg( mExtent.toString() );
QgsDebugMsg( QString( "Adjusted map units per pixel (x,y) : %1, %2" ).arg( qgsDoubleToString( mVisibleExtent.width() / myWidth ) ).arg( qgsDoubleToString( mVisibleExtent.height() / myHeight ) ) );
QgsDebugMsg( QString( "Recalced pixmap dimensions (x,y) : %1, %2" ).arg( qgsDoubleToString( mVisibleExtent.width() / mMapUnitsPerPixel ) ).arg( qgsDoubleToString( mVisibleExtent.height() / mMapUnitsPerPixel ) ) );
QgsDebugMsg( QString( "Scale (assuming meters as map units) = 1:%1" ).arg( qgsDoubleToString( mScale ) ) );
QgsDebugMsg( QString( "Rotation: %1 degrees" ).arg( mRotation ) );

mValid = true;
}
@@ -512,6 +544,14 @@ void QgsMapSettings::readXML( QDomNode& theNode )
QgsRectangle aoi = QgsXmlUtils::readRectangle( extentNode.toElement() );
setExtent( aoi );

// set rotation
QDomNode rotationNode = theNode.namedItem( "rotation" );
QString rotationVal = rotationNode.toElement().text();
if ( ! rotationVal.isEmpty() ) {
double rot = rotationVal.toDouble();
setRotation( rot );
}

mDatumTransformStore.readXML( theNode );
}

@@ -525,6 +565,13 @@ void QgsMapSettings::writeXML( QDomNode& theNode, QDomDocument& theDoc )
// Write current view extents
theNode.appendChild( QgsXmlUtils::writeRectangle( extent(), theDoc ) );

// Write current view rotation
QDomElement rotNode = theDoc.createElement( "rotation" );
rotNode.appendChild(
theDoc.createTextNode( qgsDoubleToString( rotation() ) )
);
theNode.appendChild(rotNode);

// projections enabled
QDomElement projNode = theDoc.createElement( "projections" );
projNode.appendChild( theDoc.createTextNode( QString::number( hasCrsTransformEnabled() ) ) );
12 src/core/qgsmapsettings.h
View file
@@ -70,6 +70,16 @@ class CORE_EXPORT QgsMapSettings
//! Set the size of the resulting map image
void setOutputSize( const QSize& size );

//! Return the rotation of the resulting map image
//! Units are clockwise degrees
//! @note added in 2.8
double rotation() const;
//! Set the rotation of the resulting map image
//! Units are clockwise degrees
//! TODO: define relation between extent and rotation
//! @note added in 2.8
void setRotation( double degrees );

//! Return DPI used for conversion between real world units (e.g. mm) and pixels
//! Default value is 96
int outputDpi() const;
@@ -217,6 +227,8 @@ class CORE_EXPORT QgsMapSettings

QgsRectangle mExtent;

double mRotation;

QStringList mLayers;

bool mProjectionsEnabled;

11 comments on commit ce8a9ba

@nyalldawson
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@strk Thanks for tackling this important change - it's much appreciated, especially if I can swap over the composer map rotation to use this better implementation instead.

There's a couple of issues with this commit which need to be addressed:

  1. Firstly, it's missing updates to the sip bindings for the header file changes.
  2. Given that rotation is stored as a double, I think all the checks for "!rotation" should be replaced by "!qgsDoubleNear( rotation, 0.0 )" -- that should be a bit safer.
  3. A change to core components like this really needs to be accompanied by new unit tests. Is this on your todo list? I'd suggest at a minimum numeric tests for the QgsMapToPixel changes, and render tests for both vectors and rasters.

@nyalldawson
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@strk some more issues I've run into when testing this:

  • Identify tool doesn't correctly shade identified points. It works if I first identify a feature, and then change the rotation, but not if I identify a feature on an already rotated map
  • Selection tools are broken - try using the rectangular selection tool on a rotated map. They should be ignoring the rotation.

@nirvn
Copy link
Contributor

@nirvn nirvn commented on ce8a9ba Dec 8, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@strk nice, nice.

Few comments (using a rotation value of 45, in case that is specific to what I see):

  • One thing needs fixing as a priority: the rotation spin box in the status bar need to allow for values containing decimal (i.e. 45.5 degrees)
  • In addition to the rotated selection tool issue raised by Nyall, I noticed the single click feature selection isn't working (i.e. select the rectangular selection tool, mouse over a feature, click on it, notice it doesn't get selected)

It'd be nice for you to put up a TODO list of what hasn't been implemented yet (north arrow, grid, etc.) as a comment to this commit.

Also, maybe a pro like @wonder-sk could give his seal of approval on this significant code change :)

Cheers and thanks again.

@wonder-sk
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to me. In addition to @nyalldawson 's comments, I would just add:

  • it is probably worth constructing the transform matrix in QgsMapToPixel once and storing it in the instance. Common pattern is to create MTP object once and do transforms many times

Regarding your questions map center vs extent... I would be in favor of using map center + resolution (map units per pixel) as a canonical definition of the view. As you can see, the current solution that recognizes "requested" extent and "visible" extent is becoming even more clumsy now with rotation - as the "visible" extent does not mean anymore that everything inside it is actually visible.

@wonder-sk
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more UX related thing - could we have the map rotation configuration in a less prominent place than the status bar? (e.g. project properties). IMHO the status bar is already quite busy and the rotation is not really something that users commonly need access to all the time.

@nirvn
Copy link
Contributor

@nirvn nirvn commented on ce8a9ba Dec 8, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@strk , I spotted a regression; zoom to mouse cursor is broken (even on 0 degree rotation value).

@gioman
Copy link
Contributor

@gioman gioman commented on ce8a9ba Dec 8, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I open tickets for this issues, or you will?

@nirvn
Copy link
Contributor

@nirvn nirvn commented on ce8a9ba Dec 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@strk , @gioman , I've filed a blocker on this: http://hub.qgis.org/issues/11811

@strk
Copy link
Contributor

@strk strk commented on ce8a9ba Dec 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nyalldawson the check for 0.0 doesn't need tolerance as it really only meant as a trick to avoid subtle differences in the existing rendering tests. There's no problem to always use the full matrix otherwise. A unit test for QgsMapToPixel is on my TODO list as well as a refactoring to construct the matrix once and use multiple times. I'd actually like to completely deprecate the methods to set individual components of it, what do you think ?

For issues (identify/selection tool) please file tickets on hub.

@Nirv thanks for the ticket.
@wonder-sk could you please use hub also for UX issues ? I was actually thinking to give QDial a try too, but completely dropping from status would seem unfair to me. Wouldn't it be nice to get the defining triplet over there (center,resolution,rotation) ? Anyway, better move this to list or specific tickets.

@strk
Copy link
Contributor

@strk strk commented on ce8a9ba Dec 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, sorry @Nirv, again this was for @nirvn

@strk
Copy link
Contributor

@strk strk commented on ce8a9ba Dec 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record I've pushed single-constructed transform matrix with cebb6ff and unit test with 87de9f5 and precedessor

Please sign in to comment.