Reduce vertical size, BMesh *bm = ...

All this feels to belong to two separate function, one of which handles geometry case, another one which handles visibility case. Interleaving them together is confusing.

I don't understand this. You shouldn't be checking for events in the invoke.

You should be able to

const float mouse[2] = {event->mval[0], event->mval[1]};

@Julian Eisel (Severin), is it how we handle other operators where we need them to be called from mouse events?

check_single_face_set is something what checks the state of the mesh regardless of the operation. Hence, having an argument modify_hidden is confusing and misleading:

• The function foes not modify anything
• It is not clear how this affects on the check process itself (without reading the code, which is still quite cryptic).

For good API it will be clear what function does from its name and argument list.

I do not quite follow the explanation. My interpretation:

/* Modification of base mesh geometry requires special remapping of multires displacement, which does not happen here.
 * Disable delete operation. It can be supported in the future by
 * by doing similar displacement data remapping as what happens in the mesh edit mode. */

Doing it like this is fine. Although I'd prefer it to be const (it's not in the current patch). Why make int active_face_set const but not an immutable array that's also a reference? :)