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

[0.20] PartDesign: Hole performance improvements #4381

Merged
merged 1 commit into from Mar 8, 2021

Conversation

davidosterberg
Copy link
Contributor

@davidosterberg davidosterberg commented Feb 6, 2021

With PR #4274, the hole feature can be very slow if thread modeling is enabled for a large hole pattern.
This PR contains some performance optimization. With debug build the performance improvement is about 30%.

  Build type Debug    
  Test case Hole pattern 6 x M10x25    
         
Commit Variant time (average of 10)   Improvement
db9525e Original Hole 3,1325 s 0,00 %
6f63b10 Compound then cut 2,22 s 29,13 %
354ab55 remove unused code 2,19 s 30,09 %

A note about changes in behavior.

Previously it was posssible for the holes to overlap each other. Now this is no-longer supported as this would be more computationally demanding. If it is seen as needed to support overlapping holes, support for overlapping holes can be implemented if necessary.

@davidosterberg davidosterberg changed the title PartDesign: Hole performance improvements [0.20] PartDesign: Hole performance improvements Feb 11, 2021
@donovaly
Copy link
Member

donovaly commented Feb 13, 2021

If PR #4274 should go in for 0.19, this PR is really necessary.

Copy link

@tomasix tomasix left a comment

Choose a reason for hiding this comment

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

Works for me

- Remove unused code
- BooleanOperation trick

For some reason doing the cut in a boolean operation is
20% faster than doing it directly. Unclear why.
@davidosterberg
Copy link
Contributor Author

@wwmayer, Any objections to this being merged?

@wwmayer
Copy link
Contributor

wwmayer commented Mar 7, 2021

Any objections to this being merged?

No, but I just haven't had the time yet to test it. I will have a look today afternoon/evening.

@wwmayer wwmayer merged commit 80af82a into FreeCAD:master Mar 8, 2021
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

4 participants