search

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

Issue 511 - Save and Restore Instance State #522

Closed moagrius closed 5 years ago

moagrius commented 5 years ago

@peterLaurence So this is branched from 510. I've spend a few days on it now and I'm not clear at all about what's failing. Version 2 worked as expected out of the box - I suspect it's the whole builder/prepare thing, but that doesn't make a ton of sense either because onCreate is firing again so the new Builder should take precedence. I think it might have something to do with when scale is being set that's throwing off the plugins, because I can get it right with just the tiles, it's the plugins (markers, paths, most obviously) that are breaking.

Since I'd like to get all the other fixes in a release, I've applied a simple workaround: suppress config changes in each of the demo activities. That's obviously not a long term solution but I think does buy another week or so to figure out what's wrong with save/restore.

Note that this is branched from 510, so ideally i'd merge this into 510, then 510 into master, if it passed code review.

The whole UUID and getBuilder stuff are just artifacts. I'll clean it up before more, but they're not being used now in any way.

Thanks as always.

p-lr commented 5 years ago

So its working, but with a constant shift. Let me explain: 1- I pan the map so a specific point of interest is exactly at the center of the screen, in portrait mode 2- I rotate - I'm in landscape mode - and now the POI is not at center as user would expect, but a little shifted to the bottom left 3- Alternating portrait - landscape don't show additional shift

TV2 don't have this little shift issue.

moagrius commented 5 years ago

yeah TV2 has proper save and restore (see my initial comments in this PR). this is definitely not a solution but rather a temporary bandaid, so that we can get the stuff from 510 into a release. I would immediately resume working on proper save and restore (in my feature branch, everything works except plugins - I think it's a timing issue with the new builder/prepare pattern).

So yeah to just summarize: this is only meant as a bandaid to get 510 out. as soon as I can I would implement the proper fix (I just don't know how, at this point).

p-lr commented 5 years ago

Yes, developing a fully working TileView is anything but easy. This mixes view subtilities, threading, high throughput requirements,..

moagrius commented 5 years ago

I know :sob: It's been rough, I'm just hoping to drive through these last few items then let it coast for a really long time on it's own.

So are you +1 for this temporary patch, or would you prefer to handle it differently?

p-lr commented 5 years ago

Oh sure, this totally acceptable and enough for most people I think

moagrius commented 5 years ago

awesome, i'll merge this into 510, then 510 into master, upgrade the version, cut a release, then start back on save + restore next week. i'm hoping that's everything.

thanks again!