Adding function to export scenes as glTF #630
Conversation
There was a problem hiding this comment.
Hello @xtanion ,
Great job. It's working well.
I had some issues trying to export actors with no indices such as dot and line actors. It's causing the following error: ValueError: zero-size array to reduction operation maximum which has no identity which is triggered by this section of code:
if indices is not None:
indices = indices.reshape((-1, ))
amax = [np.max(indices)]
amin = [np.min(indices)]
Also, there are some typos in the gltf.py file. It's not related to this PR:
There was a problem hiding this comment.
Hi @xtanion ,
I had a few suggestions regarding the docs which I have mentioned below.
Also, we should discuss the name of the parameters as some of them don't clearly explain what they contain.
I haven't reviewed and tested the code yet, I will do it as soon as possible and let you know if I found anything.
Thanks!
fury/gltf.py
Outdated
| ---------- | ||
| scene : Scene | ||
| FURY scene object. | ||
| name : str, optional |
fury/gltf.py
Outdated
| gltf2glb(f'{name}.gltf', destination=filename) | ||
|
|
||
|
|
||
| def _connect_primitives(gltf, actor, buff_file, boffset, count, name): |
There was a problem hiding this comment.
missing docs for boffset
fury/gltf.py
Outdated
| count += 1 | ||
| material = None if tcoords is None else 0 | ||
| prim = get_prim(vertex, index, color, tcoord, normal, material) | ||
| return prim, boffset, count |
There was a problem hiding this comment.
maybe you should change boffset to byteoffset as per mention in the docs
fury/gltf.py
Outdated
| Pygltflib GLTF2 object | ||
| mesh : int, optional | ||
| Mesh index | ||
| camera : int, optional |
There was a problem hiding this comment.
I think camera_index would be a more explanatory name
fury/gltf.py
Outdated
| gltf.cameras.append(cam) | ||
|
|
||
|
|
||
| def get_prim(verts, indices, cols, tcoords, normals, mat): |
There was a problem hiding this comment.
Maybe you should try using full terms for eg. vertex, color, material, or something similar depending on the value they have. I am also a bit confused because you have written verts (plural) but as per the docs they store int (single value).
fury/gltf.py
Outdated
| return prim | ||
|
|
||
|
|
||
| def write_material(gltf, bct: int, uri: str): |
There was a problem hiding this comment.
writing the entire parameter name for these functions can make the lines very long, also these parameters are explained in the docstring too.
fury/gltf.py
Outdated
| gltf.images.append(image) | ||
|
|
||
|
|
||
| def write_accessor(gltf, bv, bo, ct, cnt, atype, max=None, min=None): |
fury/gltf.py
Outdated
| gltf.accessors.append(accessor) | ||
|
|
||
|
|
||
| def write_bufferview(gltf, buffer, bo, bl, bs=None): |
|
Thanks for the review @ganimtron-10 and @m-agour. Also, thanks @m-agour for finding the method to differentiate between lines and points, using Polydata.GetNumberOfPolys()
Polydata.GetNumberOfLines()
Polydata.GetNumberOfPolys()is easier than comparing the ratio of I figured out the |
Codecov Report
@@ Coverage Diff @@
## master #630 +/- ##
==========================================
+ Coverage 51.63% 52.00% +0.36%
==========================================
Files 105 108 +3
Lines 22829 24066 +1237
Branches 2531 2643 +112
==========================================
+ Hits 11788 12515 +727
- Misses 10657 11145 +488
- Partials 384 406 +22
|
| ============================== | ||
| Exporting scene as a glTF file | ||
| ============================== | ||
| In this tutorial, we will show how to create a glTF file for a scene. |
This PR adds the ability to convert a scene as gltf file and can be loaded back using glTF loader (added in #600 ).