Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wrong direction of label direction symbol #13702

Closed
qgib opened this issue Mar 20, 2011 · 24 comments
Closed

wrong direction of label direction symbol #13702

qgib opened this issue Mar 20, 2011 · 24 comments
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Symbology Related to vector layer symbology or renderers
Milestone

Comments

@qgib
Copy link
Contributor

qgib commented Mar 20, 2011

Author Name: Sandro Santilli (@strk)
Original Redmine Issue: 3643

Redmine category:symbology
Assignee: Marco Hugentobler


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 #13494) so a similar approach could be just copied over

The bug occurs with r11539 on a x84-64


@qgib
Copy link
Contributor Author

qgib commented Mar 23, 2011

Author Name: Marco Hugentobler (@mhugent)


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

@qgib
Copy link
Contributor Author

qgib commented Mar 24, 2011

Author Name: Sandro Santilli (@strk)


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.

@qgib
Copy link
Contributor Author

qgib commented Mar 24, 2011

Author Name: Marco Hugentobler (@mhugent)


Fixed in 76d0e28 (SVN r15592)


  • status_id was changed from Open to Closed
  • resolution was configured as fixed

@qgib
Copy link
Contributor Author

qgib commented Mar 29, 2011

Author Name: Sandro Santilli (@strk)


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


  • resolution was changed from fixed to
  • status_id was changed from Closed to Feedback

@qgib
Copy link
Contributor Author

qgib commented Mar 29, 2011

Author Name: Sandro Santilli (@strk)


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.

@qgib
Copy link
Contributor Author

qgib commented Mar 29, 2011

Author Name: Sandro Santilli (@strk)


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

@qgib
Copy link
Contributor Author

qgib commented Mar 29, 2011

Author Name: Sandro Santilli (@strk)


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

@qgib
Copy link
Contributor Author

qgib commented Mar 29, 2011

Author Name: Sandro Santilli (@strk)


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

@qgib
Copy link
Contributor Author

qgib commented Mar 29, 2011

Author Name: Sandro Santilli (@strk)


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 ||.

@qgib
Copy link
Contributor Author

qgib commented Mar 29, 2011

Author Name: Marco Hugentobler (@mhugent)


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


  • resolution was changed from to fixed
  • status_id was changed from Feedback to Closed

@qgib
Copy link
Contributor Author

qgib commented Mar 29, 2011

Author Name: Sandro Santilli (@strk)


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

@qgib
Copy link
Contributor Author

qgib commented Jul 8, 2011

Author Name: Mayeul Kauffmann (@mayeulk)


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"


  • resolution was changed from fixed to
  • 3328 was configured as line-driven_label_orientation.png

@qgib
Copy link
Contributor Author

qgib commented Jul 8, 2011

Author Name: Mayeul Kauffmann (@mayeulk)


  • 3329 was configured as test_label_orientation.qgs

@qgib
Copy link
Contributor Author

qgib commented Jul 8, 2011

Author Name: Mayeul Kauffmann (@mayeulk)


  • 3330 was configured as test_label_orientation.sqlite

@qgib
Copy link
Contributor Author

qgib commented Oct 22, 2011

Author Name: Sandro Santilli (@strk)


Mayeul: I've filed #14356 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 ?!


  • pull_request_patch_supplied was configured as 0

@qgib
Copy link
Contributor Author

qgib commented Oct 22, 2011

Author Name: Sandro Santilli (@strk)


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".


  • fixed_version_id was changed from Version 1.7.0 to Version 1.7.1
  • must_fix was changed from No to Yes

@qgib
Copy link
Contributor Author

qgib commented Oct 22, 2011

Author Name: Sandro Santilli (@strk)


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

@qgib
Copy link
Contributor Author

qgib commented Oct 22, 2011

Author Name: Sandro Santilli (@strk)


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

@qgib
Copy link
Contributor Author

qgib commented Oct 22, 2011

Author Name: Sandro Santilli (@strk)


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

are available in the git repository at:

git@github.com:strk/Quantum-GIS.git dirsym

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

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

@qgib
Copy link
Contributor Author

qgib commented Oct 22, 2011

Author Name: Sandro Santilli (@strk)


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

@qgib
Copy link
Contributor Author

qgib commented Oct 22, 2011

Author Name: Sandro Santilli (@strk)


Fix pushed to master as r697b35a4.


  • resolution was changed from to fixed

@qgib
Copy link
Contributor Author

qgib commented Nov 8, 2011

Author Name: Martin Dobias (@wonder-sk)


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


  • resolution was changed from fixed to
  • status_id was changed from Closed to Open

@qgib
Copy link
Contributor Author

qgib commented Nov 8, 2011

Author Name: Sandro Santilli (@strk)


I'm not seeing that from the logs ?
Unless the commit log is broken:
https://issues.qgis.org/projects/quantum-gis/repository/revisions/697b35a4185df804baa0f648389a05ed8a7d59fa

@qgib
Copy link
Contributor Author

qgib commented Nov 8, 2011

Author Name: Martin Dobias (@wonder-sk)


Fixed in c37b63a, better variable naming and added some comments


  • resolution was changed from to fixed
  • status_id was changed from Open to Closed

@qgib qgib added Bug Either a bug report, or a bug fix. Let's hope for the latter! Symbology Related to vector layer symbology or renderers labels May 24, 2019
@qgib qgib added this to the Version 1.7.1 milestone May 24, 2019
@qgib qgib closed this as completed May 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Symbology Related to vector layer symbology or renderers
Projects
None yet
Development

No branches or pull requests

1 participant