Esri / offline-editor-js

ArcGIS JavaScript library for handling offline editing and tiling.
http://esri.github.io/offline-editor-js/demo/
Apache License 2.0
159 stars 142 forks source link

improve estimateTileSize #107

Closed jabadia closed 10 years ago

jabadia commented 10 years ago

presumably, the size of layer._lastTileUrl will always be the same for the same url. Given that when we are offline the url of _lastTileUrl never changes, there is no need to try to download the tile again and again (and remember, we are offline!)

andygup commented 10 years ago

Yes, when I wrote this it was designed to be static. The logic to determine if estimateTileSize was called again and again is the responsibility of the client. I don't believe there is any code in the offlineTileEnabler library that calls this as an internally used method.

To your point about using this method in an offline condition. There are a couple of approaches that I can think of:

Thoughts?

jabadia commented 10 years ago

One option would be to have an internal variable somewhere that would be returned immediately to the calling code In our getTileUrl() we could launch a request to get one tile and just update that variable in the background. What happens the first time? Is it possible that any code tries to get a tile size before the lib has seen any tile?

Let's make it simple though: this is not core functionality and should not make our code more complex.

Back to your suggestions.

  1. global - I like it because of it's simplicity
  2. wouldn't work if there are no tiles in the db, don't like it
  3. definitely no, KISS (Keep It Simple Sir)

2014-02-25 23:58 GMT+01:00 Andy notifications@github.com:

Yes, when I wrote this it was designed to be static. The logic to determine if estimateTileSize was called again and again is the responsibility of the client. I don't believe there is any code in the offlineTileEnabler library that calls this as an internally used method.

To your point about using this method in an offline condition. There are a couple of approaches that I can think of:

  • We can store the tile size in a global. The approach is easy, assuming we start from an online condition and then go offline. If we start from an online condition then this use case won't work.
  • We can query dbstore.js and grab the first tile image we find and return its size. This approach will make the most sense the library is ready for 100% offline. Example scenario would be reading from a TPK or one of our libraries CSV files.
  • It might make sense to break this out into two methods, such as estimateOnlineTileSize() and estimateOfflineTileSize().

Thoughts?

Reply to this email directly or view it on GitHubhttps://github.com/Esri/offline-editor-js/issues/107#issuecomment-36069685 .

Javier Abadía

andygup commented 10 years ago

this is not core functionality and should not make our code more complex.

True to some extent. This is a helper method and it should be as helpful as possible.

In our getTileUrl() we could launch a request to get one tile and just update that variable in the background.

Yes, I've thought of doing that as well. I also know we don't retry on failure. In the current code, we get one shot at getting a tile size. We should consider adding one retry and tweaking the xhr timeout.

Is it possible that any code tries to get a tile size before the lib has seen any tile?

Yes. In comparison the esri.Map has the same problem. To get around this requires much more sophisticated logic. We would have do something like check for response errors on all tiles. If there were no errors then issue a "tile-cache-loaded" event or a "tiled-cache-failed" if tiles are missing.

Right now we have no control over this.

  1. global - I like it because of it's simplicity

Right now extend() is essentially an overloaded singleton. I think we'll be okay adding a global variable.

andygup commented 10 years ago

@jabadia I did some testing and I don't see a way to change this. We should continue to keep basemap.estimateTileSize() logically separate from getTileUrl(). The current pattern still seems to give the best tile size results.

Take a look at https://github.com/andygup/offline-editor-js/tree/improve_estimatetilesize - I implemented a debouncer in an attempt to only retrieve one file.

The problem is we don't have control over the map "load" event and it can fire before we get a valid tile estimate. Therefore, any code immediately after the "load" event has a high probability of encountering an estimated tile size = 0 or NaN;

Because different zoom levels have different tile sizes, we should continue to allow someone to get a new/refreshed tile size estimate every time the extent changes. Tile size estimates can vary by 25% or greater:

What we really need is direct access to the getTileUrl() callback to cherry pick an image directly from that.