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

do something sensible when opencasde returns negative volumes #23

Merged
merged 5 commits into from
Feb 1, 2022

Conversation

smason
Copy link
Contributor

@smason smason commented Jan 31, 2022

need to calculate volume for solids so that we can subtract the overlap off the smaller volume

opencascade sometimes produces a negative value when calculating this which isn't right. James Cook suggested taking the absolute value of the volume, lets see if this does the right thing for their files

all the examples I've found so far have been large files which have made tracking down the source of this awkward. if/when this can be done this change can be reverted back to a hard error

@makeclean
Copy link

CAD is notoriously unreliable at calculating volumes correctly, the best thing to do would be to tesselate the surfaces of the volume, and use that to determine the volume.

@smason
Copy link
Contributor Author

smason commented Jan 31, 2022

@makeclean yup, you've said before, but I've been struggling to see how I'd convince OCCT to do that

I've had a another read of the docs, and think it might just a matter of using BRepMesh_IncrementalMesh to ensure the solid is tesselated, then pass UseTriangulation=True to BRepGProp::VolumeProperties to force it to use the the calculated "face triangulations" in place of "exact geometry objects"

@smason
Copy link
Contributor Author

smason commented Jan 31, 2022

think I'm finally making some useful progress with tracking down where coming from, at least some of the cases anyway

have just noticed that it seems to occur in Colin's files when some surfaces of each shape are within the tolerance distance. the result of the BOP (boolean operation) causes the shapes to slightly increase in size, due to the gap between them being incorporated into them. I've had some unit tests covering this for a while, but only using cubes, and these have always come back with positive volumes.

for example, one file has what looks like a bolt and washer close to each other, when I perform a BOP asking for the common volume between them at a tolerance of 1e-3 I get back a negative volume. repeating with higher (1e-5) or lower (1e-2) tolerances succeeds for 7 of 8 test cases, and when running at max precision (i.e. tolerance of 1e-9) I don't see negative volumes

not sure if it matters, but volume I see is sometimes consistent with calculating orig - result, which would indeed be negative if the result of the BOP is larger than the original solid. e.g. I get back a common volume of -0.0215646 while the above calculation gives me -0.0215462. however sometimes these values are quite different, presumably because of other changes that occurred to the solids in order to perform the BOP

not sure why it's taken me quite so long to notice this, seems kind of obvious thing to look for in retrospect!

@smason
Copy link
Contributor Author

smason commented Jan 31, 2022

the last commit reverts volume_of_shape to throwing an exception when it sees a negative volume, under the observation that this is likely indicates something has gone wrong and it would be good to know about it

it also introduces volume_of_shape_maybe_neg and uses this within the overlap checker to retry with stricter tolerances in the case we do see a negative volume coming back. this is based on the above observations, and might want to be used elsewhere in the codebase

this fixes the failures in step blueprint and with the few files from Colin I've tested. will run a more complete set tests, but this takes many hours on my computer!

after sleeping on my change last night, this seems like the least bad
solution. at least until this gets fixed in OCCT!

this limit of 10% is debatable, in practice I've only seen volumes of
smaller than 0.1% so should be generous.
@smason
Copy link
Contributor Author

smason commented Feb 1, 2022

after thinking about this overnight, I think it's best to work around these by treating these negative volumes as indicating the objects are touching. I've put an explanatory comment that tries to explain the reasoning, as well as also checking to make sure we don't get extreme negative values, which would indicate a more serious bug in OCCT. hopefully this is enough of a band-aid!

would be nice to get down to a small example that we could submit to the OCCT project for a larger fix, but the failing examples I have are potentially sensitive.

think I can cut them down to something minimal, but would like to check first

@makeclean
Copy link

As long as the resultant topological changes during merging result in non-overlapping geometry, then this is ok

@smason smason merged commit d9b6fec into main Feb 1, 2022
@smason smason deleted the negative-volumes branch February 1, 2022 10:59
@smason
Copy link
Contributor Author

smason commented Feb 1, 2022

As long as the resultant topological changes during merging result in non-overlapping geometry, then this is ok

this is just used for pairwise checks to see which solids are "near" to each other, and hence will need to be considered later when merging. these negative values aren't seen during the actual imprint/merge operations

if that makes sense!

@makeclean
Copy link

ah sure, yeah :)

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.

why are we getting negative volumes back from the imprint step
2 participants