8000 Add offset property to Tpms to be able to update it by kmarchais · Pull Request #55 · 3MAH/microgen · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add offset property to Tpms to be able to update it #55

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
May 3, 2024
Merged

Add offset property to Tpms to be able to update it #55

merged 10 commits into from
May 3, 2024

Conversation

kmarchais
Copy link
Member
@kmarchais kmarchais commented Apr 28, 2024 •
edited
Loading

This PR brings a property for the offset attribute.

TLDR

  • Add offset property to update the offset without recomputing the grid
  • Fix for return types where an UnstructuredGrid was returned instead of the expected PolyData
  • Rename vtk_sheet to grid_sheet for more clarity (same for skeletals)

Details

Especially with the new infill feature, the generation of a Tpms object can have a computational cost pretty high due to the number of points in the grid. For an Infill object, the clip_surface method is applied on the grid and is expensive.
If we need to change the offset but keep the same grid, the grid does not need to be computed again.

At the same time, this PR fixes an issue of return type for the sheet, lower_skeletal, upper_skeletal properties and the generateVtk method where UnstructuredGrid were returned instead of a PolyData. The extract_surface is now called to return a PolyData instead of a UnstructuredGrid. The vtk_* methods have been renamed to grid_* for more clarity as their purpose is to return the corresponding UnstructuredGrid.

For now we can give a variable offset but not a variable density. It would be nice to make this possible in this PR or another.

ricardo0115
ricardo0115 approved these changes May 3, 2024
View reviewed changes
Copy link
Collaborator
@ricardo0115 ricardo0115 left a comment

Choose a reason for hiding this comment

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

LGTM

@ricardo0115 ricardo0115 merged commit 48e63ba into main May 3, 2024
@ricardo0115 ricardo0115 deleted the feat/improve-offset-and-density branch May 3, 2024 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ricardo0115 ricardo0115 ricardo0115 approved these changes

@chemiskyy chemiskyy Awaiting requested review from chemiskyy

@ylgrst ylgrst Awaiting requested review from ylgrst

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kmarchais @ricardo0115
0