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

Add check for empty polydata topology #46

Merged
merged 3 commits into from
Aug 17, 2024

Conversation

Siimeloni
Copy link
Contributor

When parsing a .vtp file using vtkio the program crashed with this error message:

The application panicked (crashed).
Message:  index out of bounds: the len is 0 but the index is 0
Location: /home/maik/Rust/vtkio/src/xml.rs:1569

The problem was that the data vector is always used, even if the vector is empty but still Some. This happened for my files when the different types of topology are specified in the file but don't contain any data. E.g. like this:

      <Polys>
        <DataArray type="Int64" Name="connectivity" format="ascii" RangeMin="0" RangeMax="95">
        </DataArray>
        <DataArray type="Int64" Name="offsets" format="ascii" RangeMin="28" RangeMax="96">
        </DataArray>
      </Polys>

So I added a check that also returns None if the Vectors of offset or connectivity are empty.

Fix a bug that occurs, when an topology type (verts, lines, polys, strips) are defined in the .vtk file but contain no data.
@elrnv
Copy link
Owner

elrnv commented Aug 16, 2024

Thank you for finding this issue and submitting a PR! Unfortunately this is not the right solution here. We should indeed parse the empty DataArray as an empty DataArray element. The VTKFile type is ought to represent what is in the xml as closely as possible to avoid confusion and inconsistencies. The right solution is indeed to fix the conversion to the model version, where in xml.rs at line 1569 we indeed should avoid assuming that there is at least one element and instead return an empty buf if this happens. It is also arguably a simpler solution.
Please let me know if you have any questions about this change. It would also be amazing to have a test for this included in the PR.

Thanks again!

@Siimeloni
Copy link
Contributor Author

Okay, thanks for the feedback. I will look into it again!

@Siimeloni
Copy link
Contributor Author

So, I added that the string variable contains an empty String if data is empty. I also added a test that creates two DataArray objects. One of them is empty, the other contains data. For me the test works.

I hope I got what you meant with empty buf. Please let me know if that was not the right solution or something doesn't work.

@elrnv elrnv merged commit 02d9100 into elrnv:master Aug 17, 2024
1 check passed
@elrnv
Copy link
Owner

elrnv commented Aug 17, 2024

Yes this is perfect, thank you!

@Siimeloni Siimeloni deleted the Siimeloni-patch-empty-polydata branch August 26, 2024 08:19
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.

2 participants