From c9887d940007a32b5d07b6ca47acbeff9ccb609e Mon Sep 17 00:00:00 2001 From: Kevin Martin Date: Mon, 23 Sep 2024 05:39:02 -0400 Subject: [PATCH] DXF: Place objects in layer all at once rather than one at a time to improve DXF import speed dramatically. (#16596) * Place objects in layer all at once rather than one at a time. This reduces (by a factor of the number of objects in the layer) the number of times that the layer contents are traversed to set the properties of the contained objects. This means the layer insertion of O(n) rather then O(n^2) on the number of objects. Also remove a loop invariant in view_layer so the chidren are not traversed if colours or appearnces are not inherited from the layer. Fixes #15732 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .../Draft/draftviewproviders/view_layer.py | 11 ++-- src/Mod/Import/App/dxf/ImpExpDxf.cpp | 55 +++++++++---------- src/Mod/Import/App/dxf/ImpExpDxf.h | 4 +- 3 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/Mod/Draft/draftviewproviders/view_layer.py b/src/Mod/Draft/draftviewproviders/view_layer.py index 04c0199921..a4d676ebb9 100644 --- a/src/Mod/Draft/draftviewproviders/view_layer.py +++ b/src/Mod/Draft/draftviewproviders/view_layer.py @@ -228,16 +228,15 @@ class ViewProviderLayer: # Return if the property does not exist if not hasattr(vobj, prop): return + # If the override properties are not set return without change + if prop == "LineColor" and not vobj.OverrideLineColorChildren: + return + elif prop == "ShapeAppearance" and not vobj.OverrideShapeAppearanceChildren: + return for target_obj in obj.Group: target_vobj = target_obj.ViewObject - # If the override properties are not set return without change - if prop == "LineColor" and not vobj.OverrideLineColorChildren: - return - elif prop == "ShapeAppearance" and not vobj.OverrideShapeAppearanceChildren: - return - # This checks that the property exists in the target object, # and then sets the target property accordingly if hasattr(target_vobj, prop): diff --git a/src/Mod/Import/App/dxf/ImpExpDxf.cpp b/src/Mod/Import/App/dxf/ImpExpDxf.cpp index 6b55fb45b9..24c49d492d 100644 --- a/src/Mod/Import/App/dxf/ImpExpDxf.cpp +++ b/src/Mod/Import/App/dxf/ImpExpDxf.cpp @@ -112,14 +112,8 @@ bool ImpExpDxfRead::ReadEntitiesSection() } } if (m_preserveLayers) { - // Hide the Hidden layers if possible (if GUI exists) - // We do this now rather than when the layer is created so all objects - // within the layers also become hidden. for (auto& layerEntry : Layers) { - auto layer = (Layer*)layerEntry.second; - if (layer->DraftLayerView != nullptr && layer->Hidden) { - PyObject_CallMethod(layer->DraftLayerView, "hide", nullptr); - } + ((Layer*)layerEntry.second)->FinishLayer(); } } return true; @@ -610,16 +604,38 @@ ImpExpDxfRead::Layer::Layer(const std::string& name, std::string&& lineType, PyObject* drawingLayer) : CDxfRead::Layer(name, color, std::move(lineType)) - , DraftLayer(drawingLayer) + , GroupContents( + drawingLayer == nullptr + ? nullptr + : (App::PropertyLinkListHidden*)(((App::FeaturePythonPyT*) + drawingLayer) + ->getPropertyContainerPtr()) + ->getDynamicPropertyByName("Group")) , DraftLayerView(drawingLayer == nullptr ? nullptr : PyObject_GetAttrString(drawingLayer, "ViewObject")) {} ImpExpDxfRead::Layer::~Layer() { - Py_XDECREF(DraftLayer); Py_XDECREF(DraftLayerView); } +void ImpExpDxfRead::Layer::FinishLayer() const +{ + if (GroupContents != nullptr) { + // We have to move the object to layer->DraftLayer + // The DraftLayer will have a Proxy attribute which has a addObject attribute which we + // call with (draftLayer, draftObject) Checking from python, the layer is a + // App::FeaturePython, and its Proxy is a draftobjects.layer.Layer + GroupContents->setValue(Contents); + } + if (DraftLayerView != nullptr && Hidden) { + // Hide the Hidden layers if possible (if GUI exists) + // We do this now rather than when the layer is created so all objects + // within the layers also become hidden. + PyObject_CallMethod(DraftLayerView, "hide", nullptr); + } +} + CDxfRead::Layer* ImpExpDxfRead::MakeLayer(const std::string& name, ColorIndex_t color, std::string&& lineType) { @@ -669,26 +685,7 @@ ImpExpDxfRead::MakeLayer(const std::string& name, ColorIndex_t color, std::strin void ImpExpDxfRead::MoveToLayer(App::DocumentObject* object) const { if (m_preserveLayers) { - static PyObject* addObjectName = - PyUnicode_FromString("addObject"); // This never gets freed, we always have a reference - auto layer = static_cast(m_entityAttributes.m_Layer); - if (layer->DraftLayer != nullptr) { - // We have to move the object to layer->DraftLayer - // The DraftLayer will have a Proxy attribute which has a addObject attribute which we - // call with (draftLayer, draftObject) Checking from python, the layer is a - // App::FeaturePython, and its Proxy is a draftobjects.layer.Layer - PyObject* proxy = PyObject_GetAttrString(layer->DraftLayer, "Proxy"); - if (proxy != nullptr) { - // TODO: De we have to check if the method exists? The legacy importer does. - PyObject_CallMethodObjArgs(proxy, - addObjectName, - layer->DraftLayer, - object->getPyObject(), - nullptr); - Py_DECREF(proxy); - return; - } - } + static_cast(m_entityAttributes.m_Layer)->Contents.push_back(object); } // TODO: else Hide the object if it is in a Hidden layer? That won't work because we've cleared // out m_entityAttributes.m_Layer diff --git a/src/Mod/Import/App/dxf/ImpExpDxf.h b/src/Mod/Import/App/dxf/ImpExpDxf.h index d219e0f99f..5c0478c936 100644 --- a/src/Mod/Import/App/dxf/ImpExpDxf.h +++ b/src/Mod/Import/App/dxf/ImpExpDxf.h @@ -143,8 +143,10 @@ protected: void operator=(const Layer&) = delete; void operator=(Layer&&) = delete; ~Layer() override; - PyObject* const DraftLayer; PyObject* const DraftLayerView; + std::vector Contents; + void FinishLayer() const; + App::PropertyLinkListHidden* GroupContents; }; using FeaturePythonBuilder =