moagrius / TileView

TileView is a subclass of android.view.ViewGroup that asynchronously displays, pans and zooms tile-based images. Plugins are available for features like markers, hotspots, and path drawing.
MIT License
1.46k stars 337 forks source link

Calling addMarker causes a crash #1

Closed klattimer closed 11 years ago

klattimer commented 11 years ago

08-07 15:59:00.230: E/AndroidRuntime(20562): FATAL EXCEPTION: main 08-07 15:59:00.230: E/AndroidRuntime(20562): java.lang.IllegalStateException: The specified child already has a parent. You must call removeView() on the child's parent first. 08-07 15:59:00.230: E/AndroidRuntime(20562): at android.view.ViewGroup.addViewInner(ViewGroup.java:3435) 08-07 15:59:00.230: E/AndroidRuntime(20562): at android.view.ViewGroup.addView(ViewGroup.java:3306) 08-07 15:59:00.230: E/AndroidRuntime(20562): at android.view.ViewGroup.addView(ViewGroup.java:3282) 08-07 15:59:00.230: E/AndroidRuntime(20562): at com.qozix.tileview.markers.MarkerManager.addMarker(MarkerManager.java:46) 08-07 15:59:00.230: E/AndroidRuntime(20562): at com.qozix.tileview.markers.MarkerManager.addMarker(MarkerManager.java:42) 08-07 15:59:00.230: E/AndroidRuntime(20562): at com.qozix.tileview.TileView.addMarker(TileView.java:449)

Removing my calls to addMarker prevents the crash, am I missing something?

moagrius commented 11 years ago

08-07 15:59:00.230: E/AndroidRuntime(20562): java.lang.IllegalStateException: The specified child already has a parent. You must call removeView() on the child's parent first.

Looks like you're trying to add a View that's already in another ViewParent. You need to detach it from the display list before adding it (just like you would with any addView call).

HTH, LMK if that solves it.

klattimer commented 11 years ago

Seems I was re-using the marker imageview when I shouldn't have been, I've fixed that problem and also fixed an off-by-one error in the DetailLevelPatternParserDefault which was causing my map to mis-align by one tile. However the markers never stay in the same location relative to the map, they shift back as if they too are off by one tile.

klattimer commented 11 years ago

I figured out why the markers aren't aligned and I'm not sure if it is intentional. Initially I'd specified the anchor x/y to be 0.5, 1.0 (e.g. from the top left, half the width in and the entire height down) however it appears that this is incorrect and instead I've used -0.5 and -1.0 which seem to work a lot better. This is not intuitive as you would imagine that you're effectively specifying a percentage of the image width and height. I imagine that this is because you want to shift the anchor point by half the width in the negative direction however from an API perspective it's slightly confusing.

moagrius commented 11 years ago

The math is x + (child.measuredWidth * anchorX)

It's much more common to have a google-maps style offset, where the image is pointing down and centered over the point, but I believe implying a negative offset is presumptive. It's actually gone back and forth between the (implied negative, or not) in the old widget (MapView)

klattimer commented 11 years ago

I think what you should try to express is the percentage from top left of the image (or even bottom left) to the pixel of the image you wish to align with the point you're specifying on the map. So flipping these to imply a positive is better for the API consumer.

You can also, for instance specify exact pixel co-ordinates as per using 33 / image.getWidth(), 22 / image.getHeight() to calculate the anchors and the API consumer doesn't need to flip it.

moagrius commented 11 years ago

It's been both ways (in the previous widget, MapView); I settled on this for a number of reasons, but mostly because presuming a google-maps style marker didn't make sense - this widget can be used for a variety of applications that aren't necessarily map-centric, and positive offsets are very likely. When setting offsets, I think flipping the sign implicitly is more confusing to the consumer.

FWIW, when it was implicitly flipped, users said the same thing, that it was confusing. I think it's mostly just personal preference, or the perspective from user's specific use case.

I'm always happy to try to fix bugs, or add features, but it's pretty unlikely I'll break the existing API. I am considering converting all TileView/ZoomPanLayout private members to protected, for more flexible extensions... I'll see if there's a need for that. Otherwise, feel free to fork your own version.