Gltf update #1058
Gltf update #1058
Conversation
…ample with texture
| # ============================================================================== | ||
|
|
||
| if __name__ == '__main__': | ||
| if __name__ == "__main__": |
There was a problem hiding this comment.
I see the value of the changes you've made to this sections pointing out caveats to adding images and textures. But I also see the value of providing a bare minimum example of how to create a GLTF. Do you think it would be worthwhile to keep both?
The other changes in the PR look great!
There was a problem hiding this comment.
thank you beverly!
yes, for now, I would like to keep both, as there might be further improvements needed.
| ------- | ||
| int or None | ||
| """ | ||
| for key, material in self.materials: |
There was a problem hiding this comment.
I think this needs .items()
|
ipy keeps failing, but the tests with ipy work fine on my computer |
|
Since for me locally these tests fail as well, I had a look and to be honest I had difficulties following everything that's happening in this code (again, not familiar with GLTF) so I'll go ahead and put here what I could find and maybe it would have more of a meaning to you, knowing what's going on. Focusing on parsing and exporting of When exporting though, 3 out of the four textures are removed here for being "unvisited" # in GLTFExporter.load()
self._content.remove_orphans()#before removing orphans:
{
0: <TextureData object at 0x0000000000000A7A>,
1: <TextureData object at 0x0000000000000A7B>,
2: <TextureData object at 0x0000000000000A7C>,
3: <TextureData object at 0x0000000000000A7D>
}
# removing unvisited item - key:1 item:<TextureData object at 0x0000000000000A7B>
# removing unvisited item - key:2 item:<TextureData object at 0x0000000000000A7C>
# removing unvisited item - key:3 item:<TextureData object at 0x0000000000000A7D>which results in the following {
0: <TextureData object at 0x0000000000000A7A>
}Later, in The index of the Whether or not these textures should have been removed, or if this should get somehow updated in the references to them inside the extensions, I cannot say. But it seems that the reason these tests don't fail in CPython is that there these textures don't get removed in the first place. @romanarust can you say if these textures should even be removed or not? |
|
@chenkasirer : Thanks a lot for checking and the detailed explanation! |
|
|
I was just thinking about this, and I agree with your suspicions @romanarust. There are a few things in python 2 that can behave a bit differently than in python 3 that might be exposed in that function. In particular, there is always the new vs old style class issue (which I think shouldn't be a problem because everything inherits from |
|
@romanarust @beverlylytle So following your suspicions, I put some more time into this issue and sure enough, the culprit seems to be Excerpt from my debugging extravaganza: # GLTFExporter.check_extensions_texture_recursively
print("a: {} instance:{} is texture:{}".format(a, a_instance, isinstance(a_instance, TextureInfoData)))
...
a: specular_texture instance:<TextureInfoData object at 0x0000000000000352> is texture:FalseThis prevents the texture from being flagged as Annoyingly enough, this problem is intermittent:
To check for the types more reliably I'd suggest either comparing strings e.g. # data_classes.py
class TextureInfoData(object):
IS_TEXTURE_INFO_DATA = True # value is not actually checked..
...
# gltf_content.py
...
if getattr(instance, "IS_TEXTURE_INFO_DATA", False): # instead of isinstance(a, TextureInfoData)
texture_visit_log[getattr(item, a).index] = True
...I'd do the same for the check |
|
oh ironpython... what is dead may never die. |
|
Thank you @chenkasirer for the detailed analysis and yet more proof of why IronPython should be buried. |
|
@romanarust Not your fault actually. Seems there's a new version of I'll sort it out in a separate PR. **Update: opened #1061 |
Gltf update for allowing extensions, see here. Check the changelog for details.
What type of change is this?
Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.CHANGELOG.mdfile in theUnreleasedsection under the most fitting heading (e.g.Added,Changed,Removed).invoke test).invoke lint).compas.datastructures.Mesh.