Skip to content

Commit

Permalink
Refactor QgsMapToPixel to only build matrix on parameter change
Browse files Browse the repository at this point in the history 
Also add deprecation notices and provide full-parameters constructor
  • Loading branch information
Sandro Santilli committed Dec 9, 2014
1 parent ec085bf commit cebb6ff
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 38 deletions.
116 changes: 92 additions & 24 deletions src/core/qgsmaptopixel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,52 @@

#include "qgslogger.h"

// @deprecated in 2.8
QgsMapToPixel::QgsMapToPixel( double mapUnitsPerPixel,
double xc,
double yc,
int width,
int height,
double rotation )
: mMapUnitsPerPixel( mapUnitsPerPixel )
, mWidth( width )
, mHeight( height )
, mRotation( rotation )
, xCenter( xc )
, yCenter( yc )
, xMin ( xc - ( mWidth * mMapUnitsPerPixel / 2.0 ) )
, yMin ( yc - ( mHeight * mMapUnitsPerPixel / 2.0 ) )
{
updateMatrix();
}

QgsMapToPixel::QgsMapToPixel()
: mMapUnitsPerPixel( 1 )
, mWidth( 1 )
, mHeight( 1 )
, mRotation( 0.0 )
, xCenter( 0.5 )
, yCenter( 0.5 )
, xMin ( 0 )
, yMin ( 0 )
{
updateMatrix();
}

QgsMapToPixel::QgsMapToPixel( double mapUnitsPerPixel,
double height,
double ymin,
double xmin )
: mMapUnitsPerPixel( mapUnitsPerPixel )
, mWidth( -1 )
, mHeight( height )
, yMin( ymin )
, xMin( xmin )
, mMapRotation( 0 )
, mRotation( 0.0 )
, xCenter( 0.0 )
, yCenter( 0.0 )
, xMin ( xmin )
, yMin ( ymin )
{
updateMatrix();
}

QgsMapToPixel::~QgsMapToPixel()
Expand All @@ -46,13 +82,11 @@ int QgsMapToPixel::mapHeight() const

int QgsMapToPixel::mapWidth() const
{
return (( xCenter -xMin )*2 ) / mMapUnitsPerPixel;
return mWidth;
}

QTransform QgsMapToPixel::getMatrix() const
void QgsMapToPixel::updateMatrix()
{
double cy = mapHeight() / 2.0;
double cx = mapWidth() / 2.0;
double rotation = mapRotation();

#if 0 // debugging
Expand All @@ -71,17 +105,25 @@ QTransform QgsMapToPixel::getMatrix() const
// Returning a simplified matrix in hope it'll give expected
// results from an existing test, see
// https://travis-ci.org/qgis/QGIS/builds/42508945
return QTransform::fromScale( 1.0 / mMapUnitsPerPixel, -1.0 / mMapUnitsPerPixel )
mMatrix = QTransform::fromScale( 1.0 / mMapUnitsPerPixel, -1.0 / mMapUnitsPerPixel )
.translate( -xMin, - ( yMin + mHeight*mMapUnitsPerPixel ) );
return;
}

return QTransform::fromTranslate( cx, cy )
double cy = mapHeight() / 2.0;
double cx = mapWidth() / 2.0;
mMatrix = QTransform::fromTranslate( cx, cy )
.rotate( rotation )
.scale( 1 / mMapUnitsPerPixel, -1 / mMapUnitsPerPixel )
.translate( -xCenter, -yCenter )
;
}

const QTransform& QgsMapToPixel::getMatrix() const

This comment has been minimized.

Sign in to view

Copy link
@m-kuhn

m-kuhn Dec 10, 2014

Member

Does it make sense to have a private method that does nothing else but returning a private variable?
Or asked differently... Is there a reason not to make this public?

This comment has been minimized.

Sign in to view

Copy link
@strk

strk via email Dec 11, 2014

Contributor

This comment has been minimized.

Sign in to view

Copy link
@jef-n

jef-n Dec 11, 2014

Member

