PAL line label placement does not consider map rotation

[qgib]

Author Name: Sandro Santilli (@strk)
Original Redmine Issue: 11814
Affected QGIS version: master
Redmine category:labelling
Assignee: Sandro Santilli

---

Labels placement for lines does not properly consider map rotation.
With "horizontal" placement all labels are horizontal, which is nice, but placement is somewhat offsetted from the expected one.
With "parallel" placement the labels are parallel to the pre-rotated lines, so stop being parallel once lines are rotated.

I'm happy to look at it but would need help by Larry

[qgib]

Author Name: Sandro Santilli (@strk)

---

• tag was changed from to rotation

[qgib]

Author Name: Sandro Santilli (@strk)

---

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?)

[qgib]

Author Name: Sandro Santilli (@strk)

---

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

[qgib]

Author Name: Sandro Santilli (@strk)

---

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

[qgib]

Author Name: Sandro Santilli (@strk)

---

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?)

[qgib]

Author Name: Sandro Santilli (@strk)

---

@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

[qgib]

Author Name: Sandro Santilli (@strk)

---

Some support for map rotation in labels rendering is been pushed here:
#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.

[qgib]

Author Name: Larry Shaffer (Larry Shaffer)

---

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.

[qgib]

Author Name: Sandro Santilli (@strk)

---

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 ?

[qgib]

Author Name: Sandro Santilli (@strk)

---

Label placement (for horizontal and parallel) is supported as of commit 917cee0
I've filed #20071 for the refactoring task

---

• assigned_to_id was changed from Larry Shaffer to Sandro Santilli
• resolution was changed from to fixed/implemented
• status_id was changed from Open to Closed

[qgib]

Author Name: Sandro Santilli (@strk)

---

Reopening to try the pre-rotation approach.

---

• status_id was changed from Closed to Reopened
• resolution was changed from fixed/implemented to

[qgib]

Author Name: Sandro Santilli (@strk)

---

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

[qgib]

Author Name: Sandro Santilli (@strk)

---

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

[qgib]

Author Name: Sandro Santilli (@strk)

---

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: #1750

[qgib]

Author Name: Sandro Santilli (@strk)

---

Experimental pre-rotation code is here: #1757
Note how working on map (rather than device) coordinate space makes things complex (ref #20071)
Comments welcome

[qgib]

Author Name: Sandro Santilli (@strk)

---

Pre-rotated approach pushed to master with commit 9c6fe06 -- please file new tickets if other issues arise

---

• done_ratio was changed from 0 to 100
• status_id was changed from Reopened to Closed
• resolution was changed from to fixed/implemented