I can fix this, but I'd like some input from @jwesthues or @Evil-Spirit.

In order to make the end caps of Revolve and Helix groups selectable I made the following commit: 700b5d6

In that I copied some bits from Extrude. The changes in Surface.cpp are needed to make the mouse-over highlighting work and associate the shell surfaces with something - I'm not really sure how that works because the Remap uses Entity::NO_ENTITY. Insight there is welcome, but probably not needed to fix the issue.

The changes in Group.cpp and in particular in Group::MakeRevolveEndFaces creates a couple of new face entites. These are not copies of any existing face. I would have thought some kind of face would exist for the sketch and it would just get transformed along with all the others, but no. These are of a special type Entity::Type::FACE_NORMAL_PT which I borrowed from regular Extrude (that function is directly under this new one in the file.

The problem is that these new face entities are defined by a regular point and a numeric normal vector. The normal is copied from the sketch normal or the workplane normal and with this entity type the normal is not transformed at all. In an extrude group the end faces have the same normal as the sketch, but with Revolve and Helix they need to rotate.

Every time I constrain a point to an end of a Revolve or Helix, it goes to the plane of the original sketch and usually doesn't move if I manipulate the extrusion. I verified that it's probably using the origin for the point in the new face, and that isn't moving. Making it select another point allowed me to verify that the new face is attached to a new point but using the old normal - dragging the constrained point changes the angle of the revolve.

So my questions are:

1. Is there really no face entity already that would get copied?

2. Is my best approach to create a new face type say FACE_ROT_NORMAL_PT and make the same as FACE_NORMAL_PT but also rotate the normal using the parameters from the group?

3. As it is, once a point is constrained to one of the faces, that new point can be dragged but the ends of the Revolve no longer drag with the mouse directly (those points will move if a constraint is applie to them). Is this a result of this new entity not being a regular transformed copy of an existing one?

4. what is the best approach to making this work right?

There's no existing face entity. The plane of an extrusion (or lathe, etc.) is defined by the plane of the points of the curves to be extruded. That's not necessarily any existing workplane, for example if you link a sketch from a separate file and extrude that. Since the generated face entity is associated with the extrusion itself and not any previous entity--every extrusion has exactly two faces--there's no entity in the remap, thus Entity::NO_ENTITY.

I believe you want a new face type, as you say. I'm not sure I understand the issue in (3) and (4)? If a point is constrained to an extrude face, then dragging either the constrained point or other points/curves in the face both drag the extrusion depth, as expected.

OK. I'll create a new face type created from a sketch point via a rotation. If we had it all to do again I'd say it's simpler to create a face prior to extrusion and let the other copy types transform it, since that has to be possible for transformations that can apply to shells which includes translations, rotations and (soon) reflections.

How would it help to create the face prior to extrusion (and where would you create it, given that the plane of the face isn't determined until you choose what to extrude)? Translations and rotations of faces should work now with FACE_N_xxx, and reflection should be analogous.

I'm calling a face and plane the same thing. A 2D sketch always exists in a plane, so there may as well be one. Granted, the sketch face/plane can not be highligthed via mouse-over nor can it be selected. But all face types need to have transformations applied in the solver resulting in MxN methods for doing so. Fortunately we use fall-through to make a lot of the code for those the same and only appear once. I still don't like to proliferate entity types. It seems wasteful to have one face type that is created from a point-with-translation and another created from a point-with-rotation. Any subsequent copy of those faces (step rotating or translating) will copy those using a different copy type anyway. It is redundant to have the extra face types for creation-and-transformation, simpler to have creation and then transformation. I could just create an extra face and use CopyEntity() to make the transformed copies but I'm not sure what to do with the extra untransformed copy to make it non-visible. If I hadn't made Revolve allow two-sided that would be a good way to go - one face using the existing mechanism from extrude and one copy of that via rotation.

The extrude face isn't necessarily any existing workplane. For example, you could create a "Sketch in 3d" group, sketch a triangle in an arbitrary plane, and extrude that. The user benefit to that behavior may be small; but it's how everything involving a plane section works now, and it means that face can't be created before the extrusion.

I don't think the MxN problem exists? For example, a translation of a FACE_NORMAL_PT or a FACE_XPROD both become a FACE_N_TRANS. Any face that implements ::FaceGetNormalXXX() and ::FaceGetPointXXX() should get transformed without further work.

Oh, and I do agree that it would have been nicer if we used the same entity for extruded and translated faces, lathed and axis-angle rotated, etc. Maybe there was some good reason that I don't remember why those are distinct, or maybe that's just unfortunate duplication. It's linear duplication though, not MxN.

Ultimately I'm creating FACE_ROT_NORMAL_PT because FACE_NORMAL_PT only supports creating a face with a numeric normal. Or does it? The FACE_NORMAL_PT does not actually translate the point, it's referencing an already translated point via remap. I need another variant of FACE that works like that but also rotates the normal and returns expressions for a rotated normal. If there were an existing normal I could probably reference the transformed normals via remap as well.

I suspect that you want a new face type. Possibly one of the existing FACE_N_xxx could be made to do what you want, but since we have the creation/transform split now I suspect it's clearest to continue that.

Fixed in PR #586.