iosandroiddev / osmdroid

Automatically exported from code.google.com/p/osmdroid
0 stars 0 forks source link

PathOverlay not displayed correctly #221

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Display a PathOverlay (for example an itinerary)
2. Zoom on it (zoom at least 20, by 20 I mean the tile source zoom)

What is the expected output? What do you see instead?
We should see a nice path but the path is not complete. Some lines are missing 
or too small.

What version of the product are you using? On what operating system?
Android 2.2/2.3.4 osmdroid-android-3.0.3

Please provide any additional information below.

Original issue reported on code.google.com by johan.le...@gmail.com on 26 May 2011 at 12:29

GoogleCodeExporter commented 8 years ago
Here is what I got.

The first picture shows the path correctly displayed and the second picture 
shows the same path but with the view zoomed in.

Original comment by johan.le...@gmail.com on 29 May 2011 at 3:27

Attachments:

GoogleCodeExporter commented 8 years ago
I'm struggling with the same problem. Tested it on Android 2.2, 2.3 and 3.1 - 
osmdroid 3.0.6.
This is a problem of drawing all kind of shapes on canvas in higher zoom 
levels. Lines, circles, paths - all are drawn incorrectly (using drawLine 
drawCircle methods on canvas). What is interesting is that from what I see 
bitmaps are still OK in high levels.
I see the problem can be visible starting from 18 zoom level.
The higher level, the bigger problem is.

Screens below (in zoom 20 line disappears but it depends on paint used):

Original comment by m.wilnie...@gmail.com on 1 Feb 2012 at 10:13

Attachments:

GoogleCodeExporter commented 8 years ago
I have the same issue using OSMdriod-3.0.7-snapshot.

More details are at 
http://stackoverflow.com/questions/9273561/osmdroid-pathoverlay-drawing-is-corru
pted-at-high-zoom-levels/9287851#9287851

Original comment by cayle.sh...@gmail.com on 16 Feb 2012 at 2:30

GoogleCodeExporter commented 8 years ago
This issue appears to be rooted in Android. I believe it is due to a rounding 
error that happens when you scroll a view to a large offset (possibly due to 
use of SKScalar). This bug can be isolated by creating a new android project 
with an activity and a view:

 1. Begin with the view at the origin with no scroll offset yet.
 2. Draw a circle on the canvas: canvas.drawCircle(screenCenterX, screenCenterY, 100, mPaint)
 3. Scroll the view to a large number: mainView.scrollTo(536870912, 536870912)
 4. Draw a circle on the canvas: canvas.drawCircle(newScreenCenterX, newScreenCenterY, 100, mPaint)

The first circle draws ok, the second one draws distorted. For further 
evidence, try to draw your path near 0,0 and zoom in - notice the distortion no 
longer appears.

So the solution to this is not easy. Ultimately the only way to combat this is 
to not really scroll the view at all. To keep this working with existing code, 
we would have to do something like:

 1. Clear the translation values that Android applies in the matrix for the canvas. This would effectively ignore the scroll offsets.
 2. Intercept all draw...() methods to adjust x,y values by the expected scroll offsets.
 3. Only do this for overlays and skip when drawing child views of the maps. Also skip when zooming.

This could probably be done with a Canvas wrapper, but that would have to be 
updated as new Canvas methods are added to newer APIs. It's a tricky 
proposition, but it would work. I would like to hear other suggestions.

Original comment by kurtzm...@gmail.com on 2 Jul 2012 at 8:38

GoogleCodeExporter commented 8 years ago
I am attaching a solution to this problem. This patch will add an additional 
method for Overlay to override called drawSafe(). It will pass an ISafeCanvas 
that will intercept all draw methods and adjust them so that they draw properly 
even at high zoom levels. Read the comments for a technical explanation of what 
I am doing.

Implementors don't have to do much of anything different - just change your 
draw() method to use drawSafe() and then draw the canvas like they normally 
would (using the normal coordinate system they have been using all along). If 
they are using any unsafe methods they may have to change their parameters to 
pass a double instead of a float, or a Rect instead of a RectF. Otherwise don't 
change anything else!

Implementors that want to use SafeTranslatedPath only have to call 
onDrawCycleStart() at the beginning of their drawSafe() method. If they are 
using any unsafe methods they will get a deprecation warning along with a 
replacement to use that is safe.

Please test!! This is a pretty significant addition, but it is implemented in a 
way that doesn't disrupt existing projects so I would like to apply this to the 
trunk. Let me know how it works for you!

