Merge branch 'main' into bgbsww-toponamingMakeElementShape

src/Mod/AddonManager/NetworkManager.py

@@ -531,7 +531,7 @@ if HAVE_QTNETWORK:
             any notifications have been called."""
             reply = self.sender()
             if not reply:
-                print("Network Manager Error: __reply_finished not called by a Qt signal")
+                # This can happen during a cancellation operation: silently do nothing
                 return
 
             if reply.error() == QtNetwork.QNetworkReply.NetworkError.OperationCanceledError:

src/Mod/AddonManager/addonmanager_git.py

@@ -322,6 +322,9 @@ class GitManager:
             if "Windows" in platform.system():
                 git_exe += ".exe"
 
+        if platform.system() == "Darwin" and not self._xcode_command_line_tools_are_installed():
+            return
+
         if not git_exe or not os.path.exists(git_exe):
             git_exe = shutil.which("git")
 
@@ -331,6 +334,18 @@ class GitManager:
         prefs.SetString("GitExecutable", git_exe)
         self.git_exe = git_exe
 
+    def _xcode_command_line_tools_are_installed(self) -> bool:
+        """On Macs, there is *always* an executable called "git", but sometimes it's just a
+        script that tells the user to install XCode's Command Line tools. So the existence of git
+        on the Mac actually requires us to check for that installation."""
+        try:
+            subprocess.check_output(["xcode-select", "-p"])
+            fci.Console.PrintMessage("XCode command line tools are installed: git is available\n")
+            return True
+        except subprocess.CalledProcessError:
+            fci.Console.PrintMessage("XCode command line tools are not installed: not using git\n")
+            return False
+
     def _synchronous_call_git(self, args: List[str]) -> str:
         """Calls git and returns its output."""
         final_args = [self.git_exe]

src/Mod/AddonManager/addonmanager_installer.py

@@ -24,7 +24,7 @@
 """ Contains the classes to manage Addon installation: intended as a stable API, safe for external
 code to call and to rely upon existing. See classes AddonInstaller and MacroInstaller for details.
 """
-
+import json
 from datetime import datetime, timezone
 from enum import IntEnum, auto
 import os
@@ -503,16 +503,34 @@ class MacroInstaller(QtCore.QObject):
                 self.finished.emit()
                 return False
 
-            # If it succeeded, move all of the files to the macro install location
+            # If it succeeded, move all the files to the macro install location,
+            # keeping a list of all the files we installed, so they can be removed later
+            # if this macro is uninstalled.
+            manifest = []
             for item in os.listdir(temp_dir):
                 src = os.path.join(temp_dir, item)
                 dst = os.path.join(self.installation_path, item)
                 shutil.move(src, dst)
+                manifest.append(dst)
+            self._write_installation_manifest(manifest)
         self.success.emit(self.addon_to_install)
         self.addon_to_install.set_status(Addon.Status.NO_UPDATE_AVAILABLE)
         self.finished.emit()
         return True
 
+    def _write_installation_manifest(self, manifest):
+        manifest_file = os.path.join(
+            self.installation_path, self.addon_to_install.macro.filename + ".manifest"
+        )
+        try:
+            with open(manifest_file, "w", encoding="utf-8") as f:
+                f.write(json.dumps(manifest, indent="  "))
+        except OSError as e:
+            FreeCAD.Console.PrintWarning(
+                translate("AddonsInstaller", "Failed to create installation manifest " "file:\n")
+            )
+            FreeCAD.Console.PrintWarning(manifest_file)
+
     @classmethod
     def _validate_object(cls, addon: object):
         """Make sure this object provides an attribute called "macro" with a method called

src/Mod/AddonManager/addonmanager_macro.py

@@ -69,6 +69,7 @@ class Macro:
         self.version = ""
         self.date = ""
         self.src_filename = ""
+        self.filename_from_url = ""
         self.author = ""
         self.icon = ""
         self.icon_source = None
@@ -104,6 +105,8 @@ class Macro:
         """The filename of this macro"""
         if self.on_git:
             return os.path.basename(self.src_filename)
+        elif self.filename_from_url:
+            return self.filename_from_url
         return (self.name + ".FCMacro").replace(" ", "_")
 
     def is_installed(self):
@@ -211,8 +214,14 @@ class Macro:
                 )
                 return None
             code = u2.decode("utf8")
+            self._set_filename_from_url(self.raw_code_url)
         return code
 
+    def _set_filename_from_url(self, url: str):
+        lhs, slash, rhs = url.rpartition("/")
+        if rhs.endswith(".py") or rhs.lower().endswith(".fcmacro"):
+            self.filename_from_url = rhs
+
     @staticmethod
     def _read_code_from_wiki(p: str) -> Optional[str]:
         code = re.findall(r"<pre>(.*?)</pre>", p.replace("\n", "--endl--"))

src/Mod/AddonManager/addonmanager_readme_viewer.py

@@ -25,6 +25,8 @@
 
 import Addon
 from PySide import QtCore, QtGui, QtWidgets
+from enum import Enum, auto
+from html.parser import HTMLParser
 
 import addonmanager_freecad_interface as fci
 import addonmanager_utilities as utils
@@ -43,18 +45,21 @@ class ReadmeViewer(QtWidgets.QTextBrowser):
         super().__init__(parent)
         NetworkManager.InitializeNetworkManager()
         NetworkManager.AM_NETWORK_MANAGER.completed.connect(self._download_completed)
-        self.request_index = 0
+        self.readme_request_index = 0
+        self.resource_requests = {}
         self.url = ""
         self.repo: Addon.Addon = None
         self.setOpenExternalLinks(True)
         self.setOpenLinks(True)
         self.image_map = {}
+        self.stop = True
 
     def set_addon(self, repo: Addon):
         """Set which Addon's information is displayed"""
 
         self.setPlainText(translate("AddonsInstaller", "Loading README data..."))
         self.repo = repo
+        self.stop = False
         if self.repo.repo_type == Addon.Addon.Kind.MACRO:
             self.url = self.repo.macro.wiki
             if not self.url:
@@ -62,11 +67,13 @@ class ReadmeViewer(QtWidgets.QTextBrowser):
         else:
             self.url = utils.get_readme_url(repo)
 
-        self.request_index = NetworkManager.AM_NETWORK_MANAGER.submit_unmonitored_get(self.url)
+        self.readme_request_index = NetworkManager.AM_NETWORK_MANAGER.submit_unmonitored_get(
+            self.url
+        )
 
     def _download_completed(self, index: int, code: int, data: QtCore.QByteArray) -> None:
         """Callback for handling a completed README file download."""
-        if index == self.request_index:
+        if index == self.readme_request_index:
             if code == 200:  # HTTP success
                 self._process_package_download(data.data().decode("utf-8"))
             else:
@@ -76,34 +83,56 @@ class ReadmeViewer(QtWidgets.QTextBrowser):
                         "Failed to download data from {} -- received response code {}.",
                     ).format(self.url, code)
                 )
+        elif index in self.resource_requests:
+            if code == 200:
+                self._process_resource_download(self.resource_requests[index], data.data())
+            else:
+                self.image_map[self.resource_requests[index]] = None
+            del self.resource_requests[index]
+            if not self.resource_requests:
+                self.set_addon(self.repo)  # Trigger a reload of the page now with resources
 
     def _process_package_download(self, data: str):
         if self.repo.repo_type == Addon.Addon.Kind.MACRO:
-            self.setHtml(data)
+            parser = WikiCleaner()
+            parser.feed(data)
+            self.setHtml(parser.final_html)
         else:
+            # Check for recent Qt (e.g. Qt5.15 or later). Check can be removed when
+            # we no longer support Ubuntu 20.04LTS for compiling.
             if hasattr(self, "setMarkdown"):
                 self.setMarkdown(data)
             else:
                 self.setPlainText(data)
 
+    def _process_resource_download(self, resource_name: str, resource_data: bytes):
+        image = QtGui.QImage.fromData(resource_data)
+        if image:
+            self.image_map[resource_name] = self._ensure_appropriate_width(image)
+        else:
+            self.image_map[resource_name] = None
+
     def loadResource(self, resource_type: int, name: QtCore.QUrl) -> object:
         """Callback for resource loading. Called automatically by underlying Qt
         code when external resources are needed for rendering. In particular,
         here it is used to download and cache (in RAM) the images needed for the
         README and Wiki pages."""
-        if resource_type == QtGui.QTextDocument.ImageResource:
+        if resource_type == QtGui.QTextDocument.ImageResource and not self.stop:
             full_url = self._create_full_url(name.toString())
             if full_url not in self.image_map:
                 self.image_map[full_url] = None
                 fci.Console.PrintMessage(f"Downloading image from {full_url}...\n")
-                data = NetworkManager.AM_NETWORK_MANAGER.blocking_get(full_url)
-                if data and data.data():
-                    image = QtGui.QImage.fromData(data.data())
-                    if image:
-                        self.image_map[full_url] = self._ensure_appropriate_width(image)
+                index = NetworkManager.AM_NETWORK_MANAGER.submit_unmonitored_get(full_url)
+                self.resource_requests[index] = full_url
             return self.image_map[full_url]
         return super().loadResource(resource_type, name)
 
+    def hideEvent(self, event: QtGui.QHideEvent):
+        self.stop = True
+        for request in self.resource_requests:
+            NetworkManager.AM_NETWORK_MANAGER.abort(request)
+        self.resource_requests.clear()
+
     def _create_full_url(self, url: str) -> str:
         if url.startswith("http"):
             return url
@@ -117,3 +146,113 @@ class ReadmeViewer(QtWidgets.QTextBrowser):
         if image.width() < ninety_seven_percent:
             return image
         return image.scaledToWidth(ninety_seven_percent)
+
+
+class WikiCleaner(HTMLParser):
+    """This HTML parser cleans up FreeCAD Macro Wiki Page for display in a
+    QTextBrowser widget (which does not deal will with tables used as formatting,
+    etc.) It strips out any tables, and extracts the mw-parser-output div as the only
+    thing that actually gets displayed. It also discards anything inside the [edit]
+    spans that litter wiki output."""
+
+    class State(Enum):
+        BeforeMacroContent = auto()
+        InMacroContent = auto()
+        InTable = auto()
+        InEditSpan = auto()
+        AfterMacroContent = auto()
+
+    def __init__(self):
+        super().__init__()
+        self.depth_in_div = 0
+        self.depth_in_span = 0
+        self.depth_in_table = 0
+        self.final_html = "<html><body>"
+        self.previous_state = WikiCleaner.State.BeforeMacroContent
+        self.state = WikiCleaner.State.BeforeMacroContent
+
+    def handle_starttag(self, tag: str, attrs):
+        if tag == "div":
+            self.handle_div_start(attrs)
+        elif tag == "span":
+            self.handle_span_start(attrs)
+        elif tag == "table":
+            self.handle_table_start(attrs)
+        else:
+            if self.state == WikiCleaner.State.InMacroContent:
+                self.add_tag_to_html(tag, attrs)
+
+    def handle_div_start(self, attrs):
+        for name, value in attrs:
+            if name == "class" and value == "mw-parser-output":
+                self.previous_state = self.state
+                self.state = WikiCleaner.State.InMacroContent
+        if self.state == WikiCleaner.State.InMacroContent:
+            self.depth_in_div += 1
+            self.add_tag_to_html("div", attrs)
+
+    def handle_span_start(self, attrs):
+        for name, value in attrs:
+            if name == "class" and value == "mw-editsection":
+                self.previous_state = self.state
+                self.state = WikiCleaner.State.InEditSpan
+                break
+        if self.state == WikiCleaner.State.InEditSpan:
+            self.depth_in_span += 1
+        elif WikiCleaner.State.InMacroContent:
+            self.add_tag_to_html("span", attrs)
+
+    def handle_table_start(self, unused):
+        if self.state != WikiCleaner.State.InTable:
+            self.previous_state = self.state
+            self.state = WikiCleaner.State.InTable
+        self.depth_in_table += 1
+
+    def add_tag_to_html(self, tag, attrs=None):
+        self.final_html += f"<{tag}"
+        if attrs:
+            self.final_html += " "
+            for attr, value in attrs:
+                self.final_html += f"{attr}='{value}'"
+        self.final_html += ">\n"
+
+    def handle_endtag(self, tag):
+        if tag == "table":
+            self.handle_table_end()
+        elif tag == "span":
+            self.handle_span_end()
+        elif tag == "div":
+            self.handle_div_end()
+        else:
+            if self.state == WikiCleaner.State.InMacroContent:
+                self.add_tag_to_html(f"/{tag}")
+
+    def handle_span_end(self):
+        if self.state == WikiCleaner.State.InEditSpan:
+            self.depth_in_span -= 1
+            if self.depth_in_span <= 0:
+                self.depth_in_span = 0
+                self.state = self.previous_state
+        else:
+            self.add_tag_to_html(f"/span")
+
+    def handle_div_end(self):
+        if self.state == WikiCleaner.State.InMacroContent:
+            self.depth_in_div -= 1
+            if self.depth_in_div <= 0:
+                self.depth_in_div = 0
+                self.state = WikiCleaner.State.AfterMacroContent
+                self.final_html += "</body></html>"
+        else:
+            self.add_tag_to_html(f"/div")
+
+    def handle_table_end(self):
+        if self.state == WikiCleaner.State.InTable:
+            self.depth_in_table -= 1
+            if self.depth_in_table <= 0:
+                self.depth_in_table = 0
+                self.state = self.previous_state
+
+    def handle_data(self, data):
+        if self.state == WikiCleaner.State.InMacroContent:
+            self.final_html += data

