QGIS » QGIS Application

Am I reading the code correctly that the PAL library does all its work in geographical units rather than device units ?
That is, it performs no coordinate transformation to device until it's time to rendering them (ie: not during problem resolution?)

Debugging prints confirm, pal::LabelPosition getX(), getY(), getWidth() and getHeight() all return map (geographical) units, not device (pixels).

And, when rotation is active, the height and the width are wrong.

QgsPalLayerSettings::calculateLabelSize is the earlier point of failure I've found for today.
It does indeed looks like the function assumes QgsMapToPixel.toMapCoordinates( width, height ) would give correct width and height in presence of rotation, which is obviously not the case.

Maybe we could add another function to QgsMapToPixel to transform sizes... (rotation would not affect them, nor translation but only scale, isn't that correct?)

@dakarto: am I right that candidate label boxes are NOT stored as rotated, when being parallel to lines ?
as per:

// save the rect                                                              
  rect.moveTo( outPt.x(), outPt.y() );                                          
  mCandidates.append( QgsLabelCandidate( rect, lp->getCost() * 1000 ) );

in src/core/qgspallabeling.cpp, where label alpha (angle) is not taken in consideration

Some support for map rotation in labels rendering is been pushed here:
https://github.com/qgis/QGIS/pull/1729

Larry your review is welcome. I had the impression a refactoring of the labeling system would be useful (like to work in pixel space rather than in geographical units space, for the sake of collision prevention, and to pass the whole map QTransform to correctly convert from one space to the other) -- the commits in the PR do not attempt to do any refactoring.

Hi Sandro,

Correct me if I am wrong (have not had time to review your map rotation committed changes), but unless the features are pre-rotated and (optionally) clipped prior to going into the labeling engine (right before PAL registration), it will be a major refactoring to support map rotation. Are the features not being pre-rotated? Note: the engine works with temporary duplicates of the geometries.

I think refactoring the engine output (and calculations) to be relative to pixel space might not gain anything here, plus PAL library works with map units, and the map scale is passed in. So, it would also require a refactoring of the PAL library as well. I think working only in map units is the correct approach to the labeling engine, then convert to output device units/resolution during output rendering.

However, I have not before considered doing everything at output resolution pixel dimensions as a means of simplifying the engine. You may want to ask Martin Dobias what he thinks about such an approach.

The vectors are not pre-rotated by the provider, but the MapSetting's matrix (QgsMapToPixel) would be able to both rotate and scale to the output coordinate space.
I noticed "scale" is accepted by the label engine. In order to rotation to be taken in consideration it should also take rotation and rotation center (output size).
All in all having PAL directly work on the output coordinate space should simplify things as it would not even need to be passed a scale.

Why do you think working in map units is the correct approach to the labeling engine ? After all labels are configured in output device terms (size, collision).
@wonder-sk what do you think about pre-scaling as well, rather than passing scale in ?

Where should I look at to pre-rotate them for the PAL registration ?

It looks like there's no function to rotate a QgsGeometry right now, so this won't be as simple as I was hoping for.
Or am I missing a code path to rotation ? The renderer must be doing it, right ?
http://qgis.org/api/classQgsGeometry.html

GEOS does not have a rotation method either. The enhancement was requested over 10 months ago:
http://trac.osgeo.org/geos/ticket/687

Another problem with the pre-rotation approach is that the current PAL code is full of places where the current QgsMapToPixel is used to transform map points to device points (for label placement). All such pieces of code would need to use a modified QgsMapToPixel so that the rotation portion of it is removed. To make things worst some code snippets use the QgsMapToPixel registered in the QgsMapSettings (mMapSettings) and others in the QgsRenderingContext (context).

I'm not sure anymore if pre-rotating is any better than building on the path already taking of further considering the rotation component of the already-used QgsMapToPixel... One contribution in that direction arrived recently: https://github.com/qgis/QGIS/pull/1750

Experimental pre-rotation code is here: https://github.com/qgis/QGIS/pull/1757
Note how working on map (rather than device) coordinate space makes things complex (ref #11856)
Comments welcome