Original comment by kurtzm...@gmail.com on 12 Sep 2012 at 6:13

GoogleCodeExporter commented 8 years ago
patch attached.

Original comment by kurtzm...@gmail.com on 12 Sep 2012 at 6:13

Attachments:

GoogleCodeExporter commented 8 years ago
Attaching a new patch - includes more documentation and includes a fix for very 
long dashed-lines using DashPathEffect which apparently bugs out around 60,000 
pixels.

Original comment by kurtzm...@gmail.com on 17 Sep 2012 at 7:30

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago

Original comment by kurtzm...@gmail.com on 17 Sep 2012 at 8:39

Attachments:

GoogleCodeExporter commented 8 years ago
Updated patch - fixes issue with SafeTranslatedPath.computeBounds()

Original comment by kurtzm...@gmail.com on 4 Oct 2012 at 7:24

GoogleCodeExporter commented 8 years ago
Note one more change needed from last patch - line 144, drawBitmap(Bitmap, 
Matrix, SafePaint) needs to be postTranslated so as not to screw up any 
rotations, etc:

        sMatrix.postTranslate(xOffset, yOffset);

Original comment by kurtzm...@gmail.com on 25 Oct 2012 at 12:33

GoogleCodeExporter commented 8 years ago
Latest patch - added postTranslate fix from above, and broke Overlay changes 
into their own SaveDrawOverlay class so it is completely optional and keeps 
Overlay.draw() abstract.

Original comment by kurtzm...@gmail.com on 29 Oct 2012 at 5:11

Attachments:

GoogleCodeExporter commented 8 years ago
Additional fix for previous patch - in SafeDashPathEffect line 11, it should be 
using:

PathDashPathEffect.Style.MORPH

Original comment by kurtzm...@gmail.com on 30 Oct 2012 at 5:24

GoogleCodeExporter commented 8 years ago
kurtzm...@gmail - Could you integrate your patches?

Original comment by kre...@gmail.com on 27 Nov 2012 at 7:11

GoogleCodeExporter commented 8 years ago
Posting final patch. Includes fixes for getClipBounds(), the PathDashPathEffect 
fix above, plus the MinimapOverlay is migrated over to SafeDrawOverlay.

Original comment by kurtzm...@gmail.com on 27 Nov 2012 at 6:26

GoogleCodeExporter commented 8 years ago
I think it's time to apply this to the trunk. It's been working well for me, 
it's non-invasive for existing overlays, and I think people are legitimately 
experiencing these issues out in the field.

Original comment by kurtzm...@gmail.com on 27 Nov 2012 at 6:29

GoogleCodeExporter commented 8 years ago
Final patch attached...

Original comment by kurtzm...@gmail.com on 27 Nov 2012 at 6:36

Attachments:

GoogleCodeExporter commented 8 years ago
Applied in r1130.

Original comment by kurtzm...@gmail.com on 27 Nov 2012 at 6:38

GoogleCodeExporter commented 8 years ago
This does not work if you use something like the SlidingMenu. The map stays 
static while the other overlays move correctly. Also there is a small area of 
tiles missing at the bottom.

Original comment by reinhard...@gmail.com on 8 Dec 2012 at 4:10

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Please submit a full bug report that includes exactly what you are doing. 
Specifically where the SlidingMenu is being used (is it added to the MapView, 
is it in a parent layout from the MapView?)

Original comment by kurtzm...@gmail.com on 10 Dec 2012 at 5:50

GoogleCodeExporter commented 8 years ago
I am unable to apply the patch and compile osmdroid (cant for the life of me 
get osmdroid to compile to a .jar).  Do you have the .jar with this change 
implemented by any chance?

Original comment by jamieb...@gmail.com on 10 Dec 2012 at 8:17

GoogleCodeExporter commented 8 years ago
managed to get the source compiled, looks like it was already merged into the 
trunk.  Worked perfectly, thanks!

Original comment by jamieb...@gmail.com on 11 Dec 2012 at 10:45

GoogleCodeExporter commented 8 years ago
I am having the same problem while using osmbonuspack v3.1 and osmdroid v3.0.8. 