src/Mod/AddonManager/addonmanager_uninstaller.py

@@ -24,7 +24,7 @@
 """ Contains the classes to manage Addon removal: intended as a stable API, safe for
 external code to call and to rely upon existing. See classes AddonUninstaller and
 MacroUninstaller for details."""
-
+import json
 import os
 from typing import List
 
@@ -228,7 +228,10 @@ class MacroUninstaller(QObject):
         directories = set()
         for f in self._get_files_to_remove():
             normed = os.path.normpath(f)
-            full_path = os.path.join(self.installation_location, normed)
+            if os.path.isabs(normed):
+                full_path = normed
+            else:
+                full_path = os.path.join(self.installation_location, normed)
             if "/" in f:
                 directories.add(os.path.dirname(full_path))
             try:
@@ -246,6 +249,10 @@ class MacroUninstaller(QObject):
                     + str(e)
                 )
                 success = False
+            except Exception:
+                # Generic catch-all, just in case (because failure to catch an exception
+                # here can break things pretty badly)
+                success = False
 
         self._cleanup_directories(directories)
 
@@ -256,8 +263,17 @@ class MacroUninstaller(QObject):
         self.addon_to_remove.set_status(Addon.Status.NOT_INSTALLED)
         self.finished.emit()
 
-    def _get_files_to_remove(self) -> List[os.PathLike]:
+    def _get_files_to_remove(self) -> List[str]:
         """Get the list of files that should be removed"""
+        manifest_file = os.path.join(
+            self.installation_location, self.addon_to_remove.macro.filename + ".manifest"
+        )
+        if os.path.exists(manifest_file):
+            with open(manifest_file, "r", encoding="utf-8") as f:
+                manifest_data = f.read()
+                manifest = json.loads(manifest_data)
+                manifest.append(manifest_file)  # Remove the manifest itself as well
+                return manifest
         files_to_remove = [self.addon_to_remove.macro.filename]
         if self.addon_to_remove.macro.icon:
             files_to_remove.append(self.addon_to_remove.macro.icon)

src/Mod/AddonManager/package_details.py

@@ -49,8 +49,6 @@ except ImportError:
 
 translate = fci.translate
 
