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

Add directional arrows on polylines #792

Merged
merged 7 commits into from Dec 5, 2017

Conversation

Projects
None yet
4 participants
@jmarks213

jmarks213 commented Nov 28, 2017

for #791

@monsieurtanuki

This comment has been minimized.

Show comment
Hide comment
@monsieurtanuki

monsieurtanuki Nov 29, 2017

Collaborator

@jmarks213 I like this idea of improving the look of the Polylines, and that makes perfect sense to add arrows.

My comments:

  • Would it be possible to add a Bitmap instead of other Paths?
  • Would it be possible to do the same thing through a carefully set up Paint?
  • It should be implemented for Polygons too
  • I don't think LinearRing is the perfect place to add the "arrow logo" code: I would keep LinearRing as is (the code was already big enough) and extend it into something like ArrowLinearRing, dedicated to the arrow logo additional feature. That would then work for Polygons too. From what I read in your code, I would say that something like overriding SegmentClipper.SegmentClippable.lineTo would do the trick: each time I add a point with lineTo, I add the corresponding arrow logo (if the distance with the previous point is big enough). And that new class should be easy to implement, as you just have to move your code from LinearRing. Or even, LinearRing should have a settable array of SegmentClippable members, typically a standard one drawing the mere polyline (to be created, named SegmentStandard?), and another one displaying the arrow logos (to be created, named SegmentArrow?).

What do you think of it?

Collaborator

monsieurtanuki commented Nov 29, 2017

@jmarks213 I like this idea of improving the look of the Polylines, and that makes perfect sense to add arrows.

My comments:

  • Would it be possible to add a Bitmap instead of other Paths?
  • Would it be possible to do the same thing through a carefully set up Paint?
  • It should be implemented for Polygons too
  • I don't think LinearRing is the perfect place to add the "arrow logo" code: I would keep LinearRing as is (the code was already big enough) and extend it into something like ArrowLinearRing, dedicated to the arrow logo additional feature. That would then work for Polygons too. From what I read in your code, I would say that something like overriding SegmentClipper.SegmentClippable.lineTo would do the trick: each time I add a point with lineTo, I add the corresponding arrow logo (if the distance with the previous point is big enough). And that new class should be easy to implement, as you just have to move your code from LinearRing. Or even, LinearRing should have a settable array of SegmentClippable members, typically a standard one drawing the mere polyline (to be created, named SegmentStandard?), and another one displaying the arrow logos (to be created, named SegmentArrow?).

What do you think of it?

@spyhunter99

This comment has been minimized.

Show comment
Hide comment
@spyhunter99

spyhunter99 Nov 29, 2017

Collaborator

Ran into a few issues drawing very large polylines. All of the lines have the arrows set to true.
image

Collaborator

spyhunter99 commented Nov 29, 2017

Ran into a few issues drawing very large polylines. All of the lines have the arrows set to true.
image

spyhunter99 added some commits Nov 29, 2017

Merge branch 'master' into feature/#791
# Conflicts:
#	OpenStreetMapViewer/src/main/java/org/osmdroid/samplefragments/SampleFactory.java
@spyhunter99

This comment has been minimized.

Show comment
Hide comment
@spyhunter99

spyhunter99 Nov 29, 2017

Collaborator

just pushed the sample along with fixing the merge conflicts

Collaborator

spyhunter99 commented Nov 29, 2017

just pushed the sample along with fixing the merge conflicts

@spyhunter99

This comment has been minimized.

Show comment
Hide comment
@spyhunter99

spyhunter99 Nov 29, 2017

Collaborator

I also noticed when drawing, some of the lines would immediately have the arrows and some would not. Not entirely sure why but it may be related to multiple "worlds" being visible concurrently

Collaborator

spyhunter99 commented Nov 29, 2017

I also noticed when drawing, some of the lines would immediately have the arrows and some would not. Not entirely sure why but it may be related to multiple "worlds" being visible concurrently

@spyhunter99

This comment has been minimized.

Show comment
Hide comment
@spyhunter99

spyhunter99 Nov 29, 2017

Collaborator

image
last one. seems to work fine with zooming, scaling and rotation

Collaborator

spyhunter99 commented Nov 29, 2017

image
last one. seems to work fine with zooming, scaling and rotation

@jmarks213

This comment has been minimized.

Show comment
Hide comment
@jmarks213

jmarks213 Nov 29, 2017

Thanks for the comments!

