r1130 'Apply SafeTranslatedCanvas patch' breaks current Osmdroid based apps

[GoogleCodeExporter]

Hi,

I am referring to:
http://code.google.com/p/osmdroid/source/detail?r=1130
r1130: "Update issue 221. Apply SafeTranslatedCanvas patch. Migrate 
ScaleBarOverlay, TilesOverlay, and MinimapOverlay to SafeDrawOverlay. See issue 
221 for more info."

It seems that the approach with the safe canvas has side effects, breaking 
already working apps which are based on Osmdroid extending its capabilities.

For example to an application in order to accomplish map rotation I use the 
Google MapsDemo example as described in 
"\android-sdk\add-ons\addon-google_apis-google-10\samples\MapsDemo\".
There the map view size is made bigger than the screen in order for the 
rotation to not have black areas at the screen corners.
I suppose that the translation 'to a coordinate system where the origin (0,0) 
is the center of the screen' is the cause that the overlay items and the map 
are now misplaced and shifted. And at rotation the center of rotation is not 
anymore the center of the visible map, but a shift of about half the screen.

I don't know what changes the developers have to make at their code in order to 
use the latest Osmdroid but any help to point the right direction is mostly 
appreciated, as this is a serious issue.

Thanks.

Original issue reported on code.google.com by devemu...@gmail.com on 13 Dec 2012 at 7:46

[GoogleCodeExporter]

Glad it wasn't just me that had issues... I didn't think about the rotation 
modification I also am using it for my main code but I'm sure I reverted to 
just osmdroid's trunk code for the map library and its still broken.

I didn't log an issue as it seemed to only be me affected:
https://groups.google.com/forum/?fromgroups=#!topic/osmdroid/LN8iyeXQebU

Original comment by t...@chellew.co.uk on 14 Dec 2012 at 2:08

[GoogleCodeExporter]

Well I had to change the SafeDrawOverlay.draw method in order to take care of 
above issues. I retrieve the rotation angle (if any) from the matrix values. 
Also I take account of map view being bigger than screen (that is if it is 
hosted in a RotateView) with proper traslation.

//sMatrix.setValues(sMatrixValues);
sMatrix.setRotate(
        (float) Math.toDegrees(Math.atan2(sMatrixValues[Matrix.MSKEW_Y], sMatrixValues[Matrix.MSCALE_X])),
        osmv.getWidth() * 0.5f,
        osmv.getHeight() * 0.5f);
sMatrix.postTranslate(
        -(osmv.getWidth() / 2 - ((View) osmv.getParent()).getWidth() / 2),
        -(osmv.getHeight() / 2 - ((View) osmv.getParent()).getHeight() / 2) + (c.getHeight() - ((View) osmv.getParent()).getHeight()));

Hope these help those that have issues with safe canvas.

Original comment by devemu...@gmail.com on 14 Dec 2012 at 7:30

[GoogleCodeExporter]

A better solution which also takes care of scale would be:

final double angrad = Math.atan2(sMatrixValues[Matrix.MSKEW_Y], 
sMatrixValues[Matrix.MSCALE_X]);
if (angrad != 0) {
    sMatrix.setRotate((float) Math.toDegrees(angrad), osmv.getWidth() * 0.5f, osmv.getHeight() * 0.5f);
    sMatrix.postTranslate(-osmv.getWidth() * 0.5f, -osmv.getHeight() * 0.5f);
}
else
    sMatrix.setTranslate(-osmv.getWidth() * 0.5f, -osmv.getHeight() * 0.5f);
final double angcos = Math.cos(angrad);
sMatrix.postScale(sMatrixValues[Matrix.MSCALE_X] / (float) angcos, 
sMatrixValues[Matrix.MSCALE_Y] / (float) angcos);
sMatrix.postTranslate(
        ((View) osmv.getParent()).getWidth() * 0.5f,
        ((View) osmv.getParent()).getHeight() * 0.5f + (c.getHeight() - ((View) osmv.getParent()).getHeight()));

Original comment by devemu...@gmail.com on 17 Dec 2012 at 9:12

[GoogleCodeExporter]

In the end, it's probably easiest to add an option to disable the safe canvas, 
but really I'd rather fix these things proper. If you are doing any 
manipulation of the Matrix it must all be done with consideration to the 
x/yOffset. If all modifications are done directly to the ISafeCanvas then it 
should all be handled for you.

Original comment by kurtzm...@gmail.com on 19 Dec 2012 at 2:37

[GoogleCodeExporter]

Added ability to disable safe canvas drawing - r1133

Original comment by kurtzm...@gmail.com on 19 Dec 2012 at 8:18

[GoogleCodeExporter]

The option to disable safe canvas drawing is most welcomed!

As many of us already use the provided default canvas in order to draw extra 
things on top of map view and overlays, and simply replacing draw with drawSafe 
does not work as expected.
For example I tried your advise for MyLocationOverlay at
http://code.google.com/p/osmdroid/issues/detail?id=378#c21
and it didn't work for me, the my location arrow disappeared.
The change of origin (if it is the cause) probably has confused existing code 
at many of us.

Original comment by devemu...@gmail.com on 20 Dec 2012 at 10:35

[GoogleCodeExporter]

Hi , by using this safe canvas drawing, I notice on my application using 
osmdroid that at high zoom, my markers on the map are shaking. When disabling 
safe canvas, it's going back to a normal behavior.

using svn rev 1134

Original comment by tamisier...@gmail.com on 3 Jan 2013 at 9:09

[GoogleCodeExporter]

When you have safe canvas drawing off and are zoomed in look closely at the 
tiles at the edge as you scroll - are they really scrolling one pixel at a time 
or are they jumping a few pixels at a time?

