Mirror about the workplane of a face.

[blooop]

I am making a function that mirrors about a face. From looking at the code this is not possible, as it only supports string arguments for the basic XY XZ etc workplanes.

cadquery/cadquery/cq.py

Line 1024 in b22436c

def mirror(self, mirrorPlane="XY", basePointVector=(0, 0, 0)):

my attempt:

obj = cq.Workplane("XY").rect(1, 2).extrude(1)

def mirrorAboutFace(obj, face):
    planeString = "XY"  # hardcoded, cannot work out how to get this via code
    planeOffset = face.objects[0].Center()
    return obj.mirror(planeString, planeOffset)


obj = mirrorAboutFace(obj, obj.faces("<Z"))  # works as expected
obj = mirrorAboutFace(obj, obj.faces(">Z"))  # works as expected

obj = mirrorAboutFace(obj, obj.faces("<X"))  # does not work because of hardcoded "XY" plane

Even if its not possible to mirror about an arbitrary workplane, is it possible to extract the string "XY" parameter from a face object?

Thanks

[adam-urbanczyk]

You could modify Shape.mirror to accept cq.Plane objects. Note that the current plane is stored in cq.Workplane.plane.

cadquery/cadquery/occ_impl/shapes.py

Line 467 in b22436c

def mirror(

[blooop]

Thanks for the help. I've implemented the feature as two new functions and left the original mirror alone while i was debugging and testing. I could make a pull request for the new functionality. Should I leave them as separate functions, or integrate them into the existing mirror function?

    def mirrorAboutPlane(self, mirrorPlaneNormal=(1, 0, 0), basePointVector=(0, 0, 0), union=False):
        """
        Mirror a single CQ object about an arbitrary plane defined by a point and a normal

        :param mirrorPlaneNormal: the normal vector of the plane to mirror about
        :type mirrorPlaneNormal: tuple
        :param basePointVector: the base point to mirror about
        :type basePointVector: tuple
        :param union: If true will perform a union operation on the mirrored object
        :type union: bool
        """
        newS = self.newObject([self.objects[0].mirrorAboutPlaneNormal(mirrorPlaneNormal, basePointVector)])
        if union:
            return self.union(newS.first())
        else:
            return newS.first()

    def mirrorAboutFace(self, faceObj, union=True):
        """
        Mirror a single CQ object about a the plane in a face

        :param faceObj: A cq workplane object that defines the mirror plane
        :type mirrorPlaneNormal: cq workplane
        :param union: If true will perform a union operation on the mirrored object
        :type union: bool
        """
        planeOffset = faceObj.objects[0].Center()
        mirrorPlaneNormal = faceObj.objects[0].normalAt(planeOffset)
        return self.mirrorAboutPlane(mirrorPlaneNormal, planeOffset, union)

[blooop]

I think mirrorAboutPlane and mirror could be merged because they have the same type of arguments
mirror(planeDefinition, basePointVec)
but I'm not sure if merging the mirrorAboutFace works as well, because it has a different type of function signature
mirror(face).
I think it would be confusing to have
mirror(plane =(1,0,0), basePointVec=(0,0,0),mirrorFace=None)
because its not entirely clear which arguments work with each other. If mirrorFace was used, then the other arguments would need to be completely ignored, so I think it should be a separate function.

Also, I think the mirrorAboutFace(union =True) makes sense as a default because the mirrored object is likely to be in contact with the original, but the generic mirrorAboutPlane(union=False) makes more sense as a default because the mirrored object is likely to be in in free space not contacting the original.

[adam-urbanczyk]

I'd prefer to put everything in mirror and use typing annotations (maybe with @overload), i.e. something like:

@overload
def mirror(self, 
    mirrorPlane: Face,
    ) -> Workplane:
...

@overload
def mirror(self, 
    mirrorPlane:union[Literal["XY",...], VectorLike, Face]="XY"
    basePointVector: VectorLike=(0, 0, 0)) -> Workplane:
    ...

This way if someone wants to extend this, they won't need to create more mirrorAbout... methods (e.g. mirrorAboutWire).

[adam-urbanczyk]

Fixed by #527