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

fixes #2218 #2222

Merged
merged 1 commit into from
May 12, 2024
Merged

fixes #2218 #2222

merged 1 commit into from
May 12, 2024

Conversation

mikaelcabot
Copy link
Contributor

See issue for details: #2218

@@ -1518,7 +1518,7 @@ def _read_buffers(
# indices are apparently optional and we are supposed to
# do the same thing as webGL drawArrays?
kwargs["faces"] = np.arange(
len(kwargs["vertices"]) * 3, dtype=np.int64
np.shape(kwargs["vertices"])[0], dtype=np.int64
Copy link
Contributor Author

Choose a reason for hiding this comment

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

or could we just assume it's safe to do e.g. (@mikedh) ?

kwargs["vertices"].shape[0]

@mikedh
Copy link
Owner

mikedh commented May 7, 2024

Thanks for the PR! Ignoring the spurious failures that will go away when we can drop gmsh haha looks like the corpus model that fails is this one:

Message: 'assimp-c2967cf79acdc4cd48ecb0729e2733bf45b38a6f/test/models/glTF2/glTF-Asset-Generator/Mesh_PrimitiveMode/Mesh_PrimitiveMode_04.gltf'
Arguments: (ValueError('cannot reshape array of size 4 into shape (3)'),)
Traceback (most recent call last):
  File "/home/runner/work/trimesh/trimesh/tests/corpus.py", line 178, in <module>
    report = on_repo(
             ^^^^^^^^
  File "/home/runner/work/trimesh/trimesh/tests/corpus.py", line 118, in on_repo
    raise E
  File "/home/runner/work/trimesh/trimesh/tests/corpus.py", line 70, in on_repo
    m = trimesh.load(
        ^^^^^^^^^^^^^
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/trimesh/exchange/load.py", line 126, in load
    loaded = load_mesh(file_obj, file_type=file_type, resolver=resolver, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/trimesh/exchange/load.py", line 202, in load_mesh
    results = loader(file_obj, file_type=file_type, resolver=resolver, **kwargs)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/trimesh/exchange/gltf.py", line 326, in load_gltf
    kwargs = _read_buffers(
             ^^^^^^^^^^^^^^
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/trimesh/exchange/gltf.py", line 1587, in _read_buffers
    raise E
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/trimesh/exchange/gltf.py", line 1522, in _read_buffers
    ).reshape((-1, 3))
      ^^^^^^^^^^^^^^^^
ValueError: cannot reshape array of size 4 into shape (3)

@mcadsk
Copy link

mcadsk commented May 8, 2024

changed to using

np.size(kwargs["vertices"])

(tested with Mesh_PrimitiveMode_04.gltf 👌 )

@mikedh
Copy link
Owner

mikedh commented May 9, 2024

Awesome thanks for the followup looks great!! I'll release in #2220 when that's working. Would it be possible to add a small test model that previously failed just to make sure we don't break this in the future? No worries if not, thanks for the fix!

@mikaelcabot
Copy link
Contributor Author

Turns out I was a bit quick there, it actually does not work using np.size(kwargs["vertices"]) for
test.glb.zip, but does work with len(kwargs["vertices"]).

@mikaelcabot
Copy link
Contributor Author

Updated code with tests (cc: @mikedh).

@mikedh mikedh changed the base branch from main to feat/np2 May 12, 2024 03:31
@mikedh mikedh merged commit 0f5f5ce into mikedh:feat/np2 May 12, 2024
2 of 9 checks passed
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.

None yet

3 participants