Unexpected behavior with hole and hull?
c-mauderer opened this issue · 14 comments
Hello,
I'm not sure whether the behavior is expected or not. But for me it was a bit of a surprise: If I do a hull on two objects with holes, the result is a hull of the object minus a hull of the holes. To make it more clear: Here is an example:
#!/usr/bin/env python3
from solid import *
from solid.utils import *
c = cylinder(r=10) - hole()(cylinder(r=5))
x = hull()(c, left(10)(c))
print(scad_render(x))
This generates one long hole:
difference(){
hull() {
difference() {
cylinder(r = 10);
}
translate(v = [-10, 0, 0]) {
difference() {
cylinder(r = 10);
}
}
}
/* Holes Below*/
hull(){
union(){
cylinder(r = 5);
}
translate(v = [-10, 0, 0]){
union(){
cylinder(r = 5);
}
}
} /* End Holes */
}
I would have expected two round holes:
difference(){
hull() {
difference() {
cylinder(r = 10);
}
translate(v = [-10, 0, 0]) {
difference() {
cylinder(r = 10);
}
}
}
/* Holes Below*/
union(){
cylinder(r = 5);
}
translate(v = [-10, 0, 0]){
union(){
cylinder(r = 5);
}
} /* End Holes */
}
Is that expected behavior?
I agree with your analysis; two circular holes is what we would expect intuitively. What's happening is that the hull() invocation is getting added to the negative holes as well as the positive areas, and that isn't what we expect.
Let me have a think on this. We want this hull() to apply to the positive cylinders and NOT to the holes. But we could very reasonably want a hole that included a hull of negative space as well.
Let me have a think on this. We want this
hull()to apply to the positive cylinders and NOT to the holes. But we could very reasonably want a hole that included a hull of negative space as well.
That's correct. For example the following slightly modified code should generate one long "bar" with two slotted holes at the end. Instead it generates only one big hole with rounded edges.
#!/usr/bin/env python3
from solid import *
from solid.utils import *
slot = hull()([back(i)(cylinder(r=2)) for i in [-2, 2]])
c = cylinder(r=10) - hole()(slot)
x = hull()(c, left(50)(c))
print(scad_render(x))
The hole() mechanism does basically what you’ve proposed, Jeff. In this particular situation, we’d need to disambiguate between: a hole() with hull() children (where the hull should persist in what’s subtracted) and a hull() with hole() grandchildren, where the hull should only apply to positive geometry.
We could check for exactly that circumstance, but it feels hacky, like I don’t have a theoretical grasp of what’s going on there. That was some of the problem with the original implementation (there’s some “I think this works” comments in the source) but I’ve never fully understood the mechanics of how the hole setup should work in all situations. :-/
Thanks for the explanations and that you have a look at all these problems. I don't know the code well enough that I can really help a lot solving the problem. I just noted that I have some more edge cases where I'm really not sure about what to expect from holes (for example using an object with a hole as a hole to some other object - there are multiple reasonable interpretations what's right in this case). Is there some documentation about how the holes should work in different situations?
PS: I solved my problem a bit different so that I don't need a workaround at the moment.
#! /usr/bin/env python
from solid import *
from solid.extensions.bosl2.std import cube, zcyl, diff
def bosl2_diff_hull():
obj = cube([100, 100, 10], _tags="pos")
hole1 = zcyl(d=10, l=100, _tags="neg").translate(10, 10, 0)
hole2 = zcyl(d=10, l=100, _tags="neg").translate(50, 50, 0)
return diff("neg", "pos")(hull()(obj + hole1 + hole2))
bosl2_diff_hull().save_as_scad()handles it the same (unexpected) way as the solidpython implementation
hull(object) - hull(hole1) - hull(hole2)
hmmmm this would create new "unexpected cases" if the holes are not convex shapes.....
Your approach seems to be the way to go....
Try to change line solid/solidpython.py:212 like this:
#from this:
if self.name in non_rendered_classes:
pass
#to this:
if self.name in non_rendered_classes or self.name == 'hull':
passThis seems to work at least for this examples:
from solid import *
c = cube([100, 100, 10])
h1 = translate([20, 20, -20])(cylinder(r=10, h=100))
h2 = translate([30, 30, -20])(cylinder(r=10, h=100))
h3 = hull()(translate([50, 50, 0])(h1 + h2))
hh = hull()(c - hole()(h1) - hole()(h2) - hole()(h3))
scad_render_to_file(hh)@etjones I'm with you concerning other transformations. I was also wondering whether there might be more. But we could extend the line I proposed above to if not self.name in translationalNodesList: pass that might do the trick.
I think there may be a little more to it: does your change differentiate between
hull(a, b - hole()(c))(where the hull should be excluded from the hole) anda - hole()(hull(b,c)), (where the hull should be included)? I don’t think the current tree traversal pays attention to whether objects are above or below a given hole object in the hierarchy.
If I understand it correctly, yeah it does.
Take a look at the example snippet a posted above. This is the result of that code, which I think is what we would expect:
Excellent. Well, that’s a bit closer to a solution.