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

vtp support attempt #1181

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

vtp support attempt #1181

wants to merge 1 commit into from

Conversation

kayarre
Copy link

@kayarre kayarre commented Sep 17, 2021

Starting from the vtu implementation I am working through adding vtp support.

Current status: WIP starting with reading, currently I am arbitrarily add the cell types that mimics the vtu description, but does not yet handle the triangle strips.

Suggestions or things to make it better are welcome.

hopefully can resolve #527

@kurtsansom
Copy link

I have done some ad hoc testing and was able to read a .vtp file with only triangle strips and get out a .vtu that looks correct.
I was also able to go from .vtp with vertex, lines, triangle strips to a vtp wiith vertices, lines, and polygons, but the .vtu output appears garbled.

I will need to read the development guide about adding tests and getting everything else up to snuff, but currently content with progress made.

@kayarre
Copy link
Author

kayarre commented Sep 22, 2021

Found my logic mistakes in the reader and wrong indentation in the writer.
It appears to be working for me now. Still need to add some tests.

@kayarre kayarre changed the title WIP: vtp support attempt vtp support attempt Sep 22, 2021
@kayarre
Copy link
Author

kayarre commented Sep 22, 2021

adding simple tests, need to rethink the polys section. should I read the polys section and then separate them into triangles, quads, and polygons?

@kayarre
Copy link
Author

kayarre commented Sep 28, 2021

the current implementation merges triangles, quads, and polygons into polygons on write.
The read process specifies the cell types into the same triangles, quads, and polygons.

Currently meshio doesn't support triangle strips, and I am not sure there is a reason to do so. this means that on read triangle strips get converted to triangles, which become polygons on write to vtp.

This means that any cell dell data from triangle strips gets duplicated for the decomposed triangle strips.

@kayarre
Copy link
Author

kayarre commented Oct 4, 2021

I forgot to commit the test files. they made need a little tweaking as I simply used the vtu files and applied the extract surface mesh in paraview and exported the vtp files. they all have uint64 headers, so 00_raw_binary.vtp and 01_raw_binary_int64.vtp might be redundant.

@kayarre
Copy link
Author

kayarre commented Oct 15, 2021

@nschloe Would it make more sense to derive a new package that contains format extensions rather than include it in meshio?

does anyone have any thoughts or feedback on this effort?

@nschloe nschloe marked this pull request as draft October 15, 2021 09:53
@nschloe
Copy link
Owner

nschloe commented Oct 15, 2021

meshio has support for polygons, so I'd say it makes sense to have it in meshio.

@kayarre
Copy link
Author

kayarre commented Oct 17, 2021

Thank you for the input. tests appear to be passing now. More tests might be good and/or making better binary files tests.

Currently I have the reader generating a warning when it finds strips, not sure if that is what makes sense, but certainly better than silence.

@kayarre
Copy link
Author

kayarre commented Oct 17, 2021

What are the codecov checks checking?

@nschloe
Copy link
Owner

nschloe commented Oct 17, 2021

Coverage

@nschloe
Copy link
Owner

nschloe commented Oct 17, 2021

Can you attach the test files here so I can download them, please?

@kayarre
Copy link
Author

kayarre commented Oct 29, 2021

Apologies for taking so long to respond.

vtp_testfiles.tar.gz

@kayarre
Copy link
Author

kayarre commented Dec 2, 2021

@nschloe any feedback on this?

@nschloe
Copy link
Owner

nschloe commented Dec 2, 2021

Sorry, too much work right now.

@kayarre
Copy link
Author

kayarre commented Dec 2, 2021

Thank you for the update. A kofi is coming your way as soon as I remember how to do it.

@kayarre kayarre force-pushed the vtp_support branch 6 times, most recently from b427eab to e51fbcd Compare December 20, 2021 23:05
@kayarre
Copy link
Author

kayarre commented Dec 21, 2021

I rebased, squashed, and included some matching changes from the vtu reader/writer. tests passed locally for me.

@kayarre
Copy link
Author

kayarre commented Dec 28, 2021

@nschloe would mind running the workflow to see if it passes?

@kayarre kayarre marked this pull request as ready for review January 6, 2022 18:34
@kayarre kayarre force-pushed the vtp_support branch 2 times, most recently from b7796f8 to 92f3f9d Compare February 2, 2022 16:23
  Starting from the vtu implementation worked through adding vtp support.
  Currently meshio doesn't support triangle strips, and not clear
  reason to do so. This means that on read triangle strips get
  converted to triangles, which become polygons on write to vtp.

  Merges triangles, quads, and polygons into polygons on
  write. The read process specifies the cell types into the same
  triangles, quads, and polygons.

  Cell dell data from triangle strips gets duplicated for the
  decomposed triangle strips.

  Rebase/squashed from original draft

  Resolve nschloe#527

  fix: add tests for triangle strip reading
  chore: format
@kayarre
Copy link
Author

kayarre commented Feb 2, 2022

I added more tests to bump up the code coverage.

@kshunterco
Copy link

Hey not sure if this is a welcome bump but I was just wondering what the status of a possible VTP writer. I'm using VTK in Anaconda and the AVS-UCD read function is broken right now AFAIK so I'd like to switch to this but need VTP.

I know python and have LOTS of UCD, VTK, and VTP files to test things with, so if I can help, maybe throw me a bone or I'll try to see how I can help when I get a bit more free time next week?

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.

Feature Request - Support for POLYDATA in vtk format
4 participants