-show_javascript_console_output = False
-
 
 class PackageDetails(QtWidgets.QWidget):
     """The PackageDetails QWidget shows package README information and provides
@@ -90,7 +88,7 @@ class PackageDetails(QtWidgets.QWidget):
 
         # If this is the same repo we were already showing, we do not have to do the
         # expensive refetch unless reload is true
-        if self.repo != repo or reload:
+        if True or self.repo != repo or reload:
             self.repo = repo
 
             if self.worker is not None:

src/Mod/Arch/ArchBuildingPart.py

@@ -883,8 +883,12 @@ class ViewProviderBuildingPart:
         return True
 
     def setEdit(self, vobj, mode):
-        # For some reason mode is always 0.
-        # Using FreeCADGui.getUserEditMode() as a workaround.
+        # mode == 1 if Transform is selected in the Tree view contex menu.
+        # mode == 2 has been added for consistency.
+        if mode == 1 or mode == 2:
+            return None
+        # For some reason mode is always 0 if the object is double-clicked in
+        # the Tree view. Using FreeCADGui.getUserEditMode() as a workaround.
         if FreeCADGui.getUserEditMode() in ("Transform", "Cutting"):
             return None
 
@@ -892,8 +896,8 @@ class ViewProviderBuildingPart:
         return False # Return `False` as we don't want to enter edit mode.
 
     def unsetEdit(self, vobj, mode):
-        # For some reason mode is always 0.
-        # Using FreeCADGui.getUserEditMode() as a workaround.
+        if mode == 1 or mode == 2:
+            return None
         if FreeCADGui.getUserEditMode() in ("Transform", "Cutting"):
             return None
 

src/Mod/Draft/drafttaskpanels/task_scale.py

@@ -46,7 +46,7 @@ class ScaleTaskPanel:
     """The task panel for the Draft Scale tool."""
 
     def __init__(self):
-        decimals = params.get_param("Decimals", path="Units")
+        decimals = max(6, params.get_param("Decimals", path="Units"))
         self.sourceCmd = None
         self.form = QtGui.QWidget()
         self.form.setWindowIcon(QtGui.QIcon(":/icons/Draft_Scale.svg"))

src/Mod/Draft/draftutils/init_draft_statusbar.py

@@ -78,12 +78,12 @@ def get_scales(unit_system = 0):
 
     if unit_system == 0:
         scale_units_system = params.get_param("UserSchema", path="Units")
-        if scale_units_system in [0, 1, 4, 6]:
-            return draft_scales_metrics
-        elif scale_units_system in [2, 3, 5]:
+        if scale_units_system in [2, 3, 5]:
             return draft_scales_arch_imperial
         elif scale_units_system in [7]:
             return draft_scales_eng_imperial
+        else:
+            return draft_scales_metrics
     elif unit_system == 1:
         return draft_scales_metrics
     elif unit_system == 2:

src/Mod/Fem/App/FemConstraintContact.cpp

@@ -33,9 +33,31 @@ PROPERTY_SOURCE(Fem::ConstraintContact, Fem::Constraint)
 ConstraintContact::ConstraintContact()
 {
     /*Note: Initialise parameters here*/
-    ADD_PROPERTY(Slope, (0.0));
-    ADD_PROPERTY(Friction, (0.0));
-    /* */
+    ADD_PROPERTY_TYPE(Slope,
+                      (0.0),
+                      "ConstraintContact",
+                      App::PropertyType(App::Prop_None),
+                      "Contact stiffness");
+    ADD_PROPERTY_TYPE(Adjust,
+                      (0.0),
+                      "ConstraintContact",
+                      App::PropertyType(App::Prop_None),
+                      "Node clearance adjustment limit");
+    ADD_PROPERTY_TYPE(Friction,
+                      (false),
+                      "ConstraintContact",
+                      App::PropertyType(App::Prop_None),
+                      "Enable friction interaction");
+    ADD_PROPERTY_TYPE(FrictionCoefficient,
+                      (0.0),
+                      "ConstraintContact",
+                      App::PropertyType(App::Prop_None),
+                      "Friction coefficient");
+    ADD_PROPERTY_TYPE(StickSlope,
+                      (0.0),
+                      "ConstraintContact",
+                      App::PropertyType(App::Prop_None),
+                      "Stick slope");
 
     ADD_PROPERTY_TYPE(Points,
                       (Base::Vector3d()),
@@ -47,6 +69,7 @@ ConstraintContact::ConstraintContact()
                       "ConstraintContact",
                       App::PropertyType(App::Prop_ReadOnly | App::Prop_Output),
                       "Normals where symbols are drawn");
+    /* */
     Points.setValues(std::vector<Base::Vector3d>());
     Normals.setValues(std::vector<Base::Vector3d>());
 }
@@ -77,3 +100,28 @@ void ConstraintContact::onChanged(const App::Property* prop)
         }
     }
 }
+
+void ConstraintContact::handleChangedPropertyType(Base::XMLReader& reader,
+                                                  const char* typeName,
+                                                  App::Property* prop)
+{
+    if (prop == &Slope && strcmp(typeName, "App::PropertyFloat") == 0) {
+        App::PropertyFloat oldSlope;
+        oldSlope.Restore(reader);
+        // old slope value stored as MPa/mm equivalent to 1e3 kg/(mm^2*s^2)
+        Slope.setValue(oldSlope.getValue() * 1000);
+
+        // stick slope internally generated as slope/10
+        StickSlope.setValue(Slope.getValue() / 10);
+    }
+    else if (prop == &Friction && strcmp(typeName, "App::PropertyFloat") == 0) {
+        App::PropertyFloat oldFriction;
+        oldFriction.Restore(reader);
+        FrictionCoefficient.setValue(oldFriction.getValue());
+
+        Friction.setValue(oldFriction.getValue() > 0 ? true : false);
+    }
+    else {
+        Constraint::handleChangedPropertyType(reader, typeName, prop);
+    }
+}

src/Mod/Fem/App/FemConstraintContact.h

@@ -50,8 +50,11 @@ public:
      * This is only the definitions of the variables
      ******/
     // ex.
-    App::PropertyFloat Slope;
-    App::PropertyFloat Friction;
+    App::PropertyStiffnessDensity Slope;
+    App::PropertyLength Adjust;
+    App::PropertyBool Friction;
+    App::PropertyFloat FrictionCoefficient;
+    App::PropertyStiffnessDensity StickSlope;
 
     // etc
     /* */
@@ -64,6 +67,9 @@ public:
 
 protected:
     void onChanged(const App::Property* prop) override;
+    void handleChangedPropertyType(Base::XMLReader& reader,
+                                   const char* typeName,
+                                   App::Property* prop) override;
 };
 
 }  // namespace Fem

src/Mod/Fem/Gui/Command.cpp

@@ -250,10 +250,19 @@ void CmdFemConstraintContact::activated(int)
               "App.activeDocument().addObject(\"Fem::ConstraintContact\",\"%s\")",
               FeatName.c_str());
     doCommand(Doc,
-              "App.activeDocument().%s.Slope = 1000000.00",
+              "App.activeDocument().%s.Slope = \"1e6 GPa/m\"",
               FeatName.c_str());  // OvG: set default not equal to 0
     doCommand(Doc,
-              "App.activeDocument().%s.Friction = 0.0",
+              "App.activeDocument().%s.Adjust = 0.0",
+              FeatName.c_str());  // OvG: set default equal to 0
+    doCommand(Doc,
+              "App.activeDocument().%s.Friction = False",
+              FeatName.c_str());  // OvG: set default equal to 0
+    doCommand(Doc,
+              "App.activeDocument().%s.FrictionCoefficient = 0.0",
+              FeatName.c_str());  // OvG: set default equal to 0
+    doCommand(Doc,
+              "App.activeDocument().%s.StickSlope = \"1e4 GPa/m\"",
               FeatName.c_str());  // OvG: set default not equal to 0
     doCommand(Doc,
               "App.activeDocument().%s.Scale = 1",

src/Mod/Fem/Gui/Resources/ui/ConstraintTie.ui

@@ -28,31 +28,13 @@
      <layout class="QVBoxLayout" name="verticalLayout_5">
       <item>
        <layout class="QFormLayout" name="formLayout_1">
-        <property name="fieldGrowthPolicy">
-         <enum>QFormLayout::AllNonFixedFieldsGrow</enum>
-        </property>
         <item row="1" column="1">
-         <widget class="Gui::InputField" name="if_tolerance">
-          <property name="sizePolicy">
-           <sizepolicy hsizetype="Fixed" vsizetype="Fixed">
-            <horstretch>0</horstretch>
-            <verstretch>0</verstretch>
-           </sizepolicy>
+         <widget class="Gui::QuantitySpinBox" name="spb_tolerance">
+          <property name="unit" stdset="0">
+           <string notr="true">mm</string>
           </property>
-          <property name="minimumSize">
-           <size>
-            <width>80</width>
-            <height>20</height>
-           </size>
-          </property>
-          <property name="layoutDirection">
-           <enum>Qt::LeftToRight</enum>
-          </property>
-          <property name="text">
-           <string>0 mm</string>
-          </property>
-          <property name="alignment">
-           <set>Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter</set>
+          <property name="value" stdset="0">
+           <double>0.000000000000000</double>
           </property>
           <property name="singleStep">
            <double>1.000000000000000</double>
@@ -60,9 +42,6 @@
           <property name="maximum">
            <double>1000000000.000000000000000</double>
           </property>
-          <property name="unit" stdset="0">
-           <string notr="true">mm</string>
-          </property>
           <property name="decimals" stdset="0">
            <number>2</number>
           </property>
@@ -74,7 +53,14 @@
         <item row="1" column="0">
          <widget class="QLabel" name="l_tolerance">
           <property name="text">
-           <string>Tolerance:     </string>
+           <string>Tolerance</string>
+          </property>
+         </widget>
+        </item>
+        <item row="2" column="0">
+         <widget class="QCheckBox" name="ckb_adjust">
+          <property name="text">
+           <string>Enable Adjust</string>
           </property>
          </widget>
         </item>
@@ -87,9 +73,9 @@
  </widget>
  <customwidgets>
   <customwidget>
-   <class>Gui::InputField</class>
-   <extends>QLineEdit</extends>
-   <header>Gui/InputField.h</header>
+   <class>Gui::QuantitySpinBox</class>
+   <extends>QWidget</extends>
+   <header>Gui/QuantitySpinBox.h</header>
   </customwidget>
  </customwidgets>
  <resources/>

src/Mod/Fem/Gui/TaskFemConstraintContact.cpp

@@ -91,14 +91,36 @@ TaskFemConstraintContact::TaskFemConstraintContact(ViewProviderFemConstraintCont
 
     std::vector<App::DocumentObject*> Objects = pcConstraint->References.getValues();
     std::vector<std::string> SubElements = pcConstraint->References.getSubValues();
-    double slope = pcConstraint->Slope.getValue();
-    double friction = pcConstraint->Friction.getValue();
+
+    bool friction = pcConstraint->Friction.getValue();
 
     // Fill data into dialog elements
-    ui->spSlope->setMinimum(1.0);
-    ui->spSlope->setValue(slope);
-    ui->spFriction->setValue(friction);
+    ui->spbSlope->setUnit(pcConstraint->Slope.getUnit());
+    ui->spbSlope->setMinimum(0);
+    ui->spbSlope->setMaximum(FLOAT_MAX);
+    ui->spbSlope->setValue(pcConstraint->Slope.getQuantityValue());
+    ui->spbSlope->bind(pcConstraint->Slope);
 
+    ui->spbAdjust->setUnit(pcConstraint->Adjust.getUnit());
+    ui->spbAdjust->setMinimum(0);
+    ui->spbAdjust->setMaximum(FLOAT_MAX);
+    ui->spbAdjust->setValue(pcConstraint->Adjust.getQuantityValue());
+    ui->spbAdjust->bind(pcConstraint->Adjust);
+
+    ui->ckbFriction->setChecked(friction);
+
+    ui->spbFrictionCoeff->setMinimum(0);
+    ui->spbFrictionCoeff->setMaximum(FLOAT_MAX);
+    ui->spbFrictionCoeff->setValue(pcConstraint->FrictionCoefficient.getValue());
+    ui->spbFrictionCoeff->setEnabled(friction);
+    ui->spbFrictionCoeff->bind(pcConstraint->FrictionCoefficient);
+
+    ui->spbStickSlope->setUnit(pcConstraint->StickSlope.getUnit());
+    ui->spbStickSlope->setMinimum(0);
+    ui->spbStickSlope->setMaximum(FLOAT_MAX);
+    ui->spbStickSlope->setValue(pcConstraint->StickSlope.getQuantityValue());
+    ui->spbStickSlope->setEnabled(friction);
+    ui->spbStickSlope->bind(pcConstraint->StickSlope);
     /* */
 
     ui->lw_referencesMaster->clear();
@@ -136,6 +158,11 @@ TaskFemConstraintContact::TaskFemConstraintContact(ViewProviderFemConstraintCont
             this,
             &TaskFemConstraintContact::removeFromSelectionMaster);
 
+    connect(ui->ckbFriction,
+            &QCheckBox::toggled,
+            this,
+            &TaskFemConstraintContact::onFrictionChanged);
+
     updateUI();
 }
 
@@ -428,6 +455,12 @@ void TaskFemConstraintContact::onReferenceDeletedMaster()
     TaskFemConstraintContact::removeFromSelectionMaster();
 }
 
+void TaskFemConstraintContact::onFrictionChanged(bool state)
+{
+    ui->spbFrictionCoeff->setEnabled(state);
+    ui->spbStickSlope->setEnabled(state);
+}
+
 const std::string TaskFemConstraintContact::getReferences() const
 {
     int rowsSlave = ui->lw_referencesSlave->model()->rowCount();
@@ -443,15 +476,29 @@ const std::string TaskFemConstraintContact::getReferences() const
     return TaskFemConstraint::getReferences(items);
 }
 
-/* Note: */
-double TaskFemConstraintContact::get_Slope() const
+const std::string TaskFemConstraintContact::getSlope() const
 {
-    return ui->spSlope->rawValue();
+    return ui->spbSlope->value().getSafeUserString().toStdString();
 }
 
-double TaskFemConstraintContact::get_Friction() const
+const std::string TaskFemConstraintContact::getAdjust() const
 {
-    return ui->spFriction->value();
+    return ui->spbAdjust->value().getSafeUserString().toStdString();
+}
+
+bool TaskFemConstraintContact::getFriction() const
+{
+    return ui->ckbFriction->isChecked();
+}
+
+double TaskFemConstraintContact::getFrictionCoeff() const
+{
+    return ui->spbFrictionCoeff->value();
+}
+
+const std::string TaskFemConstraintContact::getStickSlope() const
+{
+    return ui->spbStickSlope->value().getSafeUserString().toStdString();
 }
 
 void TaskFemConstraintContact::changeEvent(QEvent*)
@@ -478,7 +525,7 @@ void TaskDlgFemConstraintContact::open()
     // a transaction is already open at creation time of the panel
     if (!Gui::Command::hasPendingCommand()) {
         QString msg = QObject::tr("Contact constraint");
-        Gui::Command::openCommand((const char*)msg.toUtf8());
+        Gui::Command::openCommand(static_cast<const char*>(msg.toUtf8()));
         ConstraintView->setVisible(true);
         Gui::Command::runCommand(
             Gui::Command::Doc,
@@ -497,13 +544,25 @@ bool TaskDlgFemConstraintContact::accept()
 
     try {
         Gui::Command::doCommand(Gui::Command::Doc,
-                                "App.ActiveDocument.%s.Slope = %f",
+                                "App.ActiveDocument.%s.Slope = \"%s\"",
                                 name.c_str(),
-                                parameterContact->get_Slope());
+                                parameterContact->getSlope().c_str());
         Gui::Command::doCommand(Gui::Command::Doc,
-                                "App.ActiveDocument.%s.Friction = %f",
+                                "App.ActiveDocument.%s.Adjust = \"%s\"",
                                 name.c_str(),
-                                parameterContact->get_Friction());
+                                parameterContact->getAdjust().c_str());
+        Gui::Command::doCommand(Gui::Command::Doc,
+                                "App.ActiveDocument.%s.Friction = %s",
+                                name.c_str(),
+                                parameterContact->getFriction() ? "True" : "False");
+        Gui::Command::doCommand(Gui::Command::Doc,
+                                "App.ActiveDocument.%s.FrictionCoefficient = %f",
+                                name.c_str(),
+                                parameterContact->getFrictionCoeff());
+        Gui::Command::doCommand(Gui::Command::Doc,
+                                "App.ActiveDocument.%s.StickSlope = \"%s\"",
+                                name.c_str(),
+                                parameterContact->getStickSlope().c_str());
         std::string scale = parameterContact->getScale();  // OvG: determine modified scale
         Gui::Command::doCommand(Gui::Command::Doc,
                                 "App.ActiveDocument.%s.Scale = %s",

src/Mod/Fem/Gui/TaskFemConstraintContact.h

@@ -46,8 +46,11 @@ public:
                                       QWidget* parent = nullptr);
     ~TaskFemConstraintContact() override;
     const std::string getReferences() const override;
-    double get_Slope() const;
-    double get_Friction() const;
+    const std::string getAdjust() const;
+    const std::string getSlope() const;
+    bool getFriction() const;
+    const std::string getStickSlope() const;
+    double getFrictionCoeff() const;
 
 private Q_SLOTS:
     void onReferenceDeletedSlave();
@@ -56,12 +59,12 @@ private Q_SLOTS:
     void removeFromSelectionSlave();
     void addToSelectionMaster();
     void removeFromSelectionMaster();
+    void onFrictionChanged(bool);
 
 protected:
     void changeEvent(QEvent* e) override;
 
 private:
-    // void onSelectionChanged(const Gui::SelectionChanges& msg);
     void updateUI();
     std::unique_ptr<Ui_TaskFemConstraintContact> ui;
 };

src/Mod/Fem/Gui/TaskFemConstraintContact.ui

@@ -135,56 +135,123 @@
      </property>
     </widget>
    </item>
+
    <item>
-    <layout class="QHBoxLayout" name="horizontalLayout_3">
-     <item>
-      <widget class="QLabel" name="label">
-       <property name="text">
-        <string>Contact Stiffness</string>
-       </property>
-      </widget>
-     </item>
-     <item>
-      <widget class="Gui::InputField" name="spSlope">
-       <property name="singleStep">
-        <double>1.000000000000000</double>
-       </property>
-       <property name="maximum">
-        <double>1000000000.000000000000000</double>
-       </property>
-       <property name="unit" stdset="0">
-        <string notr="true">Pa</string>
-       </property>
-       <property name="value" stdset="0">
-        <double>1000000.000000000000000</double>
-       </property>
-      </widget>
-     </item>
-    </layout>
-   </item>
-   <item>
-    <layout class="QHBoxLayout" name="horizontalLayout_2">
-     <item>
-      <widget class="QLabel" name="label_2">
-       <property name="text">
-        <string>Friction coefficient</string>
-       </property>
-      </widget>
-     </item>
-     <item>
-      <widget class="QDoubleSpinBox" name="spFriction">
-       <property name="decimals">
-        <number>2</number>
-       </property>
-       <property name="maximum">
-        <double>1.000000000000000</double>
-       </property>
-       <property name="singleStep">
-        <double>0.100000000000000</double>
-       </property>
-      </widget>
-     </item>
-    </layout>
+    <widget class="QGroupBox" name="gpbParameter">
+     <property name="sizePolicy">
+      <sizepolicy hsizetype="Expanding" vsizetype="Preferred">
+       <horstretch>0</horstretch>
+       <verstretch>0</verstretch>
+      </sizepolicy>
+     </property>
+     <property name="toolTip">
+      <string/>
+     </property>
+     <property name="title">
+      <string>Parameters</string>
+     </property>
+     <layout class="QFormLayout" name="fltParameters">
+      <item row="0" column="0">
+       <widget class="QLabel" name="lblSlope">
+        <property name="text">
+         <string>Contact Stiffness</string>
+        </property>
+       </widget>
+      </item>
+      <item row="0" column="1">
+       <widget class="Gui::QuantitySpinBox" name="spbSlope">
+        <property name="singleStep">
+         <double>1.000000000000000</double>
+        </property>
+        <property name="maximum">
+         <double>1000000000.000000000000000</double>
+        </property>
+        <property name="unit" stdset="0">
+         <string notr="true">GPa/m</string>
+        </property>
+        <property name="value" stdset="0">
+         <double>100.000000000000000</double>
+        </property>
+       </widget>
+      </item>
+      <item row="1" column="0">
+       <widget class="QLabel" name="lblAdjust">
+        <property name="text">
+         <string>Clearance Adjustment</string>
+        </property>
+       </widget>
+      </item>
+      <item row="1" column="1">
+       <widget class="Gui::QuantitySpinBox" name="spbAdjust">
+        <property name="minimum">
+         <double>0.000000000000000</double>
+        </property>
+        <property name="unit" stdset="0">
+         <string notr="true">mm</string>
+        </property>
+        <property name="value" stdset="0">
+         <double>0.000000000000000</double>
+        </property>
+        <property name="singleStep">
+         <double>0.100000000000000</double>
+        </property>
+       </widget>
+      </item>
+      <item row="2" column="0">
+       <widget class="QCheckBox" name="ckbFriction">
+        <property name="text">
+         <string>Enable Friction</string>
+        </property>
+       </widget>
+      </item>
+      <item row="3" column="0">
+       <widget class="QLabel" name="lblFrictionCoeff">
+        <property name="text">
+         <string>Friction Coefficient</string>
+        </property>
+       </widget>
+      </item>
+      <item row="3" column="1">
+       <widget class="Gui::DoubleSpinBox" name="spbFrictionCoeff">
+        <property name="decimals">
+         <number>2</number>
+        </property>
+        <property name="minimum">
+         <double>0.000000000000000</double>
+        </property>
+        <property name="value">
+         <double>0.000000000000000</double>
+        </property>
+        <property name="singleStep">
+         <double>0.100000000000000</double>
+        </property>
+       </widget>
+      </item>
+      <item row="4" column="0">
+       <widget class="QLabel" name="lblStickSlope">
+        <property name="text">
+         <string>Stick Slope</string>
+        </property>
+       </widget>
+      </item>
+      <item row="4" column="1">
+       <widget class="Gui::QuantitySpinBox" name="spbStickSlope">
+        <property name="singleStep">
+         <double>1.000000000000000</double>
+        </property>
+        <property name="maximum">
+         <double>1000000000.000000000000000</double>
+        </property>
+        <property name="unit" stdset="0">
+         <string notr="true">GPa/m</string>
+        </property>
+        <property name="value" stdset="0">
+         <double>1.000000000000000</double>
+        </property>
+       </widget>
+      </item>
+     </layout>
+    </widget>
    </item>
   </layout>
   <zorder>lbl_info_2</zorder>
@@ -194,9 +261,14 @@
  </widget>
  <customwidgets>
   <customwidget>
-   <class>Gui::InputField</class>
-   <extends>QLineEdit</extends>
-   <header>Gui/InputField.h</header>
+   <class>Gui::QuantitySpinBox</class>
+   <extends>QWidget</extends>
+   <header>Gui/QuantitySpinBox.h</header>
+  </customwidget>
+  <customwidget>
+   <class>Gui::DoubleSpinBox</class>
+   <extends>QWidget</extends>
+   <header>Gui/SpinBox.h</header>
   </customwidget>
  </customwidgets>
  <resources/>

src/Mod/Fem/femexamples/constraint_contact_shell_shell.py

@@ -193,9 +193,8 @@ def setup(doc=None, solvertype="ccxtools"):
         (lower_tube, "Face1"),
         (upper_tube, "Face1"),
     ]
-    con_contact.Friction = 0.0
-    # con_contact.Slope = "1000000.0 kg/(mm*s^2)"  # contact stiffness
-    con_contact.Slope = 1000000.0  # should be 1000000.0 kg/(mm*s^2)
+    con_contact.Friction = False
+    con_contact.Slope = "1000000.0 GPa/m"
     analysis.addObject(con_contact)
 
     # mesh

src/Mod/Fem/femexamples/constraint_contact_solid_solid.py

@@ -177,8 +177,8 @@ def setup(doc=None, solvertype="ccxtools"):
         (geom_obj, "Face7"),  # first seams slave face, TODO proof in writer code!
         (geom_obj, "Face3"),  # second seams master face, TODO proof in writer code!
     ]
-    con_contact.Friction = 0.0
-    con_contact.Slope = 1000000.0  # contact stiffness 1000000.0 kg/(mm*s^2)
+    con_contact.Friction = False
+    con_contact.Slope = "1000000.0 GPa/m"
     analysis.addObject(con_contact)
 
     # mesh

src/Mod/Fem/femobjects/constraint_tie.py

@@ -46,5 +46,14 @@ class ConstraintTie(base_fempythonobject.BaseFemPythonObject):
             "App::PropertyLength",
             "Tolerance",
             "Geometry",
-            "set max gap between tied faces"
+            "Set max gap between tied faces"
         )
+        obj.Tolerance = "0.0 mm"
+
+        obj.addProperty(
+            "App::PropertyBool",
+            "Adjust",
+            "Geometry",
+            "Adjust connected nodes"
+        )
+        obj.Adjust = False

src/Mod/Fem/femsolver/calculix/write_constraint_contact.py

@@ -68,20 +68,24 @@ def write_meshdata_constraint(f, femobj, contact_obj, ccxwriter):
 def write_constraint(f, femobj, contact_obj, ccxwriter):
 
     # floats read from ccx should use {:.13G}, see comment in writer module
+    adjust = ""
+    if contact_obj.Adjust.Value > 0:
+        adjust = ", ADJUST={:.13G}".format(
+            contact_obj.Adjust.getValueAs("mm").Value)
 
     f.write(
-        "*CONTACT PAIR, INTERACTION=INT{},TYPE=SURFACE TO SURFACE\n"
-        .format(contact_obj.Name)
+        "*CONTACT PAIR, INTERACTION=INT{}, TYPE=SURFACE TO SURFACE{}\n"
+        .format(contact_obj.Name, adjust)
     )
     ind_surf = "IND" + contact_obj.Name
     dep_surf = "DEP" + contact_obj.Name
-    f.write("{},{}\n".format(dep_surf, ind_surf))
+    f.write("{}, {}\n".format(dep_surf, ind_surf))
     f.write("*SURFACE INTERACTION, NAME=INT{}\n".format(contact_obj.Name))
-    f.write("*SURFACE BEHAVIOR,PRESSURE-OVERCLOSURE=LINEAR\n")
-    slope = contact_obj.Slope
+    f.write("*SURFACE BEHAVIOR, PRESSURE-OVERCLOSURE=LINEAR\n")
+    slope = contact_obj.Slope.getValueAs("MPa/mm").Value
     f.write("{:.13G}\n".format(slope))
-    friction = contact_obj.Friction
-    if friction > 0:
-        f.write("*FRICTION \n")
-        stick = (slope / 10.0)
+    if contact_obj.Friction:
+        f.write("*FRICTION\n")
+        friction = contact_obj.FrictionCoefficient
+        stick = contact_obj.StickSlope.getValueAs("MPa/mm").Value
         f.write("{:.13G}, {:.13G}\n".format(friction, stick))

src/Mod/Fem/femsolver/calculix/write_constraint_tie.py

@@ -70,10 +70,13 @@ def write_constraint(f, femobj, tie_obj, ccxwriter):
     # floats read from ccx should use {:.13G}, see comment in writer module
 
     tolerance = tie_obj.Tolerance.getValueAs("mm").Value
+    adjust = ""
+    if not tie_obj.Adjust:
+        adjust = ", ADJUST=NO"
     f.write(
-        "*TIE, POSITION TOLERANCE={:.13G}, ADJUST=NO, NAME=TIE{}\n"
-        .format(tolerance, tie_obj.Name)
+        "*TIE, POSITION TOLERANCE={:.13G}{}, NAME=TIE{}\n"
+        .format(tolerance, adjust, tie_obj.Name)
     )
     ind_surf = "TIE_IND{}".format(tie_obj.Name)
     dep_surf = "TIE_DEP{}".format(tie_obj.Name)
-    f.write("{},{}\n".format(dep_surf, ind_surf))
+    f.write("{}, {}\n".format(dep_surf, ind_surf))

src/Mod/Fem/femtaskpanels/task_constraint_tie.py

@@ -52,10 +52,15 @@ class _TaskPanel:
             FreeCAD.getHomePath() + "Mod/Fem/Resources/ui/ConstraintTie.ui"
         )
         QtCore.QObject.connect(
-            self.parameterWidget.if_tolerance,
+            self.parameterWidget.spb_tolerance,
             QtCore.SIGNAL("valueChanged(Base::Quantity)"),
             self.tolerance_changed
         )
+        QtCore.QObject.connect(
+            self.parameterWidget.ckb_adjust,
+            QtCore.SIGNAL("toggled(bool)"),
+            self.adjust_changed
+        )
         self.init_parameter_widget()
 
         # geometry selection widget
@@ -67,7 +72,7 @@ class _TaskPanel:
         )
 
         # form made from param and selection widget
-        self.form = [self.parameterWidget, self.selectionWidget]
+        self.form = [self.selectionWidget, self.parameterWidget]
 
     def accept(self):
         # check values
@@ -94,6 +99,7 @@ class _TaskPanel:
             elif msgBox.clickedButton() == ignoreButton:
                 pass
         self.obj.Tolerance = self.tolerance
+        self.obj.Adjust = self.adjust
         self.obj.References = self.selectionWidget.references
         self.recompute_and_set_back_all()
         return True
@@ -112,7 +118,12 @@ class _TaskPanel:
 
     def init_parameter_widget(self):
         self.tolerance = self.obj.Tolerance
-        self.parameterWidget.if_tolerance.setText(self.tolerance.UserString)
+        self.adjust = self.obj.Adjust
+        self.parameterWidget.spb_tolerance.setProperty("value", self.tolerance)
+        self.parameterWidget.ckb_adjust.setChecked(self.adjust)
 
     def tolerance_changed(self, base_quantity_value):
         self.tolerance = base_quantity_value
+
+    def adjust_changed(self, bool_value):
+        self.adjust = bool_value

src/Mod/Fem/femtest/data/calculix/constraint_contact_shell_shell.inp

@@ -38365,10 +38365,10 @@ Efaces
 ***********************************************************
 ** Contact Constraints
 ** ConstraintContact
-*CONTACT PAIR, INTERACTION=INTConstraintContact,TYPE=SURFACE TO SURFACE
-DEPConstraintContact,INDConstraintContact
+*CONTACT PAIR, INTERACTION=INTConstraintContact, TYPE=SURFACE TO SURFACE
+DEPConstraintContact, INDConstraintContact
 *SURFACE INTERACTION, NAME=INTConstraintContact
-*SURFACE BEHAVIOR,PRESSURE-OVERCLOSURE=LINEAR
+*SURFACE BEHAVIOR, PRESSURE-OVERCLOSURE=LINEAR
 1000000
 
 ***********************************************************

src/Mod/Fem/femtest/data/calculix/constraint_contact_solid_solid.inp

@@ -5572,10 +5572,10 @@ Evolumes
 ***********************************************************
 ** Contact Constraints
 ** ConstraintContact
-*CONTACT PAIR, INTERACTION=INTConstraintContact,TYPE=SURFACE TO SURFACE
+*CONTACT PAIR, INTERACTION=INTConstraintContact, TYPE=SURFACE TO SURFACE
 DEPConstraintContact,INDConstraintContact
 *SURFACE INTERACTION, NAME=INTConstraintContact
-*SURFACE BEHAVIOR,PRESSURE-OVERCLOSURE=LINEAR
+*SURFACE BEHAVIOR, PRESSURE-OVERCLOSURE=LINEAR
 1000000
 
 ***********************************************************

src/Mod/Fem/femtest/data/calculix/constraint_tie.inp

@@ -18608,7 +18608,7 @@ Evolumes
 ** Tie Constraints
 ** ConstraintTie
 *TIE, POSITION TOLERANCE=25, ADJUST=NO, NAME=TIEConstraintTie
-TIE_DEPConstraintTie,TIE_INDConstraintTie
+TIE_DEPConstraintTie, TIE_INDConstraintTie
 
 ***********************************************************
 ** At least one step is needed to run an CalculiX analysis of FreeCAD

src/Mod/Part/App/FeaturePartCommon.cpp

@@ -80,6 +80,7 @@ short MultiCommon::mustExecute() const
 
 App::DocumentObjectExecReturn *MultiCommon::execute()
 {
+#ifndef FC_USE_TNP_FIX
     std::vector<TopoDS_Shape> s;
     std::vector<App::DocumentObject*> obj = Shapes.getValues();
 
@@ -194,4 +195,39 @@ App::DocumentObjectExecReturn *MultiCommon::execute()
     }
 
     return App::DocumentObject::StdReturn;
+#else
+    std::vector<TopoShape> shapes;
+    for (auto obj : Shapes.getValues()) {
+        TopoShape sh = Feature::getTopoShape(obj);
+        if (sh.isNull()) {
+            return new App::DocumentObjectExecReturn("Input shape is null");
+        }
+        shapes.push_back(sh);
+    }
+
+    TopoShape res {};
+    res.makeElementBoolean(Part::OpCodes::Common, shapes);
+    if (res.isNull()) {
+        throw Base::RuntimeError("Resulting shape is null");
+    }
+
+    Base::Reference<ParameterGrp> hGrp = App::GetApplication()
+                                             .GetUserParameter()
+                                             .GetGroup("BaseApp")
+                                             ->GetGroup("Preferences")
+                                             ->GetGroup("Mod/Part/Boolean");
+    if (hGrp->GetBool("CheckModel", false)) {
+        BRepCheck_Analyzer aChecker(res.getShape());
+        if (!aChecker.IsValid()) {
+            return new App::DocumentObjectExecReturn("Resulting shape is invalid");
+        }
+    }
+
+    if (this->Refine.getValue()) {
+        res = res.makeElementRefine();
+    }
+    this->Shape.setValue(res);
+
+    return Part::Feature::execute();
+#endif
 }

src/Mod/Part/App/TopoShape.h

@@ -652,7 +652,6 @@ public:
     void mapSubElement(const std::vector<TopoShape> &shapes, const char *op=nullptr);
     bool hasPendingElementMap() const;
 
-
     /** Helper class to return the generated and modified shape given an input shape
      *
      * Shape history information is extracted using OCCT APIs
@@ -674,6 +673,24 @@ public:
         }
     };
 
+    /** Core function to generate mapped element names from shape history
+     *
+     * @param shape: the new shape
+     * @param mapper: for mapping input shapes to generated/modified shapes
+     * @param sources: list of source shapes.
+     * @param op: optional string to be encoded into topo naming for indicating
+     *            the operation
+     *
+     * @return The original content of this TopoShape is discarded and replaced
+     *         with the given new shape. The function returns the TopoShape
+     *         itself as a self reference so that multiple operations can be
+     *         carried out for the same shape in the same line of code.
+     */
+    TopoShape &makeShapeWithElementMap(const TopoDS_Shape &shape,
+                                       const Mapper &mapper,
+                                       const std::vector<TopoShape> &sources,
+                                       const char *op=nullptr);
+
     /** Make a compound shape
      *
      * @param shapes: input shapes
@@ -690,6 +707,47 @@ public:
      */
     TopoShape &makeElementCompound(const std::vector<TopoShape> &shapes, const char *op=nullptr, bool force=true);
 
+    /* Make a shell using this shape
+     * @param silent: whether to throw exception on failure
+     * @param op: optional string to be encoded into topo naming for indicating
+     *            the operation
+     *
+     * @return The original content of this TopoShape is discarded and replaced
+     *         with the new shape. The function returns the TopoShape itself as
+     *         a self reference so that multiple operations can be carried out
+     *         for the same shape in the same line of code.
+     */
+    TopoShape& makeElementShell(bool silent = true, const char* op = nullptr);
+
+    /* Make a shell with input wires
+     *
+     * @param wires: input wires
+     * @param silent: whether to throw exception on failure
+     * @param op: optional string to be encoded into topo naming for indicating
+     *            the operation
+     *
+     * @return The original content of this TopoShape is discarded and replaced
+     *         with the new shape. The function returns the TopoShape itself as
+     *         a self reference so that multiple operations can be carried out
+     *         for the same shape in the same line of code.
+     */
+     // TopoShape& makeElementShellFromWires(const std::vector<TopoShape>& wires,
+     //                               bool silent = true,
+     //                               const char* op = nullptr);
+    /* Make a shell with input wires
+     *
+     * @param wires: input wires
+     * @param silent: whether to throw exception on failure
+     * @param op: optional string to be encoded into topo naming for indicating
+     *            the operation
+     *
+     * @return Return the new shape. The TopoShape itself is not modified.
+     */
+     // TopoShape& makeElementShellFromWires(bool silent = true, const char* op = nullptr)
+     // {
+     //     return makeElementShellFromWires(getSubTopoShapes(TopAbs_WIRE), silent, op);
+     // }
+
     TopoShape& makeElementFace(const std::vector<TopoShape>& shapes,
                                const char* op = nullptr,
                                const char* maker = nullptr,

src/Mod/Part/App/TopoShapeExpansion.cpp

@@ -25,21 +25,28 @@
 
 #include "PreCompiled.h"
 #ifndef _PreComp_
-#include <BRep_Builder.hxx>
-#include <BRepBuilderAPI_MakeWire.hxx>
-#include <BRep_Tool.hxx>
-#include <BRepTools.hxx>
 
+#include <BRepBuilderAPI_MakeWire.hxx>
+#include <BRepCheck_Analyzer.hxx>
+#include <BRepFill_Generator.hxx>
+#include <BRepTools.hxx>
+#include <BRep_Builder.hxx>
+#include <Precision.hxx>
 #include <ShapeFix_Shape.hxx>
 #include <ShapeFix_ShapeTolerance.hxx>
+#include <ShapeUpgrade_ShellSewing.hxx>
+#include <gp_Pln.hxx>
 
+#include <utility>
 #endif
 
 #include "TopoShape.h"
 #include "TopoShapeCache.h"
+#include "TopoShapeOpCode.h"
 #include "FaceMaker.h"
 
 #include "TopoShapeOpCode.h"
+#include <App/ElementNamingUtils.h>
 
 FC_LOG_LEVEL_INIT("TopoShape", true, true)  // NOLINT
 
@@ -336,6 +343,8 @@ void checkAndMatchHasher(TopoShape& topoShape1, const TopoShape& topoShape2)
 }
 }  // namespace
 
+
+// TODO: Refactor mapSubElementTypeForShape to reduce complexity
 void TopoShape::mapSubElementTypeForShape(const TopoShape& other,
                                           TopAbs_ShapeEnum type,
                                           const char* op,
@@ -385,8 +394,8 @@ void TopoShape::mapSubElementTypeForShape(const TopoShape& other,
             }
             std::ostringstream ss;
             char elementType {shapeName(type)[0]};
-            if ( ! elementMap() ) {
-                FC_THROWM(NullShapeException, "No element map");
+            if (!elementMap()) {
+                FC_THROWM(NullShapeException, "No element map");  // NOLINT
             }
             elementMap()->encodeElementName(elementType, name, ss, &sids, Tag, op, other.Tag);
             elementMap()->setElementName(element, name, Tag, &sids);
@@ -507,6 +516,743 @@ void TopoShape::mapSubElement(const std::vector<TopoShape>& shapes, const char*
     }
 }
 
+struct ShapeInfo
+{
+    const TopoDS_Shape& shape;
+    TopoShapeCache::Ancestry& cache;
+    TopAbs_ShapeEnum type;
+    const char* shapetype;
+
+    ShapeInfo(const TopoDS_Shape& shape, TopAbs_ShapeEnum type, TopoShapeCache::Ancestry& cache)
+        : shape(shape)
+        , cache(cache)
+        , type(type)
+        , shapetype(TopoShape::shapeName(type).c_str())
+    {}
+
+    int count() const
+    {
+        return cache.count();
+    }
+
+    TopoDS_Shape find(int index)
+    {
+        return cache.find(shape, index);
+    }
+
+    int find(const TopoDS_Shape& subshape)
+    {
+        return cache.find(shape, subshape);
+    }
+};
+
+////////////////////////////////////////
+// makESHAPE -> makeShapeWithElementMap
+///////////////////////////////////////
+
+struct NameKey
+{
+    Data::MappedName name;
+    long tag = 0;
+    int shapetype = 0;
+
+    NameKey()
+    = default;
+    explicit NameKey(Data::MappedName  n)
+        : name(std::move(n))
+    {}
+    NameKey(int type, Data::MappedName  n)
+        : name(std::move(n))
+    {
+        // Order the shape type from vertex < edge < face < other.  We'll rely
+        // on this for sorting when we name the geometry element.
+        switch (type) {
+            case TopAbs_VERTEX:
+                shapetype = 0;
+                break;
+            case TopAbs_EDGE:
+                shapetype = 1;
+                break;
+            case TopAbs_FACE:
+                shapetype = 2;
+                break;
+            default:
+                shapetype = 3;
+        }
+    }
+    bool operator<(const NameKey& other) const
+    {
+        if (shapetype < other.shapetype) {
+            return true;
+        }
+        if (shapetype > other.shapetype) {
+            return false;
+        }
+        if (tag < other.tag) {
+            return true;
+        }
+        if (tag > other.tag) {
+            return false;
+        }
+        return name < other.name;
+    }
+};
+
+struct NameInfo
+{
+    int index {};
+    Data::ElementIDRefs sids;
+    const char* shapetype {};
+};
+
+
+const std::string& modPostfix()
+{
+    static std::string postfix(Data::POSTFIX_MOD);
+    return postfix;
+}
+
+const std::string& modgenPostfix()
+{
+    static std::string postfix(Data::POSTFIX_MODGEN);
+    return postfix;
+}
+
+const std::string& genPostfix()
+{
+    static std::string postfix(Data::POSTFIX_GEN);
+    return postfix;
+}
+
+const std::string& upperPostfix()
+{
+    static std::string postfix(Data::POSTFIX_UPPER);
+    return postfix;
+}
+
+const std::string& lowerPostfix()
+{
+    static std::string postfix(Data::POSTFIX_LOWER);
+    return postfix;
+}
+
+// TODO: Refactor checkForParallelOrCoplanar to reduce complexity
+void checkForParallelOrCoplanar(const TopoDS_Shape& newShape,
+                                const ShapeInfo& newInfo,
+                                std::vector<TopoDS_Shape>& newShapes,
+                                const gp_Pln& pln,
+                                int parallelFace,
+                                int& coplanarFace,
+                                int& checkParallel)
+{
+    for (TopExp_Explorer xp(newShape, newInfo.type); xp.More(); xp.Next()) {
+        newShapes.push_back(xp.Current());
+
+        if ((parallelFace < 0 || coplanarFace < 0) && checkParallel > 0) {
+            // Specialized checking for high level mapped
+            // face that are either coplanar or parallel
+            // with the source face, which are common in
+            // operations like extrusion. Once found, the
+            // first coplanar face will assign an index of
+            // INT_MIN+1, and the first parallel face
+            // INT_MIN. The purpose of these special
+            // indexing is to make the name more stable for
+            // those generated faces.
+            //
+            // For example, the top or bottom face of an
+            // extrusion will be named using the extruding
+            // face. With a fixed index, the name is no
+            // longer affected by adding/removing of holes
+            // inside the extruding face/sketch.
+            gp_Pln plnOther;
+            if (TopoShape(newShapes.back()).findPlane(plnOther)) {
+                if (pln.Axis().IsParallel(plnOther.Axis(), Precision::Angular())) {
+                    if (coplanarFace < 0) {
+                        gp_Vec vec(pln.Axis().Location(), plnOther.Axis().Location());
+                        Standard_Real D1 = gp_Vec(pln.Axis().Direction()).Dot(vec);
+                        if (D1 < 0) {
+                            D1 = -D1;
+                        }
+                        Standard_Real D2 = gp_Vec(plnOther.Axis().Direction()).Dot(vec);
+                        if (D2 < 0) {
+                            D2 = -D2;
+                        }
+                        if (D1 <= Precision::Confusion() && D2 <= Precision::Confusion()) {
+                            coplanarFace = (int)newShapes.size();
+                            continue;
+                        }
+                    }
+                    if (parallelFace < 0) {
+                        parallelFace = (int)newShapes.size();
+                    }
+                }
+            }
+        }
+    }
+}
+
+// TODO: Refactor makeShapeWithElementMap to reduce complexity
+TopoShape& TopoShape::makeShapeWithElementMap(const TopoDS_Shape& shape,
+                                              const Mapper& mapper,
+                                              const std::vector<TopoShape>& shapes,
+                                              const char* op)
+{
+    setShape(shape);
+    if (shape.IsNull()) {
+        FC_THROWM(NullShapeException, "Null shape");
+    }
+
+    if (shapes.empty()) {
+        return *this;
+    }
+
+    size_t canMap = 0;
+    for (auto& incomingShape : shapes) {
+        if (canMapElement(incomingShape)) {
+            ++canMap;
+        }
+    }
+    if (canMap == 0U) {
+        return *this;
+    }
+    if (canMap != shapes.size() && FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) {
+        FC_WARN("Not all input shapes are mappable");  // NOLINT
+    }
+
+    if (!op) {
+        op = Part::OpCodes::Maker;
+    }
+    std::string _op = op;
+    _op += '_';
+
+    initCache();
+    ShapeInfo vertexInfo(_Shape, TopAbs_VERTEX, _cache->getAncestry(TopAbs_VERTEX));
+    ShapeInfo edgeInfo(_Shape, TopAbs_EDGE, _cache->getAncestry(TopAbs_EDGE));
+    ShapeInfo faceInfo(_Shape, TopAbs_FACE, _cache->getAncestry(TopAbs_FACE));
+    mapSubElement(shapes, op);
+
+    std::array<ShapeInfo*, 3> infos = {&vertexInfo, &edgeInfo, &faceInfo};
+
+    std::array<ShapeInfo*, TopAbs_SHAPE> infoMap {};
+    infoMap[TopAbs_VERTEX] = &vertexInfo;
+    infoMap[TopAbs_EDGE] = &edgeInfo;
+    infoMap[TopAbs_WIRE] = &edgeInfo;
+    infoMap[TopAbs_FACE] = &faceInfo;
+    infoMap[TopAbs_SHELL] = &faceInfo;
+    infoMap[TopAbs_SOLID] = &faceInfo;
+    infoMap[TopAbs_COMPOUND] = &faceInfo;
+    infoMap[TopAbs_COMPSOLID] = &faceInfo;
+
+    std::ostringstream ss;
+    std::string postfix;
+    Data::MappedName newName;
+
+    std::map<Data::IndexedName, std::map<NameKey, NameInfo>> newNames;
+
+    // First, collect names from other shapes that generates or modifies the
+    // new shape
+    for (auto& pinfo : infos) {
+        auto& info = *pinfo;
+        for (const auto & incomingShape : shapes) {
+            if (!canMapElement(incomingShape)) {
+                continue;
+            }
+            auto& otherMap = incomingShape._cache->getAncestry(info.type);
+            if (otherMap.count() == 0) {
+                continue;
+            }
+
+            for (int i = 1; i <= otherMap.count(); i++) {
+                const auto& otherElement = otherMap.find(incomingShape._Shape, i);
+                // Find all new objects that are a modification of the old object
+                Data::ElementIDRefs sids;
+                NameKey key(info.type,
+                    incomingShape.getMappedName(Data::IndexedName::fromConst(info.shapetype, i),
+                                                true,
+                                                &sids));
+
+                int newShapeCounter = 0;
+                for (auto& newShape : mapper.modified(otherElement)) {
+                    ++newShapeCounter;
+                    if (newShape.ShapeType() >= TopAbs_SHAPE) {
+                        // NOLINTNEXTLINE
+                        FC_ERR("unknown modified shape type " << newShape.ShapeType() << " from "
+                                                              << info.shapetype << i);
+                        continue;
+                    }
+                    auto& newInfo = *infoMap.at(newShape.ShapeType());
+                    if (newInfo.type != newShape.ShapeType()) {
+                        if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) {
+                            // TODO: it seems modified shape may report higher
+                            // level shape type just like generated shape below.
+                            // Maybe we shall do the same for name construction.
+                            // NOLINTNEXTLINE
+                            FC_WARN("modified shape type " << shapeName(newShape.ShapeType())
+                                                           << " mismatch with " << info.shapetype
+                                                           << i);
+                        }
+                        continue;
+                    }
+                    int newShapeIndex = newInfo.find(newShape);
+                    if (newShapeIndex == 0) {
+                        // This warning occurs in makERevolve. It generates
+                        // some shape from a vertex that never made into the
+                        // final shape. There may be incomingShape cases there.
+                        if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) {
+                            // NOLINTNEXTLINE
+                            FC_WARN("Cannot find " << op << " modified " << newInfo.shapetype
+                                                   << " from " << info.shapetype << i);
+                        }
+                        continue;
+                    }
+
+                    Data::IndexedName element = Data::IndexedName::fromConst(newInfo.shapetype, newShapeIndex);
+                    if (getMappedName(element)) {
+                        continue;
+                    }
+
+                    key.tag = incomingShape.Tag;
+                    auto& name_info = newNames[element][key];
+                    name_info.sids = sids;
+                    name_info.index = newShapeCounter;
+                    name_info.shapetype = info.shapetype;
+                }
+
+                int checkParallel = -1;
+                gp_Pln pln;
+
+                // Find all new objects that were generated from an old object
+                // (e.g. a face generated from an edge)
+                newShapeCounter = 0;
+                for (auto& newShape : mapper.generated(otherElement)) {
+                    if (newShape.ShapeType() >= TopAbs_SHAPE) {
+                        // NOLINTNEXTLINE
+                        FC_ERR("unknown generated shape type " << newShape.ShapeType() << " from "
+                                                               << info.shapetype << i);
+                        continue;
+                    }
+
+                    int parallelFace = -1;
+                    int coplanarFace = -1;
+                    auto& newInfo = *infoMap.at(newShape.ShapeType());
+                    std::vector<TopoDS_Shape> newShapes;
+                    int shapeOffset = 0;
+                    if (newInfo.type == newShape.ShapeType()) {
+                        newShapes.push_back(newShape);
+                    }
+                    else {
+                        // It is possible for the maker to report generating a
+                        // higher level shape, such as shell or solid. For
+                        // example, when extruding, OCC will report the
+                        // extruding face generating the entire solid. However,
+                        // it will also report the edges of the extruding face
+                        // generating the side faces. In this case, too much
+                        // information is bad for us. We don't want the name of
+                        // the side face (and its edges) to be coupled with
+                        // incomingShape (unrelated) edges in the extruding face.
+                        //
+                        // shapeOffset below is used to make sure the higher
+                        // level mapped names comes late after sorting. We'll
+                        // ignore those names if there are more precise mapping
+                        // available.
+                        shapeOffset = 3;
+
+                        if (info.type == TopAbs_FACE && checkParallel < 0) {
+                            if (!TopoShape(otherElement).findPlane(pln)) {
+                                checkParallel = 0;
+                            }
+                            else {
+                                checkParallel = 1;
+                            }
+                        }
+                        checkForParallelOrCoplanar(newShape,
+                                                   newInfo,
+                                                   newShapes,
+                                                   pln,
+                                                   parallelFace,
+                                                   coplanarFace,
+                                                   checkParallel);
+                    }
+                    key.shapetype += shapeOffset;
+                    for (auto& workingShape : newShapes) {
+                        ++newShapeCounter;
+                        int workingShapeIndex = newInfo.find(workingShape);
+                        if (workingShapeIndex == 0) {
+                            if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) {
+                                // NOLINTNEXTLINE
+                                FC_WARN("Cannot find " << op << " generated " << newInfo.shapetype
+                                                       << " from " << info.shapetype << i);
+                            }
+                            continue;
+                        }
+
+                        Data::IndexedName element =
+                            Data::IndexedName::fromConst(newInfo.shapetype, workingShapeIndex);
+                        auto mapped = getMappedName(element);
+                        if (mapped) {
+                            continue;
+                        }
+
+                        key.tag = incomingShape.Tag;
+                        auto& name_info = newNames[element][key];
+                        name_info.sids = sids;
+                        if (newShapeCounter == parallelFace) {
+                            name_info.index = std::numeric_limits<int>::min();
+                        }
+                        else if (newShapeCounter == coplanarFace) {
+                            name_info.index = std::numeric_limits<int>::min() + 1;
+                        }
+                        else {
+                            name_info.index = -newShapeCounter;
+                        }
+                        name_info.shapetype = info.shapetype;
+                    }
+                    key.shapetype -= shapeOffset;
+                }
+            }
+        }
+    }
+
+    // We shall first exclude those names generated from high level mapping. If
+    // there are still any unnamed elements left after we go through the process
+    // below, we set delayed=true, and start using those excluded names.
+    bool delayed = false;
+
+    while (true) {
+
+        // Construct the names for modification/generation info collected in
+        // the previous step
+        for (auto itName = newNames.begin(), itNext = itName; itNext != newNames.end();
+             itName = itNext) {
+            // We treat the first modified/generated source shape name specially.
+            // If case there are more than one source shape. We hash the first
+            // source name separately, and then obtain the second string id by
+            // hashing all the source names together.  We then use the second
+            // string id as the postfix for our name.
+            //
+            // In this way, we can associate the same source that are modified by
+            // multiple other shapes.
+
+            ++itNext;
+
+            auto& element = itName->first;
+            auto& names = itName->second;
+            const auto& first_key = names.begin()->first;
+            auto& first_info = names.begin()->second;
+
+            if (!delayed && first_key.shapetype >= 3 && first_info.index > INT_MIN + 1) {
+                // This name is mapped from high level (shell, solid, etc.)
+                // Delay till next round.
+                //
+                // index>INT_MAX+1 is for checking generated coplanar and
+                // parallel face mapping, which has special fixed index to make
+                // name stable.  These names are not delayed.
+                continue;
+            }
+            if (!delayed && getMappedName(element)) {
+                newNames.erase(itName);
+                continue;
+            }
+
+            int name_type =
+                first_info.index > 0 ? 1 : 2;  // index>0 means modified, or else generated
+            Data::MappedName first_name = first_key.name;
+
+            Data::ElementIDRefs sids(first_info.sids);
+
+            postfix.clear();
+            if (names.size() > 1) {
+                ss.str("");
+                ss << '(';
+                bool first = true;
+                auto it = names.begin();
+                int count = 0;
+                for (++it; it != names.end(); ++it) {
+                    auto& other_key = it->first;
+                    if (other_key.shapetype >= 3 && first_key.shapetype < 3) {
+                        // shapetype>=3 means it's a high level mapping (e.g. a face
+                        // generates a solid). We don't want that if there are more
+                        // precise low level mapping available. See comments above
+                        // for more details.
+                        break;
+                    }
+                    if (first) {
+                        first = false;
+                    }
+                    else {
+                        ss << '|';
+                    }
+                    auto& other_info = it->second;
+                    std::ostringstream ss2;
+                    if (other_info.index != 1) {
+                        // 'K' marks the additional source shape of this
+                        // generate (or modified) shape.
+                        ss2 << elementMapPrefix() << 'K';
+                        if (other_info.index == INT_MIN) {
+                            ss2 << '0';
+                        }
+                        else if (other_info.index == INT_MIN + 1) {
+                            ss2 << "00";
+                        }
+                        else {
+                            // The same source shape may generate or modify
+                            // more than one shape. The index here marks the
+                            // position it is reported by OCC. Including the
+                            // index here is likely to degrade name stablilty,
+                            // but is unfortunately a necessity to avoid
+                            // duplicate names.
+                            ss2 << other_info.index;
+                        }
+                    }
+                    Data::MappedName other_name = other_key.name;
+                    elementMap()->encodeElementName(*other_info.shapetype,
+                                                    other_name,
+                                                    ss2,
+                                                    &sids,
+                                                    Tag,
+                                                    nullptr,
+                                                    other_key.tag);
+                    ss << other_name;
+                    if ((name_type == 1 && other_info.index < 0)
+                        || (name_type == 2 && other_info.index > 0)) {
+                        if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) {
+                            FC_WARN("element is both generated and modified");  // NOLINT
+                        }
+                        name_type = 0;
+                    }
+                    sids += other_info.sids;
+                    // To avoid the name becoming to long, just put some limit here
+                    if (++count == 4) {
+                        break;
+                    }
+                }
+                if (!first) {
+                    ss << ')';
+                    if (Hasher) {
+                        sids.push_back(Hasher->getID(ss.str().c_str()));
+                        ss.str("");
+                        ss << sids.back().toString();
+                    }
+                    postfix = ss.str();
+                }
+            }
+
+            ss.str("");
+            if (name_type == 2) {
+                ss << genPostfix();
+            }
+            else if (name_type == 1) {
+                ss << modPostfix();
+            }
+            else {
+                ss << modgenPostfix();
+            }
+            if (first_info.index == INT_MIN) {
+                ss << '0';
+            }
+            else if (first_info.index == INT_MIN + 1) {
+                ss << "00";
+            }
+            else if (abs(first_info.index) > 1) {
+                ss << abs(first_info.index);
+            }
+            ss << postfix;
+            elementMap()
+                ->encodeElementName(element[0], first_name, ss, &sids, Tag, op, first_key.tag);
+            elementMap()->setElementName(element, first_name, Tag, &sids);
+
+            if (!delayed && first_key.shapetype < 3) {
+                newNames.erase(itName);
+            }
+        }
+
+        // The reverse pass. Starting from the highest level element, i.e.
+        // Face, for any element that are named, assign names for its lower unnamed
+        // elements. For example, if Edge1 is named E1, and its vertexes are not
+        // named, then name them as E1;U1, E1;U2, etc.
+        //
+        // In order to make the name as stable as possible, we may assign multiple
+        // names (which must be sorted, because we may use the first one to name
+        // upper element in the final pass) to lower element if it appears in
+        // multiple higher elements, e.g. same edge in multiple faces.
+
+        for (size_t infoIndex = infos.size() - 1; infoIndex != 0; --infoIndex) {
+            std::map<Data::IndexedName,
+                     std::map<Data::MappedName, NameInfo, Data::ElementNameComparator>>
+                names;
+            auto& info = *infos.at(infoIndex);
+            auto& next = *infos.at(infoIndex - 1);
+            int elementCounter = 1;
+            auto it = newNames.end();
+            if (delayed) {
+                it = newNames.upper_bound(Data::IndexedName::fromConst(info.shapetype, 0));
+            }
+            for (;; ++elementCounter) {
+                Data::IndexedName element;
+                if (!delayed) {
+                    if (elementCounter > info.count()) {
+                        break;
+                    }
+                    element = Data::IndexedName::fromConst(info.shapetype, elementCounter);
+                    if (newNames.count(element) != 0U) {
+                        continue;
+                    }
+                }
+                else if (it == newNames.end()
+                         || !boost::starts_with(it->first.getType(), info.shapetype)) {
+                    break;
+                }
+                else {
+                    element = it->first;
+                    ++it;
+                    elementCounter = element.getIndex();
+                    if (elementCounter == 0 || elementCounter > info.count()) {
+                        continue;
+                    }
+                }
+                Data::ElementIDRefs sids;
+                Data::MappedName mapped = getMappedName(element, false, &sids);
+                if (!mapped) {
+                    continue;
+                }
+
+                TopTools_IndexedMapOfShape submap;
+                TopExp::MapShapes(info.find(elementCounter), next.type, submap);
+                for (int submapIndex = 1, infoCounter = 1; submapIndex <= submap.Extent(); ++submapIndex) {
+                    ss.str("");
+                    int elementIndex = next.find(submap(submapIndex));
+                    assert(elementIndex);
+                    Data::IndexedName indexedName = Data::IndexedName::fromConst(next.shapetype, elementIndex);
+                    if (getMappedName(indexedName)) {
+                        continue;
+                    }
+                    auto& infoRef = names[indexedName][mapped];
+                    infoRef.index = infoCounter++;
+                    infoRef.sids = sids;
+                }
+            }
+            // Assign the actual names
+            for (auto& [indexedName, nameInfoMap] : names) {
+                // Do we really want multiple names for an element in this case?
+                // If not, we just pick the name in the first sorting order here.
+                auto& nameInfoMapEntry = *nameInfoMap.begin();
+                {
+                    auto& nameInfo = nameInfoMapEntry.second;
+                    auto& sids = nameInfo.sids;
+                    newName = nameInfoMapEntry.first;
+                    ss.str("");
+                    ss << upperPostfix();
+                    if (nameInfo.index > 1) {
+                        ss << nameInfo.index;
+                    }
+                    elementMap()->encodeElementName(indexedName[0], newName, ss, &sids, Tag, op);
+                    elementMap()->setElementName(indexedName, newName, Tag, &sids);
+                }
+            }
+        }
+
+        // The forward pass. For any elements that are not named, try construct its
+        // name from the lower elements
+        bool hasUnnamed = false;
+        for (size_t ifo = 1; ifo < infos.size(); ++ifo) {
+            auto& info = *infos.at(ifo);
+            auto& prev = *infos.at(ifo-1);
+            for (int i = 1; i <= info.count(); ++i) {
+                Data::IndexedName element = Data::IndexedName::fromConst(info.shapetype, i);
+                if (getMappedName(element)) {
+                    continue;
+                }
+
+                Data::ElementIDRefs sids;
+                std::map<Data::MappedName, Data::IndexedName, Data::ElementNameComparator> names;
+                TopExp_Explorer xp;
+                if (info.type == TopAbs_FACE) {
+                    xp.Init(BRepTools::OuterWire(TopoDS::Face(info.find(i))), TopAbs_EDGE);
+                }
+                else {
+                    xp.Init(info.find(i), prev.type);
+                }
+                for (; xp.More(); xp.Next()) {
+                    int previousElementIndex = prev.find(xp.Current());
+                    assert(previousElementIndex);
+                    Data::IndexedName prevElement = Data::IndexedName::fromConst(prev.shapetype, previousElementIndex);
+                    if (!delayed && (newNames.count(prevElement) != 0U)) {
+                        names.clear();
+                        break;
+                    }
+                    Data::ElementIDRefs sid;
+                    Data::MappedName name = getMappedName(prevElement, false, &sid);
+                    if (!name) {
+                        // only assign name if all lower elements are named
+                        if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) {
+                            FC_WARN("unnamed lower element " << prevElement);  // NOLINT
+                        }
+                        names.clear();
+                        break;
+                    }
+                    auto res = names.emplace(name, prevElement);
+                    if (res.second) {
+                        sids += sid;
+                    }
+                    else if (prevElement != res.first->second) {
+                        // The seam edge will appear twice, which is normal. We
+                        // only warn if the mapped element names are different.
+                        // NOLINTNEXTLINE
+                        FC_WARN("lower element " << prevElement << " and " << res.first->second
+                                                 << " has duplicated name " << name << " for "
+                                                 << info.shapetype << i);
+                    }
+                }
+                if (names.empty()) {
+                    hasUnnamed = true;
+                    continue;
+                }
+                auto it = names.begin();
+                newName = it->first;
+                if (names.size() == 1) {
+                    ss << lowerPostfix();
+                }
+                else {
+                    bool first = true;
+                    ss.str("");
+                    if (!Hasher) {
+                        ss << lowerPostfix();
+                    }
+                    ss << '(';
+                    int count = 0;
+                    for (++it; it != names.end(); ++it) {
+                        if (first) {
+                            first = false;
+                        }
+                        else {
+                            ss << '|';
+                        }
+                        ss << it->first;
+
+                        // To avoid the name becoming to long, just put some limit here
+                        if (++count == 4) {
+                            break;
+                        }
+                    }
+                    ss << ')';
+                    if (Hasher) {
+                        sids.push_back(Hasher->getID(ss.str().c_str()));
+                        ss.str("");
+                        ss << lowerPostfix() << sids.back().toString();
+                    }
+                }
+                elementMap()->encodeElementName(element[0], newName, ss, &sids, Tag, op);
+                elementMap()->setElementName(element, newName, Tag, &sids);
+            }
+        }
+        if (!hasUnnamed || delayed || newNames.empty()) {
+            break;
+        }
+        delayed = true;
+    }
+    return *this;
+}
+
 namespace
 {
 void addShapesToBuilder(const std::vector<TopoShape>& shapes,
@@ -768,7 +1514,8 @@ TopoShape& TopoShape::makeElementFace(const std::vector<TopoShape>& shapes,
 /**
  *  Encode and set an element name in the elementMap.  If a hasher is defined, apply it to the name.
  *
- * @param element   The element name(type) that provides 1 one character suffix to the name IF <conditions>.
+ * @param element   The element name(type) that provides 1 one character suffix to the name IF
+ * <conditions>.
  * @param names     The subnames to build the name from.  If empty, return the TopoShape MappedName.
  * @param marker    The elementMap name or suffix to start the name with.  If null, use the
  *                  elementMapPrefix.
@@ -1015,6 +1762,29 @@ struct MapperSewing: Part::TopoShape::Mapper
         return _res;
     }
 };
+struct MapperFill: Part::TopoShape::Mapper
+{
+    BRepFill_Generator& maker;
+    explicit MapperFill(BRepFill_Generator& maker)
+        : maker(maker)
+    {}
+    const std::vector<TopoDS_Shape>& generated(const TopoDS_Shape& s) const override
+    {
+        _res.clear();
+        try {
+            TopTools_ListIteratorOfListOfShape it;
+            for (it.Initialize(maker.GeneratedShapes(s)); it.More(); it.Next()) {
+                _res.push_back(it.Value());
+            }
+        }
+        catch (const Standard_Failure& e) {
+            if (FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) {
+                FC_WARN("Exception on shape mapper: " << e.GetMessageString());
+            }
+        }
+        return _res;
+    }
+};
 
 const std::vector<TopoDS_Shape>& MapperMaker::modified(const TopoDS_Shape& s) const
 {
@@ -1050,4 +1820,117 @@ const std::vector<TopoDS_Shape>& MapperMaker::generated(const TopoDS_Shape& s) c
     return _res;
 }
 
+// topo naming counterpart of TopoShape::makeShell()
+TopoShape& TopoShape::makeElementShell(bool silent, const char* op)
+{
+    if (silent) {
+        if (isNull()) {
+            return *this;
+        }
+
+        if (shapeType(true) != TopAbs_COMPOUND) {
+            return *this;
+        }
+
+        // we need a compound that consists of only faces
+        TopExp_Explorer it;
+        // no shells
+        if (hasSubShape(TopAbs_SHELL)) {
+            return *this;
+        }
+
+        // no wires outside a face
+        it.Init(_Shape, TopAbs_WIRE, TopAbs_FACE);
+        if (it.More()) {
+            return *this;
+        }
+
+        // no edges outside a wire
+        it.Init(_Shape, TopAbs_EDGE, TopAbs_WIRE);
+        if (it.More()) {
+            return *this;
+        }
+
+        // no vertexes outside an edge
+        it.Init(_Shape, TopAbs_VERTEX, TopAbs_EDGE);
+        if (it.More()) {
+            return *this;
+        }
+    }
+    else if (!hasSubShape(TopAbs_FACE)) {
+        FC_THROWM(Base::CADKernelError, "Cannot make shell without face");
+    }
+
+    BRep_Builder builder;
+    TopoDS_Shape shape;
+    TopoDS_Shell shell;
+    builder.MakeShell(shell);
+
+    try {
+        for (const auto& face : getSubShapes(TopAbs_FACE)) {
+            builder.Add(shell, face);
+        }
+
+        TopoShape tmp(Tag, Hasher, shell);
+        tmp.resetElementMap();
+        tmp.mapSubElement(*this, op);
+
+        shape = shell;
+        BRepCheck_Analyzer check(shell);
+        if (!check.IsValid()) {
+            ShapeUpgrade_ShellSewing sewShell;
+            shape = sewShell.ApplySewing(shell);
+            // TODO confirm the above won't change OCCT topological naming
+        }
+
+        if (shape.IsNull()) {
+            if (silent) {
+                return *this;
+            }
+            FC_THROWM(NullShapeException, "Failed to make shell");
+        }
+
+        if (shape.ShapeType() != TopAbs_SHELL) {
+            if (silent) {
+                return *this;
+            }
+            FC_THROWM(Base::CADKernelError,
+                      "Failed to make shell: unexpected output shape type "
+                          << shapeType(shape.ShapeType(), true));
+        }
+
+        setShape(shape);
+        resetElementMap(tmp.elementMap());
+    }
+    catch (Standard_Failure& e) {
+        if (!silent) {
+            FC_THROWM(Base::CADKernelError, "Failed to make shell: " << e.GetMessageString());
+        }
+    }
+
+    return *this;
+}
+
+// TopoShape& TopoShape::makeElementShellFromWires(const std::vector<TopoShape>& wires,
+//                                                 bool silent,
+//                                                 const char* op)
+// {
+//     BRepFill_Generator maker;
+//     for (auto& w : wires) {
+//         if (w.shapeType(silent) == TopAbs_WIRE) {
+//             maker.AddWire(TopoDS::Wire(w.getShape()));
+//         }
+//     }
+//     if (wires.empty()) {
+//         if (silent) {
+//             _Shape.Nullify();
+//             return *this;
+//         }
+//         FC_THROWM(NullShapeException, "No input shapes");
+//     }
+//     maker.Perform();
+//     this->makeShapeWithElementMap(maker.Shell(), MapperFill(maker), wires, op);
+//     return *this;
+// }
+
 }  // namespace Part

src/Mod/TechDraw/Gui/CommandExtensionPack.cpp

@@ -123,24 +123,39 @@ void execHoleCircle(Gui::Command* cmd)
         return;
     }
     Gui::Command::openCommand(QT_TRANSLATE_NOOP("Command", "Bolt Circle Centerlines"));
-    double scale = objFeat->getScale();
+
+    // make the bolt hole circle from 3 scaled and rotated points
     Base::Vector3d bigCenter =
         _circleCenter(Circles[0]->center, Circles[1]->center, Circles[2]->center);
-    float bigRadius = (Circles[0]->center - bigCenter).Length();
+    double bigRadius = (Circles[0]->center - bigCenter).Length();
+    // now convert the center & radius to canonical form
+    bigCenter = DU::invertY(bigCenter);
+    bigCenter = CosmeticVertex::makeCanonicalPoint(objFeat, bigCenter);
+    bigCenter = DU::invertY(bigCenter);
+    bigRadius = bigRadius / objFeat->getScale();
     TechDraw::BaseGeomPtr bigCircle =
-        std::make_shared<TechDraw::Circle>(bigCenter / scale, bigRadius / scale);
+        std::make_shared<TechDraw::Circle>(bigCenter, bigRadius);
     std::string bigCircleTag = objFeat->addCosmeticEdge(bigCircle);
     TechDraw::CosmeticEdge* ceCircle = objFeat->getCosmeticEdge(bigCircleTag);
     _setLineAttributes(ceCircle);
+
+    // make the center lines for the individual bolt holes
+    constexpr double ExtendFactor{1.1};
     for (const TechDraw::CirclePtr& oneCircle : Circles) {
-        Base::Vector3d oneCircleCenter = oneCircle->center;
-        float oneRadius = oneCircle->radius;
-        Base::Vector3d delta = (oneCircle->center - bigCenter).Normalize() * (oneRadius + 2);
+        // convert the center to canonical form
+        Base::Vector3d oneCircleCenter = DU::invertY(oneCircle->center);
+        oneCircleCenter = CosmeticVertex::makeCanonicalPoint(objFeat, oneCircleCenter);
+        oneCircleCenter = DU::invertY(oneCircleCenter);
+        // oneCircle->radius is scaled.
+        float oneRadius = oneCircle->radius / objFeat->getScale();
+        // what is magic number 2 (now ExtendFactor)?  just a fudge factor to extend the line beyond the bolt
+        // hole circle?  should it be a function of hole diameter? maybe 110% of oneRadius?
+        Base::Vector3d delta = (oneCircleCenter - bigCenter).Normalize() * (oneRadius * ExtendFactor);
         Base::Vector3d startPt = oneCircleCenter + delta;
         Base::Vector3d endPt = oneCircleCenter - delta;
-        startPt.y = -startPt.y;
-        endPt.y = -endPt.y;
-        std::string oneLineTag = objFeat->addCosmeticEdge(startPt / scale, endPt / scale);
+        startPt = DU::invertY(startPt);
+        endPt = DU::invertY(endPt);
+        std::string oneLineTag = objFeat->addCosmeticEdge(startPt, endPt);
         TechDraw::CosmeticEdge* ceLine = objFeat->getCosmeticEdge(oneLineTag);
         _setLineAttributes(ceLine);
     }
@@ -1475,10 +1490,16 @@ void execExtendShortenLine(Gui::Command* cmd, bool extend)
             TechDraw::BaseGeomPtr baseGeo = objFeat->getGeomByIndex(num);
             if (baseGeo) {
                 if (baseGeo->getGeomType() == TechDraw::GENERIC) {
-                    TechDraw::GenericPtr genLine =
-                        std::static_pointer_cast<TechDraw::Generic>(baseGeo);
-                    Base::Vector3d P0 = genLine->points.at(0);
-                    Base::Vector3d P1 = genLine->points.at(1);
+                    // start and end points are scaled and rotated. invert the points
+                    // so the canonicalPoint math works correctly.
+                    Base::Vector3d P0 = DU::invertY(baseGeo->getStartPoint());
+                    Base::Vector3d P1 = DU::invertY(baseGeo->getEndPoint());
+                    // convert start and end to unscaled, unrotated.
+                    P0 = CosmeticVertex::makeCanonicalPoint(objFeat, P0);
+                    P1 = CosmeticVertex::makeCanonicalPoint(objFeat, P1);
+                    // put the points back into weird Qt coord system.
+                    P0 = DU::invertY(P0);
+                    P1 = DU::invertY(P1);
                     bool isCenterLine = false;
                     TechDraw::CenterLine* centerEdge = nullptr;
                     if (baseGeo->getCosmetic()) {
@@ -1499,7 +1520,6 @@ void execExtendShortenLine(Gui::Command* cmd, bool extend)
                             isCenterLine = true;
                             centerEdge = objFeat->getCenterLine(uniTag);
                         }
-                        double scale = objFeat->getScale();
                         Base::Vector3d direction = (P1 - P0).Normalize();
                         Base::Vector3d delta = direction * activeDimAttributes.getLineStretch();
                         Base::Vector3d startPt, endPt;
@@ -1519,7 +1539,7 @@ void execExtendShortenLine(Gui::Command* cmd, bool extend)
                         }
                         else {
                             std::string lineTag =
-                                objFeat->addCosmeticEdge(startPt / scale, endPt / scale);
+                                objFeat->addCosmeticEdge(startPt, endPt);
                             TechDraw::CosmeticEdge* lineEdge = objFeat->getCosmeticEdge(lineTag);
                             _setLineAttributes(lineEdge, oldStyle, oldWeight, oldColor);
                             objFeat->refreshCEGeoms();
@@ -2087,7 +2107,6 @@ void _createThreadLines(std::vector<std::string> SubNames, TechDraw::DrawViewPar
                         float factor)
 {
     // create symbolizing lines of a thread from the side seen
-    double scale = objFeat->getScale();
     std::string GeoType0 = TechDraw::DrawUtil::getGeomTypeFromName(SubNames[0]);
     std::string GeoType1 = TechDraw::DrawUtil::getGeomTypeFromName(SubNames[1]);
     if ((GeoType0 == "Edge") && (GeoType1 == "Edge")) {
@@ -2103,10 +2122,22 @@ void _createThreadLines(std::vector<std::string> SubNames, TechDraw::DrawViewPar
 
         TechDraw::GenericPtr line0 = std::static_pointer_cast<TechDraw::Generic>(geom0);
         TechDraw::GenericPtr line1 = std::static_pointer_cast<TechDraw::Generic>(geom1);
-        Base::Vector3d start0 = line0->points.at(0);
-        Base::Vector3d end0 = line0->points.at(1);
-        Base::Vector3d start1 = line1->points.at(0);
-        Base::Vector3d end1 = line1->points.at(1);
+        // start and end points are scaled and rotated. invert the points
+        // so the canonicalPoint math works correctly.
+        Base::Vector3d start0 = DU::invertY(line0->getStartPoint());
+        Base::Vector3d end0 = DU::invertY(line0->getEndPoint());
+        Base::Vector3d start1 = DU::invertY(line1->getStartPoint());
+        Base::Vector3d end1 = DU::invertY(line1->getEndPoint());
+        // convert start and end to unscaled, unrotated.
+        start0 = CosmeticVertex::makeCanonicalPoint(objFeat, start0);
+        start1 = CosmeticVertex::makeCanonicalPoint(objFeat, start1);
+        end0 = CosmeticVertex::makeCanonicalPoint(objFeat, end0);
+        end1 = CosmeticVertex::makeCanonicalPoint(objFeat, end1);
+        // put the points back into weird Qt coord system.
+        start0 = DU::invertY(start0);
+        start1 = DU::invertY(start1);
+        end0 = DU::invertY(end0);
+        end1 = DU::invertY(end1);
         if (DrawUtil::circulation(start0, end0, start1)
             != DrawUtil::circulation(end0, end1, start1)) {
             Base::Vector3d help1 = start1;
@@ -2114,17 +2145,13 @@ void _createThreadLines(std::vector<std::string> SubNames, TechDraw::DrawViewPar
             start1 = help2;
             end1 = help1;
         }
-        start0.y = -start0.y;
-        end0.y = -end0.y;
-        start1.y = -start1.y;
-        end1.y = -end1.y;
         float kernelDiam = (start1 - start0).Length();
         float kernelFactor = (kernelDiam * factor - kernelDiam) / 2;
         Base::Vector3d delta = (start1 - start0).Normalize() * kernelFactor;
         std::string line0Tag =
-            objFeat->addCosmeticEdge((start0 - delta) / scale, (end0 - delta) / scale);
+            objFeat->addCosmeticEdge(start0 - delta, end0 - delta);
         std::string line1Tag =
-            objFeat->addCosmeticEdge((start1 + delta) / scale, (end1 + delta) / scale);
+            objFeat->addCosmeticEdge(start1 + delta, end1 + delta);
         TechDraw::CosmeticEdge* cosTag0 = objFeat->getCosmeticEdge(line0Tag);
         TechDraw::CosmeticEdge* cosTag1 = objFeat->getCosmeticEdge(line1Tag);
         _setLineAttributes(cosTag0);