Update CI #245
Update CI #245
Conversation
|
So Travis CI fails this of course, due to stricter checks. Sounds like Cesium Man and a few others have actual errors that need correction. I suppose we should try to get those fixes onto this branch prior to merge. There are some warnings too, like unused objects and npot textures. Do those prevent Travis from passing? I agree our samples should be more carefully crafted, but there's also something to be said for providing clients with edge cases like these, else engines might get released without testing them. Also, is there any way Travis could print out a summary at the end, enumerating a simple list of which models need attention in order for tests to pass? And finally, it looks like Travis has two actions, and the validation happens in both actions. Is this duplicate work, or why does this happen? Thanks for getting the ball rolling on this. I'll see if I can help fix some of the models, particularly the non-skinning ones. |
Such cases should not happen unintentionally. E.g., we have a dedicated test for npot textures. Also, there are lots of negative tests in the validator repo and in the Asset-Generator output.
The validator sets exit code to
From Travis docs:
We can disable one of them if needed. |
OK. Can we allow individual models to specify individual validation settings? The warning for npot textures should be suppressed for the model that intentionally tests them, for example. This would allow us to produce a clean report of the whole repo.
Sure, but if there are lots of infos and warnings in the mix, and only one model has real errors that turn the CI red, it would be nice to have the offending model(s) called out in an easily discoverable fashion. If one searches the log for the word "Error", there are lots of false positives on Otherwise it can take a lot of digging to understand which models are blockers. This is much less important if we can suppress expected warnings with per-model validation configuration, as presumably any messages in the output should be dealt with.
Interesting. I suppose it doesn't hurt much to have both, it's just noisy. |
For now, it's possible to provide a single validation config (with muted or overridden issue codes) for the whole run. To specify individual settings, we need to either:
|
|
As of now |
|
A new |
|
To fix the models here, perhaps the most historically-official way is to get the latest COLLADA2GLTF and re-convert from the sourceModels folder. Questions:
|
|
@lexaknyazev When you get a chance can you update this branch to |
|
@emackey |
|
CesiumMan is fixed. The process for fixing the remaining two models is now documented in CesiumGS/cesium#8539 (comment). I will need a bit more time to make that all happen. |
This was a simple re-run of COLLADA2GLTF 2.1.5 on sourceModels. Not all models could be fixed by a simple re-run, for example Monster has a strange 90-degree rotation and broken animation loop.
|
🎺 📢 CI has PASSED 🔊 🚀 |
|
Monster was pulled out of sampleModels on this branch. Its animation is too strange. When the animation starts, it jumps 90 degrees to its left, and the animation loop point isn't clean at all. Importing the source COLLADA into Blender really makes a mess there. So I've pulled this asset out entirely, and we should populate this repo in later PRs with more modern, better examples. |
There was a problem hiding this comment.
@lexaknyazev I think this is ready for merge. Any objections?
|
We'll continue to work some of the remaining issues in future PRs. Merging this now to get the new validator implemented in a CI-passable state. |
This PR changes the validation CI to use the recently published precompiled validator release. All other pending PRs must be rebased to ensure updated CI status.
A few highlights from existing models:
AlphaBlendModeTestunused textures