Is the problem solved already? What do I have to do to get it working? I posted 
an issue on osmbonuspack with screenshots 
(http://code.google.com/p/osmbonuspack/issues/detail?id=16). 

Thanks in advance,
Greetings!

Original comment by fbo%huma...@gtempaccount.com on 10 Jan 2013 at 4:42

GoogleCodeExporter commented 8 years ago
This fix is currently not in the latest (3.0.8) release. If you can build it 
yourself, the latest trunk has this fix included.

Original comment by kurtzm...@gmail.com on 11 Jan 2013 at 7:55

GoogleCodeExporter commented 8 years ago
Hi. I have tried the patch, but it seems that the path overlay will still be 
distorted at level 20 and above. Not sure if it is due to the incorrect usage 
of onDrawCycleStart() on my part. Anyone else who tried the patch and is able 
to display the PathOverlay without distortion for map levels 20 and above?

Thanks and regards,
PS

Original comment by PingShun...@gmail.com on 8 Mar 2013 at 7:30

GoogleCodeExporter commented 8 years ago
Hi, I guess the PathOverlay did not implement the drawSafe method. Thanks to 
Marc for all the patches.

Thanks and regards,
PS

Original comment by PingShun...@gmail.com on 8 Mar 2013 at 8:46

GoogleCodeExporter commented 8 years ago
The latest build now has this patch. Please post any issues to a new ticket. 
Thanks for the feedback!

Original comment by kurtzm...@gmail.com on 3 Apr 2013 at 9:20

GoogleCodeExporter commented 8 years ago
I've encountered this problem as well but I *think* I've fixed it in a much 
simpler fashion.  I want to emphasize *think* a bit more here because it is 
completely reasonable that I'm missing something obvious.

Instead of the parallel "ISafe..." interfaces and draw method, I modified 
MapView so that the canvas doesn't grow as the zoom level increases.  I keep a 
virtual viewport which maintains the pixel offset used to project GeoPoints to 
pixels and back.  This way, the screen coordinates are always based on the top 
left of the view and the canvas never has to do any math with very large 
floating point values.

This seems to work for me.  Is there a reason it should not?  Am I really just 
an idiot who is missing something obvious?  If this does work like I think, 
isn't this simpler than a parallel draw method?  If this is simpler, you want a 
patch?

Original comment by m...@eucleo.com on 23 Apr 2013 at 5:48

GoogleCodeExporter commented 8 years ago
I believe what you are saying is that you changed the Projection class to do 
the offset when doing LatLong->Pixel and Pixel->LatLong operations. One of the 
goals of the fix we eventually settled with was to have a minimum impact on 
existing implementations. Modifying the Projection class assumes that everyone 
is using the Projection for Mercator calculations and not using the TileSystem 
class or any external classes for Mercator calculations. I think this would 
lead to a lot more "why isn't this working" reports because users aren't 
specifying pixels in regular Mercator coordinates anymore. I think the 
ISafeCanvas abstracts more from the user and is lower-impact (ideally 
no-impact).

Original comment by kurtzm...@gmail.com on 23 Apr 2013 at 7:46

GoogleCodeExporter commented 8 years ago
Just one more follow-up point - I don't think Paths would work as-is unless you 
offset them at every draw cycle since the coordinate system of the Canvas is 
constantly changing (something similar to what we had to do in 
SafeTranslatedPath). Thanks for the suggestions though. I would like to find a 
simpler solution in this area.

Original comment by kurtzm...@gmail.com on 23 Apr 2013 at 8:30

GoogleCodeExporter commented 8 years ago
Right, the projection class changes.  To be clear, osmdroid doesn't assume 
everyone uses Projection to project geopoints to pixels?  Also, how is creating 
a second draw mechanism lower impact?

For me, paths seem to work as is.  I made sure that 
toPixelsProjected/Translated work as expected.

Thanks for listening.  I definitely have a smaller user base so I can see why 
this may be a concern for you.

Original comment by m...@eucleo.com on 23 Apr 2013 at 9:47

GoogleCodeExporter commented 8 years ago
In version 4.2 this problem still exists. Here a proposal for a fix: 
http://textuploader.com/1hw9

Original comment by notdiffi...@gmx.net on 19 Feb 2014 at 10:07

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Instead of fixed maximum values canvas.getMaximumBitmapWidth() and 
canvas.getMaximumBitmapHeight() should be used.

So it should look like this:

private boolean isRectTooBig(Rect bound, Canvas canvas) {
if(bound.right-bound.left >= canvas.getMaximumBitmapWidth() || 
bound.bottom-bound.top >= canvas.getMaximumBitmapHeight())
            return true;
        else
            return false;
    }

Original comment by notdiffi...@gmx.net on 21 Feb 2014 at 7:05

GoogleCodeExporter commented 8 years ago
Please do have anything to earn

Original comment by Spinhor...@gmail.com on 3 Mar 2014 at 1:26