Skip to content

Conversation

@daker
Copy link
Collaborator

@daker daker commented Aug 29, 2025

Context

Results

Changes

  • Documentation and TypeScript definitions were updated to match those changes

PR and Code Checklist

  • semantic-release commit messages
  • Run npm run reformat to have correctly formatted code

Testing

  • This change adds or fixes unit tests
  • Tested environment:
    • vtk.js:
    • OS:
    • Browser:

@daker daker force-pushed the feat-cleanpolydata branch from 69ef3be to 6333947 Compare August 29, 2025 23:36
@daker daker force-pushed the feat-cleanpolydata branch from 6333947 to 2739e7d Compare September 2, 2025 15:14
@daker daker requested a review from finetjul September 2, 2025 15:20
@daker daker marked this pull request as ready for review September 3, 2025 12:44
@daker
Copy link
Collaborator Author

daker commented Sep 10, 2025

@finetjul any chance i can get another review ? i will add a screenshot before the merge

@daker daker force-pushed the feat-cleanpolydata branch 3 times, most recently from 8ff597c to 09cae27 Compare September 17, 2025 13:43
@daker daker requested a review from finetjul September 17, 2025 13:57
Copy link
Member

@finetjul finetjul left a comment

Choose a reason for hiding this comment

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

I'm not sure what the screenshot emphasize. Can you please explain ?

@daker
Copy link
Collaborator Author

daker commented Sep 17, 2025

@finetjul Well i am not sure what screenshot i can put there to illustrate the filter, it's the example screenshot with the toolbar hidden

image

@finetjul
Copy link
Member

@finetjul Well i am not sure what screenshot i can put there to illustrate the filter, it's the example screenshot with the toolbar hidden

How about a before/after image:
image

@finetjul
Copy link
Member

@finetjul Well i am not sure what screenshot i can put there to illustrate the filter, it's the example screenshot with the toolbar hidden

Actually I like your screenshot but it must keep the "panel" (make it bigger ?).
Can you compute normals on the output mesh ? that would emphasize that points have been merged.
Alternatively it could be a "collage" of the cube before/after cleaning like I did above.

@daker
Copy link
Collaborator Author

daker commented Sep 18, 2025

@finetjul maybe something like this (i didn't touched the normals)

image

@finetjul
Copy link
Member

I'm not sure to understand the color scheme... (pleaes make the cube largers)

@daker
Copy link
Collaborator Author

daker commented Sep 18, 2025

@finetjul something like this ?

image

@finetjul
Copy link
Member

Maybe with a glyph mapper to show that only 1 point is per face extremity.

image image

@finetjul
Copy link
Member

@daker
How about this one:
image

@daker
Copy link
Collaborator Author

daker commented Sep 25, 2025

@finetjul yes i was trying to do that after your comment but was stuck somehow, can you share the code ?

@finetjul
Copy link
Member

I did it with Paraview and gimp:-)

@daker daker force-pushed the feat-cleanpolydata branch from 09cae27 to 604fa00 Compare September 27, 2025 18:52
@daker
Copy link
Collaborator Author

daker commented Sep 27, 2025

@finetjul I updated the example and replaced the screenshot.

Copy link
Member

@finetjul finetjul left a comment

Choose a reason for hiding this comment

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

LGTM

@finetjul finetjul added this pull request to the merge queue Sep 28, 2025
Merged via the queue into Kitware:master with commit 1b45543 Sep 28, 2025
2 checks passed
@github-actions
Copy link

🎉 This PR is included in version 34.13.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot added the released Automated label label Sep 28, 2025
@daker daker deleted the feat-cleanpolydata branch September 28, 2025 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released Automated label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants