Implementing higher zoom levels for coastal datasets #425
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
updating master (geoff's)
Merge new master into downscaling
- deleted geojsons from previous attempts
- forgot this was important
- exactly the same as having preloading at infinity for max zoom level 8 - this helps performance at higher zoom levels on lower end machines
Downscaling
- changed layer_bath url back to the old api so old caches still work
pull eslint
updating from master
|
The new MBTiles files are being hosted on Dory and have been included in the install script. |
htmlboss
reviewed
May 17, 2019
| f.write(tile[0]) | ||
| with gzip.open(directory + ".pbf", 'rb') as gzipped: | ||
| with open(directory, 'wb') as tileout: | ||
| shutil.copyfileobj(gzipped, tileout) |
There was a problem hiding this comment.
If I'm reading this code correctly, we're:
- Writing out the fetched tile from sqlite as a
.pbf - Reading it back in through gzip.
- Copying the contents of that
.pbfinto the requested tile file.
Do I have this correct?
There was a problem hiding this comment.
Yes, that's what's happening. If you have a nicer way off the top of your head I'm all ears
There was a problem hiding this comment.
I'm wondering can we just write directly to the tile file and skip the pbf entirely?
There was a problem hiding this comment.
Seems like gzip doesn't like bytesio objects for some reason. I'll create an issue and we can look at this later.
- Defined as a variable - Now using MAX_ZOOM[this.props.state.projection]
Minor changes / fixes
- Made mbt_imp a bit clearer
- code more likely to run now runs first
PR review merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is this PR doing?
Resolves #356
Resolves #357
Resolves #358
Implementing a way to display zoom levels past level 8 (currently levels up to 13 are working with this update). Next step is to add the coastal data to dory so this feature can be used efficiently.
This also resolves #4 (finally!) by changing the source of the tiles on zoom level 8.
Why have you chosen this approach?
The original idea in #356 was to use a vector layer with a single geojson file to display bathymetry and do something similar for #358. Turns out rendering a massive geojson file is very tough on performance, even with a simplified file.
VectorTile layers are easier on the client and don't have a single 500MB file associated with it that needs to be downloaded before anything can be displayed. The mbt format (mapbox tiles) is a compressed version of a geojson, so they're smaller and require even less resources.
The mbt data is stored in SQLite files with the extension ".mbtiles", if not for this each installation of the navigator would need to downloading and extracting an archive with 4^13 sub directories (this takes far too much time).
Should reviewers focus on anything specific?
Check the sanity of the API changes, since some old code (only slightly) was changed to this working.
npm run lintand resolved all errors.