@monsieurtanuki
At first, I did use bitmaps to implement this but it ended up being messier than it needs to be. You end up needing multiple bitmaps for each cardinal direction that, after rotating and resizing, end up being pixelized and elongated. Drawing them directly creates a cleaner end result. I think your idea of a separate class extending linear ring is a good idea. I did not consider this being relevant for Polygon as it is not an object I have needed to use before.

@spyhunter99
Thanks for the helpful feedback. The default value for the length of the sides is a static number so it looks fine when the Polyline is thin but when the Polyline has more width the arrow is not also taking up more space. I will change the default value to be based off the stroke width of the line instead of a static number.

I am not sure what you have done with the transparent Polylines? I am not familiar with that case but it can be that the arrows would be preferred to also not be drawn.

jmarks213 commented Nov 29, 2017

Thanks for the comments!

@monsieurtanuki
At first, I did use bitmaps to implement this but it ended up being messier than it needs to be. You end up needing multiple bitmaps for each cardinal direction that, after rotating and resizing, end up being pixelized and elongated. Drawing them directly creates a cleaner end result. I think your idea of a separate class extending linear ring is a good idea. I did not consider this being relevant for Polygon as it is not an object I have needed to use before.

@spyhunter99
Thanks for the helpful feedback. The default value for the length of the sides is a static number so it looks fine when the Polyline is thin but when the Polyline has more width the arrow is not also taking up more space. I will change the default value to be based off the stroke width of the line instead of a static number.

I am not sure what you have done with the transparent Polylines? I am not familiar with that case but it can be that the arrows would be preferred to also not be drawn.

@monsieurtanuki

This comment has been minimized.

Show comment
Hide comment
@monsieurtanuki

monsieurtanuki Nov 30, 2017

Collaborator

@jmarks213 As you are in favor of my idea of a separate class, I'll work on that once your code is merged.

Collaborator

monsieurtanuki commented Nov 30, 2017

@jmarks213 As you are in favor of my idea of a separate class, I'll work on that once your code is merged.

@jmarks213

This comment has been minimized.

Show comment
Hide comment
@jmarks213

jmarks213 Dec 1, 2017

@monsieurtanuki That's fine with me.

I updated it as is to scale the arrows with the stroke width. It looks better. I do see the issue where the arrows are not drawing correctly over the polyline at the extreme zoom levels. I will hold off on looking into it? I think you may get to the fix with the refactor you have in mind.

screenshot_20171201-113627
screenshot_20171201-113617

jmarks213 commented Dec 1, 2017

@monsieurtanuki That's fine with me.

I updated it as is to scale the arrows with the stroke width. It looks better. I do see the issue where the arrows are not drawing correctly over the polyline at the extreme zoom levels. I will hold off on looking into it? I think you may get to the fix with the refactor you have in mind.

screenshot_20171201-113627
screenshot_20171201-113617

@monsieurtanuki

This comment has been minimized.

Show comment
Hide comment
@monsieurtanuki

monsieurtanuki Dec 1, 2017

Collaborator

I do see the issue where the arrows are not drawing correctly over the polyline at the extreme zoom levels

@jmarks213 Could you be more specific / attach a screenshot? Or were you referring to @spyhunter99's screenshot on zoom level 1 with polylines on the left side of the screen and arrows on the right side?

Collaborator

monsieurtanuki commented Dec 1, 2017

I do see the issue where the arrows are not drawing correctly over the polyline at the extreme zoom levels

@jmarks213 Could you be more specific / attach a screenshot? Or were you referring to @spyhunter99's screenshot on zoom level 1 with polylines on the left side of the screen and arrows on the right side?

@jmarks213

This comment has been minimized.

Show comment
Hide comment
@jmarks213

jmarks213 Dec 1, 2017

@monsieurtanuki Yes, the screenshots @spyhunter99 provided show the behavior. The arrows are getting decoupled from the polyline. If you rotate the map the arrows and polyline will sometimes line back up as expected.

jmarks213 commented Dec 1, 2017

@monsieurtanuki Yes, the screenshots @spyhunter99 provided show the behavior. The arrows are getting decoupled from the polyline. If you rotate the map the arrows and polyline will sometimes line back up as expected.

@spyhunter99

This comment has been minimized.

Show comment
Hide comment
@spyhunter99

spyhunter99 Dec 2, 2017

Collaborator

it's acting like the map offset isn't being included.

image
image

Collaborator

spyhunter99 commented Dec 2, 2017

it's acting like the map offset isn't being included.

