Bug report #3643

wrong direction of label direction symbol

Added by Sandro Santilli over 9 years ago. Updated almost 9 years ago.

Status:Closed
Priority:Low
Assignee:Marco Hugentobler
Category:Symbology
Affected QGIS version: Regression?:No
Operating System:Debian Easy fix?:No
Pull Request or Patch supplied:No Resolution:fixed
Crashes QGIS or corrupts data: Copied to github as #:13702

Description

As you can see in the attached image, the "direction symbol" added by the label disagrees with the "direction symbol" of the line decoration.

I know the line decoration (the endpoint arrow) is correct.
Note that line decoration was fixed by r15076 (see #3434) so a similar approach could be just copied over

The bug occurs with r11539 on a x84-64

wrong_orientation_label.png (1.11 KB) Sandro Santilli, 2011-03-20 02:54 AM

edge.zip (1.67 KB) Sandro Santilli, 2011-03-24 01:11 AM

qgisbug3643_1.zip (1.04 KB) Sandro Santilli, 2011-03-29 05:23 AM

bug3643.diff Magnifier (793 Bytes) Sandro Santilli, 2011-03-29 05:53 AM

line-driven_label_orientation.png - fix orientation/put number uphill / QGIS & IGN (1.2 MB) Mayeul Kauffmann, 2011-07-08 05:53 AM

test_label_orientation.qgs (9.77 KB) Mayeul Kauffmann, 2011-07-08 05:54 AM

test_label_orientation.sqlite - data to use with test_label_orientation.qgs (3.52 MB) Mayeul Kauffmann, 2011-07-08 05:56 AM

History

#1 Updated by Marco Hugentobler over 9 years ago

Is this only for vertical lines or for others too? Could you attach a small testdataset to reproduce the problem?

#2 Updated by Sandro Santilli over 9 years ago

Seems to be vertical lines only.
I attach a shapefile. Turn on arrows decoration on symbology-ng and orientation symbol on labels. Arrow decorations are correct.

#3 Updated by Marco Hugentobler over 9 years ago

  • Status changed from Open to Closed
  • Resolution set to fixed

Fixed in 76d0e28d (SVN r15592)

#4 Updated by Sandro Santilli over 9 years ago

  • Resolution deleted (fixed)
  • Status changed from Closed to Feedback

Indeed 76d0e28d (SVN r15592) fixed the attached edge.zip case, but it's still wrong for another dataset :/
I'm attaching it..

#5 Updated by Sandro Santilli over 9 years ago

In particular, edges with id 5 and 7 of qgisbug3543_1.zip are disagreed upon between label and line decoration. They are both going down, so line decoration is correct and label orientation symbol is wrong.

#6 Updated by Sandro Santilli over 9 years ago

I'd add that the original testcase only contained lines oriented from bottom up, which may explain why this case wasn't cought.

#7 Updated by Sandro Santilli over 9 years ago

The attached patch (bug3643.diff) fixes it for me, for both datasets.
Might not be extremely elegant, but should be also more robust..

#8 Updated by Sandro Santilli over 9 years ago

A slightly more elegant version:

diff --git a/src/core/pal/feature.cpp b/src/core/pal/feature.cpp
index 6f657b1..5501434 100644
--- a/src/core/pal/feature.cpp
+++ b/src/core/pal/feature.cpp
@@ -596,7 +596,7 @@ namespace pal
       {
         //std::cout << alpha*180/M_PI << std::endl;
         if ( flags & FLAG_MAP_ORIENTATION )
-          reversed = ( alpha > M_PI / 2 || alpha < -M_PI / 2 );
+          reversed = ( (ex == bx && ey < by ) || ( alpha > M_PI / 2 || alpha < -M_PI / 2 ) );

         if (( !reversed && ( flags & FLAG_ABOVE_LINE ) ) || ( reversed && ( flags & FLAG_BELOW_LINE ) ) )
           positions->push_back( new [[LabelPosition]]( i, bx + cos( beta ) *distlabel , by + sin( beta ) *distlabel, xrm, yrm, alpha, cost, this, reversed ) ); // Line

#9 Updated by Sandro Santilli over 9 years ago

Even smaller is just moving re-adding the equal sign but to the right part:

-          reversed = ( alpha > M_PI / 2 || alpha < -M_PI / 2 );
+          reversed = ( alpha > M_PI / 2 || alpha <= -M_PI / 2 );

Works fine in my case, altought the equality comparison between the return from atan2 and M_PI/2 looks scary (miracles!)

This final version seems to be the closest to the original code, where the equal sign was in the first rather than second side of ||.

#10 Updated by Marco Hugentobler over 9 years ago

  • Resolution set to fixed
  • Status changed from Feedback to Closed

Applied the second version in 9a167ba0 (SVN r15638). Thanks!

#11 Updated by Sandro Santilli over 9 years ago

Great. I confirm the fix. You can close the grave now :)

#12 Updated by Mayeul Kauffmann about 9 years ago

Hi,
The bug is fixed only when the radio button "Orientation" is set to "map", not "line".
There are two ways to fix this:
1. Rotate 180° the label and the arrow (write upside down)
2. Reverse the arrow ">" only

I would strongly suggest solution 1. This would allow to follow a frequent cartographic convention for contour lines: put number written uphill (see attached screenshot): this is used on all official French topographic map by the IGN. This seems logical even just by reading the QGIS dialog box: "Orientation"

#15 Updated by Sandro Santilli almost 9 years ago

  • Pull Request or Patch supplied set to No

Mayeul: I've filed #4424 for your request. This bug was specifically about the "Add direction symbol".

By the way, now that I try enabling that it seems to be not working anymore ?!

#16 Updated by Sandro Santilli almost 9 years ago

  • Target version changed from Version 1.7.0 to Version 1.7.1
  • Must fix changed from No to Yes

Gah. I confirm this is still broken.

First of all direction symbol is only shown with with "parallel" placement (nothing shown with "curved" or "horizontal").

Second, the bug is still there when orientation is "line" rather than "map".

#17 Updated by Sandro Santilli almost 9 years ago

How do we reopen this bug ? I personally can't change the "Status" field.

#18 Updated by Sandro Santilli almost 9 years ago

I don't really get the meaning of "Map" vs. "Line" orientation, btw

#19 Updated by Sandro Santilli almost 9 years ago

The following changes since commit 59be561a4d9100fb2084006c2acc7df5798c52fc:
Juergen E. Fischer (1):
fix warnings and more cosmetics

are available in the git repository at:

:strk/Quantum-GIS.git dirsym

Sandro Santilli (1):
Label direction symbol shouldn't depend on "map" vs. "line" orientation. Refer to issue #3643 for further discussion

src/core/pal/feature.cpp |    5 +---
1 files changed, 2 insertions(
), 3 deletions(-)

#20 Updated by Sandro Santilli almost 9 years ago

My "dirsym" branch implements solution 2 of Mayeul (as per this ticket subject).
For solution 1 see bug #4424

#21 Updated by Sandro Santilli almost 9 years ago

  • Resolution set to fixed

Fix pushed to master as r697b35a4.

#22 Updated by Martin Dobias almost 9 years ago

  • Resolution deleted (fixed)
  • Status changed from Closed to Open

Reopening: the fix in 697b35a disables the option to set the orientation of the label to be dependent on line direction.

#24 Updated by Martin Dobias almost 9 years ago

  • Resolution set to fixed
  • Status changed from Open to Closed

Fixed in c37b63a, better variable naming and added some comments

Also available in: Atom PDF