MbTilesTileSource returns null when tile is missing.

[kwesolowski]

MbTilesTileSource method public byte[] GetTile(TileInfo tileInfo) returns null when tile does not exists in SqLite database (it affects both BruTile.MbTiles and MbTiles.Pcl).

I think that such mechanism for error report is not expected by Mapsui.Fetcher (it expects exception, null image is passed further to _fetchTileCompleted(this, new FetchTileCompletedEventArgs(error, false, _tileInfo, image));).

This causes null tile to be fetched indefinitely (retries are ignored as this is not treated as error).

Should I either:
a) Create pull request for BruTile.MbTiles.Pcl where public byte[] GetTile(TileInfo tileInfo) throws exception? Any suggestion for exception type (sqlite returns null)?
b) Create pull request to Mapsui, to check if returned image is not null, and then set error in _fetchTileCompleted(this, new FetchTileCompletedEventArgs(error, false, _tileInfo, image));).
c) Change Mapsui to not propagate DataChanged event when changed image is null?

[FObermaier]

Yes that should be settled. The code comments :) on the ITileProvider interface should be extended to what the clients of the interface should expect.

[kwesolowski]

If i understand correctly, the contract is:
a) Exception means not expected error, and retries limit is used in fetcher,
b) null value means that this tile is not available, and fetcher should use other resolutions to render this area.

But i would love clarification before making any changes.

[pauldendulk]

My original idea was that ITileProvider would throw or return a tile. I did not think about the scenario where a null result would be valid. I now think it makes sense in some cases. But I am not sure how a fetcher like the one in Mapsui should deal with this. Perhaps it should hold a representation of a missing tile (maybe just a null value) to prevent further attempts to fetch the tile. Right now I don't know exactly what the consequences will be. Should all renderers now check for null on all byte arrays.

[pauldendulk]

@kwesolowski 'This causes null tile to be fetched indefinitely', is this in a loop or on each viewport change?

[FObermaier]

I just took a look at http://www.dotnetperls.com/exception.
Bottom line: exception is for exception. Using tests like if (tile == null) ...; tends to be faster.
Suggested is tester-doer pattern, like
bool TryGetTile(TileInfo info, out byte[] tile);

[pauldendulk]

Yes, exception is for exceptions but you could also argue that a provider with a schema should return a tile for all items in the schema. This is what is promised by WMTS and TMS. By allowing null the meaning of a schema changes to it may have tiles available within it's schema. Perhaps this should only be allowed for caches.

[kwesolowski]

@pauldendulk I created RenderGetStrategy logging cache hit/miss in:

....
var feature = cache.Find(tileInfo.Index);
if (feature == null)
....

If all visible tiles are fetched rendering occurs when underlying data change (during fetching), and finally rendering is not required (CPU usage=0%).

But with some missing tiles (we use MbTiles with greater detail level in our area of interest) following occurs:

1. Rendering with cache misses (i am not sure if it causes fetch, or if missing data is reponsible for fetch)
2. Data fetching (including null tiles), causes DataChanged notification (despite fact that image is still null),
3. DataChanged causes rendering (and cache misses) (goto 1).

I have temporary "fixed" it by supressing data changed notification after fetching null tile, but would like to solve it "better".

[pauldendulk]

Thanks I will try to simulate this in debug mode.

[kwesolowski]

Have you tested it? Any of suggestion from initial post seems best viable option to explore and create PR?

[kwesolowski]

UPDATE: I have read more about MbTiles, and I think our files where literally, completely not up to spec. For different areas we should use separate files, and then I guess separate layers? Therefore i think throwing exception would be best solution hear (we would not go that far if exception would stop us:)).

[pauldendulk]

Thanks for the update. Still for practical purposes not throwing could be handy. Perhaps this should be configurable. Also Mapsui should not go into a loop if BruTile throws.

[pauldendulk]

Accepting that GetTile can return null. Mapsui needs to be fixed.