image
image

@spyhunter99

This comment has been minimized.

Show comment
Hide comment
@spyhunter99

spyhunter99 Dec 2, 2017

Collaborator

the issues with the arrows also appears at all zoom levels once the map has wrapped at least once

Collaborator

spyhunter99 commented Dec 2, 2017

the issues with the arrows also appears at all zoom levels once the map has wrapped at least once

@jmarks213

This comment has been minimized.

Show comment
Hide comment
@jmarks213

jmarks213 Dec 2, 2017

Okay, I understand it a bit better now. The projected pixels are not updated to reflect the offset so they are not a good representation of the lines that are drawn to the screen. I implemented @monsieurtanuki thoughts and it indeed does draw correctly now. I am now getting a rogue arrow for each line but I have to stop for the day. I'll take another look tomorrow.

screenshot_20171202-112708

jmarks213 commented Dec 2, 2017

Okay, I understand it a bit better now. The projected pixels are not updated to reflect the offset so they are not a good representation of the lines that are drawn to the screen. I implemented @monsieurtanuki thoughts and it indeed does draw correctly now. I am now getting a rogue arrow for each line but I have to stop for the day. I'll take another look tomorrow.

screenshot_20171202-112708

@jmarks213

This comment has been minimized.

Show comment
Hide comment
@jmarks213

jmarks213 Dec 3, 2017

I finished with ArrowsLinearRing. It seems to be working fine for all cases now. I added the arrows to Polygon as well.

Have I overlooked anything?

jmarks213 commented Dec 3, 2017

I finished with ArrowsLinearRing. It seems to be working fine for all cases now. I added the arrows to Polygon as well.

Have I overlooked anything?

@monsieurtanuki

This comment has been minimized.

Show comment
Hide comment
@monsieurtanuki

monsieurtanuki Dec 5, 2017

Collaborator

@jmarks213 Did you manage to fix the rogue arrow bug? If you're done, we can merge.

Collaborator

monsieurtanuki commented Dec 5, 2017

@jmarks213 Did you manage to fix the rogue arrow bug? If you're done, we can merge.

@jmarks213

This comment has been minimized.

Show comment
Hide comment
@jmarks213

jmarks213 commented Dec 5, 2017

@monsieurtanuki Yes, it is fixed.

screenshot_20171205-091129
screenshot_20171205-091038

@monsieurtanuki monsieurtanuki merged commit 52531ba into osmdroid:master Dec 5, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@monsieurtanuki

This comment has been minimized.

Show comment
Hide comment
@monsieurtanuki

monsieurtanuki Dec 5, 2017

Collaborator

