-
-
Notifications
You must be signed in to change notification settings - Fork 234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add custom tile schema to the TileJSON of cog #1676
base: main
Are you sure you want to change the base?
Add custom tile schema to the TileJSON of cog #1676
Conversation
martin/src/cog/mod.rs
Outdated
|
||
// Add COG-specific metadata to tilejson.other | ||
let mut cog_info = serde_json::Map::new(); | ||
// Add extent bounds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @CommanderStorm ^^
The purpose for this PR is that COG could has tile schema totally different with 3857. And even it's 3857 compatible the user still need to set a custom tile schema (same with 3857) to load it if without the support from #1637
So I plan to add a custom_tile_schema
(a better name required, let's discuss) to tileJSON
.
The field .bounds
woulbe be filled or not in this PR?
Depends on whether the SRS of the COG file could be easily transform to 4326 as per the spec of tilejson
it must be represented in WGS 84. I would only fill this field if it's 4326 already or 3857. It's the simplest case.
- If it's not 4326/3857,
bounds
would be left default(-180, -85.05112877980659, 180, 85.0511287798066) - If it's 4326/3857,
bounds
would be filled
The external extent field should be named .custom_tile_schema.bounds
or.custom_tile_schema.extent
?
It's a good question. Let's discuss. ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is in a differen projection, lets not use the bounds
name.
Some rendering engines might use the field to not request tiles ^^
A nested {
minzoom: 4,
maxzoom: 14,
origin: [0, 0],
resolutions: [ 1.1, 2.2, 3.3, ....],
tilesize: 256,
...
} |
The nested
The flat
flat or nested, I'm a little prefer flat now. |
CC @nyurik |
It's still an early draft. We could decide flat or not after all fields added. |
If we want to modify tilejson, we may need to step back for a bit and come up with a "standard-compatible" way to:
|
eff7477
to
87ba239
Compare
87ba239
to
ba222b6
Compare
ba222b6
to
78a9a05
Compare
I believe it's almost there but got really busy these days, would be back next week |
wow, quiet a lot of work went into this one! @sharkAndshark what did you think about my last comment? |
SchemaNo new tile schema added in this PR. The y-axis for tile index is from top to down in COG. I would say custom MIMEI was not aware of this. But seems we no worry about it? The format output tile of COG is TileJSON compatibleI prefer add all these fields nested in a |
@@ -1,4 +1,28 @@ | |||
{ | |||
"custom_grid": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I'm not sure shoud we added them here( It's a little bit ugly..)
Maybe add these fileds to the json of /catalog
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curl http://127.0.0.1:3000/catalog
{
"fonts": {...},
"spirtes": {...},
"tiles": {....},
"tileGrid": {
"rgb_u8": {....}
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we need to figure out a general way to support tile grid not 3857 in Martin.
If it's difficult to make a decision about where to place these fields maybe it's not bad to make a CLI to output them(or just print them in console log). The compatibility would be greatly honored. And maybe discuss more about support for tile grid not Web Mercator in another issue. It could be related this PR I guess. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I keep thinking how we can stabilize the custom grid thing - perhaps collaborate with the OpenLayers on how to parametrize it? You already have most of the needed values there I think
docs/src/sources-cog-files.md
Outdated
layers: [ | ||
new TileLayer({ | ||
source: new XYZ({ | ||
url: 'https://api.maptiler.com/maps/basic/{z}/{x}/{y}.png?key=vgdy5zgzM4IIqQCC6SOn', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably shouldn't have a key here, especially since it is not related to COG
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops. I should remove this.
update doc
I suggest to remove the doc about cog before we figure out how to support non Web Mercator tile grid and how to tell user the tile grid of cog. I'm sorry for this. But definitely I would keep working. @nyurik |
Thank you so much for your hard work on this! I am sure we can figure it out - clearly not an easy thing to do right, and even harder to change once we release it. Luckly, we are not the first to tackle this problem, so should use the knowledge gained by others |
Yea! Let's do it! ^^ |
To load tiles from COG the fields in #1676 is required. To make the users not confusing, I suggested to remove the doc of cog before * find generally way to support non web mercator tile grid in martin * find the way to tell user the fields in #1676 Let's see cog as a nightly feature I will keep working on this. And I've pinned the #343 , get more inspiration form other repos and talk more there! @nyurik
BTW, #611 was trying to add non-web-mercator projection - we may want to consolidate these efforts |
No description provided.