-
Notifications
You must be signed in to change notification settings - Fork 222
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
Material variants support #693
Conversation
Even though this is only an early draft, it could be good if someone (@kring !?) could have a quick look whether the overall approach is not entirely wrong:
One open question is: The Longer-term points that are related to this PR:
|
Thanks again for your contribution @javagl! No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy? I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with |
This PR itself has basically become obsolete with #709 : That PR adds generic metadata access. The structures there allow accessing the metadata/schema/class/properties. Based on that, an example of how the material variants could be accessed is shown here:
If someone has ideas about how to let the code in the
then feel free to chime in here. (I'm not sure whether the template infrastructure of the async system even allows that). In any case, with this utility class, the material variants information can be obtained from a tileset like that:
(with the caveat that one still has to call some functions that internally cause the pending tasks to be processed and the future to actually be resolved. Maybe examples for this can eventually be derived from the test code that will be added in the context of the other PR). |
Addressing #676
Maybe it is possible to split the conceptual questions from some lower-level questions. I'll add a comment with some conceptual questions in the issue.
The lower-level questions:
I tried to find the right place for extracting the (metadata) information from the tileset JSON, and pass it on to the
Tileset
class. The right place for parsing it may be a bit arbitrary. Since this is currently solely operating on therapidjson
document for the tileset, the magicMaterialVariants parseMaterialVariants(const rapidjson::Document& tileset)
function can basically be inserted anywhere (currently in an anonymous namespace of the
TilesetContentManager
). But... this has to be changed for the case that the metadataschema
is not contained in the tileset JSON, but referred to via aschemaUri
. This is related the the question about the "proper way" for passing this information from the place where it is loaded (or where it has to be loaded in the future) to theTileset
object. Having short descriptions about what aTilesetContentManager
actually is (beyond/* A class that manages tileset content */
), or the role that aTilesetContentLoaderResult
plays (beyond that it is/* The result of loading tileset content */
) could be really, really, really helpful.