@jmarks213 Awesome!
I'll play with this new feature, and I may open related issues on the following topics:

  • So far the arrows are displayed as Paths, but I think we should be able to display bitmaps instead (and even to orient them).
  • I don't know how the distance between arrows is computed, but I believe there might be something counter-intuitive in that. For instance, imagine we have a polyline on the equator line, from W180 to E180. If there are only 2 points in my polyline, if there are 20 points, if there 100 points, should we have the same number of arrows? I don't know what users should expect. I think this is not the case in your code, because if you look at the round part of a polyline in your latest screenshots there are no arrows, because I suspect the round part is a succession of close points (where there's not enough space for arrows). Maybe we should rather talk in terms of distance from the start of the polyline: "I draw an arrow every x pixels I drew from the very polyline start, not from the current segment start".
Collaborator

monsieurtanuki commented Dec 5, 2017

@jmarks213 Awesome!
I'll play with this new feature, and I may open related issues on the following topics:

  • So far the arrows are displayed as Paths, but I think we should be able to display bitmaps instead (and even to orient them).
  • I don't know how the distance between arrows is computed, but I believe there might be something counter-intuitive in that. For instance, imagine we have a polyline on the equator line, from W180 to E180. If there are only 2 points in my polyline, if there are 20 points, if there 100 points, should we have the same number of arrows? I don't know what users should expect. I think this is not the case in your code, because if you look at the round part of a polyline in your latest screenshots there are no arrows, because I suspect the round part is a succession of close points (where there's not enough space for arrows). Maybe we should rather talk in terms of distance from the start of the polyline: "I draw an arrow every x pixels I drew from the very polyline start, not from the current segment start".
@jmarks213

This comment has been minimized.

Show comment
Hide comment
@jmarks213

jmarks213 Dec 5, 2017

@monsieurtanuki Thanks for taking an interest and providing your comments.

  • As I mentioned before, I found the bitmap solution to be messier than it needs to be. I look forward to seeing what you come up with.

  • Your second point I agree with. In the current form arrows are drawn at the midpoint between every two points not considering distance. Drawing arrows at consistent distances along the line was something I considered originally but didn't take the time to look into; Arrows between points was more appropriate for my case. I would like to keep arrows at mid points as an available option but I do welcome the additional option for arrows at distance along the line.

jmarks213 commented Dec 5, 2017

@monsieurtanuki Thanks for taking an interest and providing your comments.

  • As I mentioned before, I found the bitmap solution to be messier than it needs to be. I look forward to seeing what you come up with.

  • Your second point I agree with. In the current form arrows are drawn at the midpoint between every two points not considering distance. Drawing arrows at consistent distances along the line was something I considered originally but didn't take the time to look into; Arrows between points was more appropriate for my case. I would like to keep arrows at mid points as an available option but I do welcome the additional option for arrows at distance along the line.

@monsieurtanuki

This comment has been minimized.

Show comment
Hide comment
@monsieurtanuki

monsieurtanuki Dec 8, 2017

Collaborator

@jmarks213 About bitmaps, I'm pretty sure it's safe and easy, because that's the way markers are currently displayed.
In fact, in order to display a map rotation, the MapView canvas is rotated. And as you need markers to be displayed without rotation, you have to rotate them with the opposite rotation value.
Cf. the (edited) code in ItemizedOverlay.onDrawItem:

canvas.save();
canvas.rotate(-aMapOrientation, x, y);
marker.copyBounds(mRect);
marker.setBounds(mRect.left + x, mRect.top + y, mRect.right + x, mRect.bottom + y);
canvas.scale(1 / scaleX, 1 / scaleY, x, y);
marker.draw(canvas);
marker.setBounds(mRect);
canvas.restore();

For the "consistent distance versus segment middle only" arrow display enhancement, we need to compute the actual coordinates (in order to compute the distance in pixels between segment points) and at the same time the truncated coordinates (that we now use in order to simplify the polylines).

Collaborator

monsieurtanuki commented Dec 8, 2017

@jmarks213 About bitmaps, I'm pretty sure it's safe and easy, because that's the way markers are currently displayed.
In fact, in order to display a map rotation, the MapView canvas is rotated. And as you need markers to be displayed without rotation, you have to rotate them with the opposite rotation value.
Cf. the (edited) code in ItemizedOverlay.onDrawItem:

canvas.save();
canvas.rotate(-aMapOrientation, x, y);
marker.copyBounds(mRect);
marker.setBounds(mRect.left + x, mRect.top + y, mRect.right + x, mRect.bottom + y);
canvas.scale(1 / scaleX, 1 / scaleY, x, y);
marker.draw(canvas);
marker.setBounds(mRect);
canvas.restore();

For the "consistent distance versus segment middle only" arrow display enhancement, we need to compute the actual coordinates (in order to compute the distance in pixels between segment points) and at the same time the truncated coordinates (that we now use in order to simplify the polylines).

@MKergall

This comment has been minimized.

Show comment
Hide comment
@MKergall

MKergall Dec 8, 2017

Collaborator

My 2 cents:
Bitmaps would be great for full flexibility on arrow shape.
But then, the user can set polyline color and size: arrow bitmap should take that into account...

Collaborator

MKergall commented Dec 8, 2017

My 2 cents:
Bitmaps would be great for full flexibility on arrow shape.
But then, the user can set polyline color and size: arrow bitmap should take that into account...

@monsieurtanuki

This comment has been minimized.

Show comment
Hide comment
@monsieurtanuki

monsieurtanuki Dec 8, 2017

Collaborator

@MKergall My goal is not to display red or black arrows, but to give the user the possibility to draw (not necessarily oriented) bitmaps, e.g. flags.
Then the user can use whatever polygon line color, polygon background color and bitmap he wants. And we can provide as examples a black arrow bitmap (to be oriented) and a work-in-progress-cone bitmap (to be not oriented).

Collaborator

monsieurtanuki commented Dec 8, 2017

@MKergall My goal is not to display red or black arrows, but to give the user the possibility to draw (not necessarily oriented) bitmaps, e.g. flags.
Then the user can use whatever polygon line color, polygon background color and bitmap he wants. And we can provide as examples a black arrow bitmap (to be oriented) and a work-in-progress-cone bitmap (to be not oriented).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment