Skip to content
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

Metadata availability for implicit tiling #440

Closed
wants to merge 2 commits into from

Conversation

ErixenCruz
Copy link

@ErixenCruz ErixenCruz commented Nov 25, 2020

Copy link
Contributor

@ptrgags ptrgags left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ErixenCruz like we talked about, we need a little more clarification on this PR before I review again.

Comment on lines +10 to +12
"additionalProperties": {
"$ref": "3DTILES_metadata_implicit_tiling_object.schema.json"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lilleyse I talked with @ErixenCruz about this. we clarified the syntax for allOf, however, we had a question, so we didn't commit any changes yet.

Does this extension directly depend on 3DTILES_metadata? if so, can we reuse the instanceTable.property.schema.json and just mention in the spec that JSON encoding is forbidden?

If not, should we copy in the schema file and just remove the parts we don't use? Not sure if we should use the same filename or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it depends on 3DTILES_metadata, and you can copy instanceTable.property.schema.json in the same way that was done for EXT_feature_metadata (I forget the specifics, but we would disallow JSON encoding here as well)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #440 (comment). I think it actually depends on 3DTILES_tile_metadata, which depends on 3DTILES_metadata

@@ -0,0 +1,16 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on the answer to the other question, we might want to merge this file with instanceTable.property.schema.json, just not sure which is the best way to do that yet.

@lilleyse
Copy link
Contributor

Ultimately I think this should depend on 3DTILES_tile_metadata instead of 3DTILES_metadata. However it does make the extension nestedness even worse which is a downside to splitting metadata across multiple extensions.

Maybe it's time to revisit 3DTILES_tile_metadata and get that into shape. Currently it just talks about explicit tiling. This branch would be a branch off of that.

@ErixenCruz
Copy link
Author

What does getting 3DTILES_tile_metadata into shape entail? I can work on it. Is there an issue or list somewhere about what should be done?

@liuyingbin123
Copy link

If we can't get the actual size of tile in memory, we will never be able to accurately control the memory usage of tileset. Because the files on the disk are compressed.the browser will decompress data in the process of using, which will lead to the increase of the amount of data.

@lilleyse
Copy link
Contributor

Will be folded into 3DTILES_metadata

@lilleyse lilleyse closed this Jan 20, 2021
@lilleyse lilleyse deleted the implicit-tiling-metadata branch January 27, 2021 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants