kindred/create
1
1
Fork 0
Code Issues 31 Pull Requests 1 Actions 5 Packages Projects 9 Releases 2 Wiki Activity

DXF: Fix Import related behavior reported in issues #13597 and #16068 (#16511)

Browse Source 
* Fix double-import on exception in ImportGUI.readDXF
The python code has a try/catch block intended to detect if the ImportGUI module is present and if so use its readDXF method, otherwise use Import.readDXF.
The block was incorrectly structured so that Import.readDXF would also be called if ImportGUI.readDXF raised an exception, causing the DXF file contents to be loaded twice into the drawing (at least, up to the point where the exception occurred)

* Make ImpExpDxfRead::MakeLayer use centralized accessor for Draft module
The importer class already had a method to find the python Draft module, but MakeLayer was doing the work itself. Now it uses the centralized accessor, which has also been improved to generate a message if the module can't be loaded.

* Give compounded objects names related to the containing layer name
If "Use Layers" is set and also "Group layers into blocks" the names of the generated compound objects will be based on the name of the containing layer.
If "Use Layers" is not set this will not occur because in general objects from several layers would be compounded together.
Fixes (partially) #16068, the remaining fix is likely just an explanation of the interaction of the options.
closes #13597

* Use correct (new) property name OverrideShapeAppearanceChildren
This corrects a bug created by commit 3aea798 which renamed the layer property "OverrideShapeColorChildren" to "OverrideShapeAppearanceChildren" but missed this particular reference to the property by its old name.

The code here called PyObject_SetAttrString which, due to the wrong attribute name, set an expection state in the Python engine, ultimately causing the next attempt at importing a module to fail, with various consequences depending on why the module is being imported.

* Wrap PyObject_SetAttrString and PyObject_GetAttrString with error-checkers
In DEBUG compiles these methods are wrapped to report errors which can occur if the property cannot be referenced.

* Make some error-printers static instead of const
They don't need the CDxfRead object to work

* Display exceptions raised by ReadEntity
Although the DXF reader has an ignore-errors flag, it should still report any errors it encounters. The flag is deemed to mean that, after an error, the reader should continue to try to read the file rather than quitting on the first error.

* [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>
This commit is contained in:
Kevin Martin
2024-09-23 11:28:00 -04:00
committed by GitHub
parent eacc2a40d5
commit a36a4b04e5
5 changed files with 43 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/Mod/Draft/importDXF.py
View File

@@ -2832,10 +2832,11 @@ def open(filename):
FreeCAD.setActiveDocument(doc.Name)
try:
import ImportGui
ImportGui.readDXF(filename)
except Exception:
import Import
Import.readDXF(filename)
else:
ImportGui.readDXF(filename)
Draft.convert_draft_texts() # convert annotations to Draft texts
doc.recompute()
@@ -2874,10 +2875,11 @@ def insert(filename, docname):
else:
try:
import ImportGui
ImportGui.readDXF(filename)
except Exception:
import Import
Import.readDXF(filename)
else:
ImportGui.readDXF(filename)
Draft.convert_draft_texts() # convert annotations to Draft texts
doc.recompute()

12
src/Mod/Import/App/dxf/ImpExpDxf.cpp
View File

@@ -103,7 +103,10 @@ bool ImpExpDxfRead::ReadEntitiesSection()
// TODO: We do end-to-end joining or complete merging as selected by the options.
for (auto& shapeSet : ShapesToCombine) {
m_entityAttributes = shapeSet.first;
CombineShapes(shapeSet.second, "Compound");
CombineShapes(shapeSet.second,
m_entityAttributes.m_Layer == nullptr
? "Compound"
: m_entityAttributes.m_Layer->Name.c_str());
}
}
else {
@@ -644,7 +647,7 @@ ImpExpDxfRead::MakeLayer(const std::string& name, ColorIndex_t color, std::strin
App::Color appColor = ObjectColor(color);
PyObject* draftModule = nullptr;
PyObject* layer = nullptr;
draftModule = PyImport_ImportModule("Draft");
draftModule = getDraftModule();
if (draftModule != nullptr) {
// After the colours, I also want to pass the draw_style, but there is an intervening
// line-width parameter. It is easier to just pass that parameter's default value than
@@ -667,12 +670,13 @@ ImpExpDxfRead::MakeLayer(const std::string& name, ColorIndex_t color, std::strin
appColor.b,
2.0,
"Solid");
Py_DECREF(draftModule);
}
auto result = new Layer(name, color, std::move(lineType), layer);
if (result->DraftLayerView != nullptr) {
PyObject_SetAttrString(result->DraftLayerView, "OverrideLineColorChildren", Py_False);
PyObject_SetAttrString(result->DraftLayerView, "OverrideShapeColorChildren", Py_False);
PyObject_SetAttrString(result->DraftLayerView,
"OverrideShapeAppearanceChildren",
Py_False);
}
// We make our own layer class even if we could not make a layer. MoveToLayer will ignore

4
src/Mod/Import/App/dxf/ImpExpDxf.h
View File

@@ -122,7 +122,11 @@ protected:
PyObject* getDraftModule()
{
if (DraftModule == nullptr) {
static int times = 0;
DraftModule = PyImport_ImportModule("Draft");
if (DraftModule == nullptr && times++ == 0) {
ImportError("Unable to locate \"Draft\" module");
}
}
return DraftModule;
}

4
src/Mod/Import/App/dxf/dxf.cpp
View File

@@ -2910,7 +2910,11 @@ bool CDxfRead::ReadEntitiesSection()
return false;
}
}
catch (const Base::Exception& e) {
e.ReportException();
}
catch (...) {
ImportError("CDxfRead::ReadEntity raised unknown exception\n");
}
}
else {

25
src/Mod/Import/App/dxf/dxf.h
View File

@@ -21,6 +21,7 @@
#include <string>
#include <vector>
#include <Base/Interpreter.h>
#include <Base/Matrix.h>
#include <Base/Vector3D.h>
#include <Base/Console.h>
@@ -685,12 +686,12 @@ protected:
// notification popup "Log" goes to a log somewhere and not to the screen/user at all
template<typename... args>
void ImportError(const char* format, args&&... argValues) const
static void ImportError(const char* format, args&&... argValues)
{
Base::ConsoleSingleton::Instance().Warning(format, std::forward<args>(argValues)...);
}
template<typename... args>
void ImportObservation(const char* format, args&&... argValues) const
static void ImportObservation(const char* format, args&&... argValues)
{
Base::ConsoleSingleton::Instance().Message(format, std::forward<args>(argValues)...);
}
@@ -922,5 +923,25 @@ public:
return m_entityAttributes.m_LineType[0] == 'h' || m_entityAttributes.m_LineType[0] == 'H';
}
static App::Color ObjectColor(ColorIndex_t colorIndex); // as rgba value
#ifdef DEBUG
protected:
static PyObject* PyObject_GetAttrString(PyObject* o, const char* attr_name)
{
PyObject* result = ::PyObject_GetAttrString(o, attr_name);
if (result == nullptr) {
ImportError("Unable to get Attribute '%s'\n", attr_name);
PyErr_Clear();
}
return result;
}
static void PyObject_SetAttrString(PyObject* o, const char* attr_name, PyObject* v)
{
if (::PyObject_SetAttrString(o, attr_name, v) != 0) {
ImportError("Unable to set Attribute '%s'\n", attr_name);
PyErr_Clear();
}
}
#endif
};
#endif