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 NumVert, NumContour, and Warp #370

Merged
merged 4 commits into from
Mar 15, 2023
Merged

Add NumVert, NumContour, and Warp #370

merged 4 commits into from
Mar 15, 2023

Conversation

geoffder
Copy link
Collaborator

@geoffder geoffder commented Mar 15, 2023

A few more methods to CrossSection to bring it another step in line with Manifold.

  • RE: Warp, do you guys think that it should go through a union to clean up any user induced shenanigans, or leave it be (similar to Manifold).

Copy link
Owner

@elalish elalish left a comment

Choose a reason for hiding this comment

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

Yes, I think a union after Warp is a good idea. I plan to eventually manage something equivalent in Manifold (#289).

EXPECT_EQ(sq.NumVert(), 4);
EXPECT_EQ(sq.NumContour(), 1);
Identical(Manifold::Extrude(a, 1.).GetMesh(),
Manifold::Extrude(b, 1.).GetMesh());
Copy link
Owner

Choose a reason for hiding this comment

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

👍

@geoffder
Copy link
Collaborator Author

Yes, I think a union after Warp is a good idea. I plan to eventually manage something equivalent in Manifold (#289).

Ok, I'll add that in then 👍

@@ -305,6 +305,24 @@ CrossSection CrossSection::Transform(const glm::mat3x2& m) const {
return transformed;
}

CrossSection CrossSection::Warp(
Copy link
Owner

Choose a reason for hiding this comment

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

Also, either now or in a follow-on PR, would you mind adding docs to these functions? The CrossSection docs are looking a little sparse.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I'll make that my next PR.

@codecov
Copy link

codecov bot commented Mar 15, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.08 🎉

Comparison is base (7c20077) 85.37% compared to head (deac432) 85.45%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #370      +/-   ##
==========================================
+ Coverage   85.37%   85.45%   +0.08%     
==========================================
  Files          36       36              
  Lines        4375     4393      +18     
==========================================
+ Hits         3735     3754      +19     
+ Misses        640      639       -1     
Impacted Files Coverage Δ
src/cross_section/src/cross_section.cpp 61.32% <100.00%> (+3.34%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@geoffder geoffder merged commit 20178cb into elalish:master Mar 15, 2023
@geoffder geoffder deleted the cross-section-methods branch March 15, 2023 06:06
cartesian-theatrics pushed a commit to SovereignShop/manifold that referenced this pull request Mar 11, 2024
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

2 participants