Faster import #1139
Faster import #1139
Conversation
pca006132
force-pushed
the
faster-import
branch
from
ee384a2 to
b0a2468
Compare
|
Basically, around 10% improvement for |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1139 +/- ##
==========================================
+ Coverage 91.58% 91.71% +0.13%
==========================================
Files 30 30
Lines 5915 5951 +36
==========================================
+ Hits 5417 5458 +41
+ Misses 498 493 -5 ☔ View full report in Codecov by Sentry. |
|
Need to think about why the sequential version is not working... |
|
Some |
| size_t index; | ||
| inline bool operator<(const SortEntry& other) const { | ||
| return start == other.start ? end < other.end : start < other.start; | ||
| struct FlagStore { |
There was a problem hiding this comment.
Perhaps a comment about what this struct is used for? I don't entirely follow from reading it.
| // place all backward edges at the end | ||
| if (!other.IsForward()) return true; | ||
| if (!self.IsForward()) return false; | ||
| // dictionary order based on start and end vertices |
There was a problem hiding this comment.
Why do we sort differently for parallel and serial?
There was a problem hiding this comment.
This is now explained in the comment:
// Note that this is slightly different from the single thread version
// because we store all indices instead of just indices of forward
// halfedges. Backward halfedges are placed at the end by modifying the
// comparison function.
| halfedge_.clear(); | ||
| vertNormal_.clear(); | ||
| faceNormal_.clear(); | ||
| halfedgeTangent_.clear(); |
src/csg_tree.cpp
Outdated
| if (numPropOut > 0) { | ||
| combined.meshRelation_.numProp = numPropOut; | ||
| combined.meshRelation_.properties.resize(numPropOut * numPropVert, 0); | ||
| combined.meshRelation_.triProperties.resize(numTri); | ||
| combined.meshRelation_.triProperties.resize(numTri, ivec3(0)); |
There was a problem hiding this comment.
This was causing the test failure. I forgot what triProperties is used for and why are we adding it instead of copying them in with a per input mesh offset.
Also, the weird thing is that the bug is order dependent. I am rerunning the tests with shuffling enabled to try and see if there is something else that is similar.
There was a problem hiding this comment.
Ah, I was mistaken, this is not the issue. Halfedge tangent is the issue. I guess this may be caused by having empty halfedge tangents?
| if (!manifold::all_of(meshGL.vertProperties.begin(), | ||
| meshGL.vertProperties.end(), | ||
| [](Precision x) { return std::isfinite(x); })) { | ||
| MarkFailure(Error::NonFiniteVertex); |
Optimize #1138 a bit.
resize_nofillto explicitly avoid initializing the buffer.FlagEdgeas mentioned in Potential boolean bug #970This seems to be faster for cases where there are not too many edges to simplify. However, for Samples.Sponge4, this is slower, probably because there are too many edges to simplify. Will try to fix that.
Should also fix #1140.