The question probably isn't why the method is private, but why it exists at all. The member variable can already be accessed directly where the method can be called. BTW we try to avoid get prefixes (like Qt).

This comment has been minimized.

Sign in to view

Copy link
@m-kuhn

m-kuhn Dec 11, 2014

Member

It exists because there was originally a calculation done which has been refactored out and that method is the leftover.
@jef-n you sound like you prefer to remove it. Is there a reason for this? I may have a good use for this but it is also possible that it will turn out it's easier to reimplement the logic outside.

This comment has been minimized.

Sign in to view

Copy link
@jef-n

jef-n Dec 11, 2014

Member

I was wondering why there is a private function to return a private method. What's the point? And if there is none, that method shouldn't exist. I wouldn't have that question if it was protected or public.
{
return mMatrix;
}

QgsPoint QgsMapToPixel::toMapPoint( double x, double y ) const
{
bool invertible;
Expand Down Expand Up @@ -115,6 +157,7 @@ QgsPoint QgsMapToPixel::toMapCoordinatesF( double x, double y ) const
void QgsMapToPixel::setMapUnitsPerPixel( double mapUnitsPerPixel )
{
mMapUnitsPerPixel = mapUnitsPerPixel;
updateMatrix();
}

double QgsMapToPixel::mapUnitsPerPixel() const
Expand All @@ -124,50 +167,75 @@ double QgsMapToPixel::mapUnitsPerPixel() const

void QgsMapToPixel::setMapRotation( double degrees, double cx, double cy )
{
mMapRotation = degrees;
mRotation = degrees;
xCenter = cx;
yCenter = cy;
assert( xCenter >= xMin );
assert( yCenter >= yMin );
//assert(yCenter <= yMin + mHeight*mMapUnitsPerPixel;
if ( mWidth < 0 ) {
// set width not that we can compute it
mWidth = (( xCenter -xMin )*2 ) / mMapUnitsPerPixel;
}
updateMatrix();
}

double QgsMapToPixel::mapRotation() const
{
return mMapRotation;
}

void QgsMapToPixel::setViewportHeight( double height )
{
mHeight = height;
return mRotation;
}

// @deprecated in 2.8
void QgsMapToPixel::setYMinimum( double ymin )
{
yMin = ymin;
yCenter = ymin + mHeight * mMapUnitsPerPixel / 2.0;
mRotation = 0.0;
updateMatrix();
}

// @deprecated in 2.8
void QgsMapToPixel::setXMinimum( double xmin )
{
xMin = xmin;
xCenter = xmin + mWidth * mMapUnitsPerPixel / 2.0;
mRotation = 0.0;
updateMatrix();
}

// @deprecated in 2.8
void QgsMapToPixel::setParameters( double mapUnitsPerPixel, double xmin, double ymin, double ymax )
{
mMapUnitsPerPixel = mapUnitsPerPixel;
xMin = xmin;
yMin = ymin;
mHeight = ymax;
xCenter = xmin + mWidth * mMapUnitsPerPixel / 2.0;
yCenter = ymin + mHeight * mMapUnitsPerPixel / 2.0;
mRotation = 0.0;
updateMatrix();
}

void QgsMapToPixel::setParameters( double mapUnitsPerPixel,
double xc,
double yc,
int width,
int height,
double rotation )
{
mMapUnitsPerPixel = mapUnitsPerPixel;
xCenter = xc;
yCenter = yc;
mWidth = width;
mHeight = height;
mRotation = rotation;
xMin = xc - ( mWidth * mMapUnitsPerPixel / 2.0 );
yMin = yc - ( mHeight * mMapUnitsPerPixel / 2.0 );
updateMatrix();
}

QString QgsMapToPixel::showParameters()
{
QString rep;
QTextStream( &rep ) << "Map units/pixel: " << mMapUnitsPerPixel
<< " X minimum: " << xMin << " Y minimum: " << yMin
<< " Height: " << mHeight << " Rotation: " << mMapRotation
<< " X center: " << xCenter << " Y center: " << yCenter;
<< " center: " << xCenter << "," << yCenter
<< " rotation: " << mRotation
<< " size: " << mWidth << "x" << mHeight;
return rep;

}
Expand Down
66 changes: 52 additions & 14 deletions src/core/qgsmaptopixel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,27 @@ class CORE_EXPORT QgsMapToPixel
* @param height Map canvas height, in pixels
* @param ymin Minimum y value of the map canvas
* @param xmin Minimum x value of the map canvas
* @deprecated in 2.8, use version with all parameters
*/
QgsMapToPixel( double mapUnitsPerPixel = 0, double height = 0, double ymin = 0,
double xmin = 0 );
QgsMapToPixel( double mapUnitsPerPixel, double height = 0, double ymin = 0, double xmin = 0 );

/* Constructor
* @param mapUnitsPerPixel Map units per pixel
* @param xc X ordinate of map center, in geographical units
* @param yc Y ordinate of map center, in geographical units
* @param width Output width, in pixels
* @param height Output height, in pixels
* @param degrees clockwise rotation in degrees
* @note added in 2.8
*/
QgsMapToPixel( double mapUnitsPerPixel, double xc, double yc, int width, int height, double rotation );

/*! Constructor
*
* Use setParameters to fill
*/
QgsMapToPixel();

//! destructor
~QgsMapToPixel();
/*! Transform the point from map (world) coordinates to device coordinates
Expand Down Expand Up @@ -103,9 +121,12 @@ class CORE_EXPORT QgsMapToPixel
double mapUnitsPerPixel() const;

//! Return current map width in pixels
//! The information is only known if setRotation was used
//! @note added in 2.8
int mapWidth() const;

//! Return current map height in pixels
//! @note added in 2.8
int mapHeight() const;

//! Set map rotation in degrees (clockwise)
Expand All @@ -120,39 +141,56 @@ class CORE_EXPORT QgsMapToPixel
double mapRotation() const;

//! Set maximum y value
// @deprecated in 2.8, use setViewportHeight
// @deprecated in 2.8, use setParameters
// @note this really sets the viewport height, not ymax
void setYMaximum( double yMax ) { setViewportHeight( yMax ); }
//! Set viewport height
//! @note added in 2.8
void setViewportHeight( double height );
void setYMaximum( double yMax ) { mHeight = yMax; }

//! Set minimum y value
// @deprecated in 2.8, use setParameters
void setYMinimum( double ymin );

//! set minimum x value
// @deprecated in 2.8, use setParameters
void setXMinimum( double xmin );

/*! Set parameters for use in transforming coordinates
* @param mapUnitsPerPixel Map units per pixel
* @param xmin Minimum x value
* @param ymin Minimum y value
* @param height Map height, in pixels
* @deprecated in 2.8, use the version with full parameters
*/
void setParameters( double mapUnitsPerPixel, double xmin, double ymin, double height );

/*! Set parameters for use in transforming coordinates
* @param mapUnitsPerPixel Map units per pixel
* @param xc X ordinate of map center, in geographical units
* @param yc Y ordinate of map center, in geographical units
* @param width Output width, in pixels
* @param height Output height, in pixels
* @param degrees clockwise rotation in degrees
* @note added in 2.8
*/
void setParameters( double mapUnitsPerPixel, double xc, double yc, int width, int height, double rotation );

//! String representation of the parameters used in the transform
QString showParameters();

private:
double mMapUnitsPerPixel;
double mHeight;
double yMin;
double xMin;
//! Map rotation around Z axis on map center as clockwise degrees
//! @note added in 2.8
double mMapRotation;
int mWidth;
int mHeight;
double mRotation;
double xCenter;
double yCenter;
double xMin; // @deprecated in 2.8
double yMin; // @deprecated in 2.8
QTransform mMatrix;

// Matrix to map from map (geographical) to screen (pixels) units
QTransform getMatrix() const;
const QTransform& getMatrix() const;

void updateMatrix();
};


Expand Down

0 comments on commit cebb6ff

Please sign in to comment.