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 512 (and others) #520

Closed moagrius closed 5 years ago

moagrius commented 5 years ago

@peterLaurence I did spend quite a bit of time verifying these fixes on both an emulator and a real device, so I think I'm ready to submit for review. If you find that what you see does not meet what I'm describing, please stop immediately and kick it back to me; I don't want to waste your time.

This PR should fix several issues:

  1. You can now scale past 100% safely. You won't get a flicker and you won't clip.
  2. Move and remove marker API has been updated. There is debug code in demo/src/main/java/com/moagrius/TileViewDemoAdvanced.java; check the inline comments to see where you can try different demo code. For example, in one block, tapping any marker will move it to position 0, so they'll all stack up - you can see the effect of the opacity increasing. If you comment that out, and uncomment the following block, tapping a marker will remove it. I've added some additional methods to the coordinate plugin to make this simpler as well.
  3. The background plugin (LowFidelityBackgroundPlugin) now works properly and scales with the tiled overlay.
  4. The COVER and CONTAIN scale modes should work properly now. demo/src/main/java/com/moagrius/TileViewDemoAssets.java has debug code with each of the three modes, and by default uses CONTAIN. TileViewDemoAdvanced uses COVER by default.
  5. My map sizes were somehow off in all the demos, causing it to look strange when centered. I was able to correct this by using the appropriate number of tiles, and deleting extraneous columns.
  6. Upgrades from com.moagrius:scrollview from 1.0.5 to 1.0.6. This updates many methods from private to protected; since the libraries are separate, I've found it handy to be able to override methods in the TileView subclass rather than updating the library and importing it in separate repo. More importantly, it establishes save and restore instance state logic that works - if you checkout the 1.0.6 demo, you can see it in action. However, TileView needs a lot more work - we have tile to compute and render, preparation logic to work around, etc. You have already filed a ticket, and I've stubbed out the method in TileView with a TODO; that will be the next ticket I work on.

I think that covers it. Let me know how it goes; please feel free to sit on this until it's convenient for you.

Thanks!

p-lr commented 5 years ago

Great! I hope I'll have time to do this today. If not, it will be no later than tomorrow.

moagrius commented 5 years ago

Great, thank you!

p-lr commented 5 years ago

So after testing, all your points are ok. Good job ! Although there's a new issue in this issue-512-w branch: in the TileViewDemoAdvanced, if I rotate the device (the activity and TileView get re-created), the small path and the markers are shifted to the upper left of the screen and the scale is very small. Looks like an initialisation issue or something that wasn't saved then restored.

moagrius commented 5 years ago

That's actually great news and I'm quite relieved, since I know about the save/restore bug (that's the souce of the issue, it happened as soon as I put the save and restore methods on ScailingScrollView. This is the very next thing I'll be working on before publishing a new release, then ill fix the remote image thing.

Are you +1 for me to merge into master, but wait to cut a release until save and restore are fixed?

(Also, TYVM)

p-lr commented 5 years ago

Yes, +1 for merge, but wait for save and restore fix before the new release.

moagrius commented 5 years ago

Great. I'll do it tomorrow (it's 4:30 am now), and jump on the save and restore ticket immediately after if I have time, or tomorrow night.

Thanks again.. I'm looking forward to the eventual release so we slow down on development for a bit

moagrius commented 5 years ago

I've cleaned up the debug code, and done some fixes around the remote http demo, more to do still: #519

Merging into master.