I see that the ItemizedOverlay is not a SafeDrawOverlay - I assume you are 
using this overlay and that is why the Markers are jumping. That needs to be 
changed.

Original comment by kurtzm...@gmail.com on 3 Jan 2013 at 1:13

[GoogleCodeExporter]

I updated ItemizedOverlay in r1136 to use SafeDrawOverlay. Give safe drawing 
another shot and see if that works now.

Original comment by kurtzm...@gmail.com on 3 Jan 2013 at 4:19

[GoogleCodeExporter]

yes I'm currently using ItemizedIconOverlay which come from ItemizedOverlay.
And actually I made a mistake, even with the flag of safe canvas set to false, 
the markers of the overlay are shaking,and seem to jump a few pixels.

Original comment by tamisier...@gmail.com on 3 Jan 2013 at 4:46

[GoogleCodeExporter]

and if I revert to svn 1129, markers are effectively properly displayed

Original comment by tamisier...@gmail.com on 3 Jan 2013 at 5:09

[GoogleCodeExporter]

ok I'll try

Original comment by tamisier...@gmail.com on 3 Jan 2013 at 5:09

[GoogleCodeExporter]

Works perfeclty with r1136
thanks a lot.. :-)

Original comment by tamisier...@gmail.com on 3 Jan 2013 at 5:14

[GoogleCodeExporter]

@davemux86: Let me know if this works for you with rotation. Replace this block 
in SafeDrawOverlay.draw():

        if (this.isUsingSafeCanvas()) {

            // Find the screen offset
            Rect screenRect = osmv.getProjection().getScreenRect();
            sSafeCanvas.xOffset = -screenRect.left;
            sSafeCanvas.yOffset = -screenRect.top;

            // Save the canvas state
            c.save();

            // Get the matrix values
            c.getMatrix(sMatrix);
            sMatrix.getValues(sMatrixValues);

            // If we're rotating, then reverse the rotation for now.
            // This gets us proper MSCALE values in the matrix.
            final double angrad = Math.atan2(sMatrixValues[Matrix.MSKEW_Y],
                    sMatrixValues[Matrix.MSCALE_X]);
            sMatrix.preRotate((float) -Math.toDegrees(angrad), screenRect.right, screenRect.bottom);

            // Get the new matrix values to find the scaling factor
            sMatrix.getValues(sMatrixValues);

            // If we're scaling, then we need to adjust our position accordingly
            int xScalingOffset = (screenRect.width() / 2 - (int) (screenRect.width()
                    * sMatrixValues[Matrix.MSCALE_X] / 2));
            int yScalingOffset = (screenRect.height() / 2 - (int) (screenRect.height()
                    * sMatrixValues[Matrix.MSCALE_Y] / 2));

            // Adjust to remove scroll
            sMatrix.postTranslate(screenRect.left, screenRect.top);
            // Adjust to account for scaling
            sMatrix.postTranslate(xScalingOffset, (c.getHeight() - osmv.getHeight())
                    + yScalingOffset);
            // Put back the rotation.
            sMatrix.preRotate((float) Math.toDegrees(angrad), screenRect.width() / 2,
                    screenRect.height() / 2);

            c.setMatrix(sMatrix);
        } else {
            sSafeCanvas.xOffset = 0;
            sSafeCanvas.yOffset = 0;
        }

Original comment by kurtzm...@gmail.com on 17 Jan 2013 at 10:24

[GoogleCodeExporter]

Hi kurtzmarc and thanks for your help!
Unfortunately there are still problems with the first tests.

For start the zoom in/out is not working correctly. Between the fixed zoom 
levels when I press the zoom controls, the intermediate scale seems to position 
the map at the corners of the screen until the zoom animation completes.

Also the rotation is taking place with center the top left corner of the screen 
instead of its center. I have mentioned that my map view size is made bigger 
than screen in order for the rotation to not have black areas at the screen 
corners.

I'll look more in your new implementation.

Original comment by devemu...@gmail.com on 18 Jan 2013 at 7:17

[GoogleCodeExporter]

@davemux86: I should point out this is intended to do away with the need for a 
oversized view so maybe that is complicating the situation. There is more to 
the patch and I will probably post a separate ticket at some point which would 
probably be more useful to you. The patch includes consideration for rotating 
touches properly, and for increasing the area where tiles are drawn (to get rid 
of the black corners). But it's still not perfect - pinch to zoom is not 
working for me and a few other small things. I was hoping the above code might 
be a quick fix.

Original comment by kurtzm...@gmail.com on 18 Jan 2013 at 8:41

[GoogleCodeExporter]

See issue 396. A new maps rotation patch.

Original comment by kurtzm...@gmail.com on 23 Jan 2013 at 3:35

[GoogleCodeExporter]

Everyone with issues

*** Make sure you have Hardware Acceleration turned OFF ***

With it on, I see a some of the problems that have been posted here. With the 
way we do canvas drawing, hardware acceleration is not desirable.

Original comment by kurtzm...@gmail.com on 23 Mar 2013 at 2:58

[GoogleCodeExporter]

See issue 413 regarding hardware acceleration.

I'd like to close this ticket soon - are there any remaining issues that are 
still outstanding?

Original comment by kurtzm...@gmail.com on 29 Mar 2013 at 3:04

[GoogleCodeExporter]

See r1154 for additional compatibility options.

Original comment by kurtzm...@gmail.com on 29 Mar 2013 at 3:05

[GoogleCodeExporter]

No, there are no any remaining issues about this ticket.
Thanks!

Original comment by devemu...@gmail.com on 29 Mar 2013 at 6:02

[GoogleCodeExporter]

This ticket can now be closed. Thanks for all the feedback!

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

• Changed state: Fixed