Skip to content

Commit

Permalink
Core: Fixes FreeCAD#18717
Browse files Browse the repository at this point in the history 
Neither in FreeCAD#18332 nor in any of the referenced issues a reason is given why the placement of the coordinate axes must be changed.
Only doing it to make the client code look more consistent is not a valid reason.

This change breaks forward and backward compatibility and with FreeCAD#18717 we already have a hard to solve issue. Furthermore, at this
point of time it's not even clear what other collateral damage the change may cause.

So, this commit hides all the implementation details of the DatumElement and restores the original placement for the axes.
  • Loading branch information
@wwmayer
wwmayer committed Jan 2, 2025
1 parent 90d8c34 commit 06af866
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 45 deletions.
62 changes: 30 additions & 32 deletions src/App/Datums.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ PROPERTY_SOURCE(App::Point, App::DatumElement)
PROPERTY_SOURCE(App::LocalCoordinateSystem, App::GeoFeature)

DatumElement::DatumElement(bool hideRole)
: baseDir{0.0, 0.0, 1.0}
{
ADD_PROPERTY_TYPE(Role,
(""),
Expand All @@ -63,7 +64,7 @@ DatumElement::~DatumElement() = default;
bool DatumElement::getCameraAlignmentDirection(Base::Vector3d& direction, const char* subname) const
{
Q_UNUSED(subname);
Placement.getValue().getRotation().multVec(Base::Vector3d(0., 0., 1.), direction);
Placement.getValue().getRotation().multVec(baseDir, direction);

return true;
}
Expand Down Expand Up @@ -95,13 +96,35 @@ Base::Vector3d DatumElement::getBasePoint() const

Base::Vector3d DatumElement::getDirection() const
{
Base::Vector3d dir(0.0, 0.0, 1.0);
Base::Vector3d dir(baseDir);
Base::Placement plc = Placement.getValue();
Base::Rotation rot = plc.getRotation();
rot.multVec(dir, dir);
return dir;
}

Base::Vector3d DatumElement::getBaseDirection() const
{
return baseDir;
}

void DatumElement::setBaseDirection(const Base::Vector3d& dir)
{
baseDir = dir;
}

// ----------------------------------------------------------------------------

Line::Line()
{
setBaseDirection(Base::Vector3d(1, 0, 0));
}

Point::Point()
{
setBaseDirection(Base::Vector3d(0, 0, 0));
}

// ----------------------------------------------------------------------------

LocalCoordinateSystem::LocalCoordinateSystem()
Expand Down Expand Up @@ -221,9 +244,9 @@ const std::vector<LocalCoordinateSystem::SetupData>& LocalCoordinateSystem::getS
{
static const std::vector<SetupData> setupData = {
// clang-format off
{App::Line::getClassTypeId(), AxisRoles[0], tr("X-axis"), Base::Rotation(Base::Vector3d(1, 1, 1), M_PI * 2 / 3)},
{App::Line::getClassTypeId(), AxisRoles[1], tr("Y-axis"), Base::Rotation(Base::Vector3d(-1, 1, 1), M_PI * 2 / 3)},
{App::Line::getClassTypeId(), AxisRoles[2], tr("Z-axis"), Base::Rotation()},
{App::Line::getClassTypeId(), AxisRoles[0], tr("X-axis"), Base::Rotation()},
{App::Line::getClassTypeId(), AxisRoles[1], tr("Y-axis"), Base::Rotation(Base::Vector3d(1, 1, 1), M_PI * 2 / 3)},
{App::Line::getClassTypeId(), AxisRoles[2], tr("Z-axis"), Base::Rotation(Base::Vector3d(1,-1, 1), M_PI * 2 / 3)},
{App::Plane::getClassTypeId(), PlaneRoles[0], tr("XY-plane"), Base::Rotation()},
{App::Plane::getClassTypeId(), PlaneRoles[1], tr("XZ-plane"), Base::Rotation(1.0, 0.0, 0.0, 1.0)},
{App::Plane::getClassTypeId(), PlaneRoles[2], tr("YZ-plane"), Base::Rotation(Base::Vector3d(1, 1, 1), M_PI * 2 / 3)},
Expand All @@ -233,7 +256,7 @@ const std::vector<LocalCoordinateSystem::SetupData>& LocalCoordinateSystem::getS
return setupData;
}

DatumElement* LocalCoordinateSystem::createDatum(SetupData& data)
DatumElement* LocalCoordinateSystem::createDatum(const SetupData& data)
{
App::Document* doc = getDocument();
std::string objName = doc->getUniqueObjectName(data.role);
Expand Down Expand Up @@ -297,10 +320,6 @@ void LocalCoordinateSystem::onDocumentRestored()

// In 0.22 origins did not have point.
migrateOriginPoint();

// In 0.22 the axis placement were wrong. The X axis had identity placement instead of the Z.
// This was fixed but we need to migrate old files.
migrateXAxisPlacement();
}

void LocalCoordinateSystem::migrateOriginPoint()
Expand All @@ -314,33 +333,12 @@ void LocalCoordinateSystem::migrateOriginPoint()
if (std::none_of(features.begin(), features.end(), isOrigin)) {
auto data = getData(PointRoles[0]);
auto* origin = createDatum(data);
origin->purgeTouched();
features.push_back(origin);
OriginFeatures.setValues(features);
}
}

void LocalCoordinateSystem::migrateXAxisPlacement()
{
constexpr const double tolerance = 1e-5;
auto features = OriginFeatures.getValues();

const auto& setupData = getSetupData();
for (auto* obj : features) {
auto* feature = dynamic_cast <App::DatumElement*> (obj);
if (!feature) { continue; }
for (auto data : setupData) {
// ensure the rotation is correct for the role
if (std::strcmp(feature->Role.getValue(), data.role) == 0) {
if (!feature->Placement.getValue().getRotation().isSame(data.rot, tolerance)) {
feature->Placement.setValue(Base::Placement(Base::Vector3d(), data.rot));
getDocument()->setStatus(App::Document::MigrateLCS, true);
}
break;
}
}
}
}

// ----------------------------------------------------------------------------

LocalCoordinateSystem::LCSExtension::LCSExtension(LocalCoordinateSystem* obj)
Expand Down
12 changes: 10 additions & 2 deletions src/App/Datums.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,18 @@ class AppExport DatumElement: public App::GeoFeature
App::LocalCoordinateSystem* getLCS() const;
Base::Vector3d getBasePoint() const;
Base::Vector3d getDirection() const;
Base::Vector3d getBaseDirection() const;

bool getCameraAlignmentDirection(Base::Vector3d& direction, const char* subname) const override;

/// Returns true if this DatumElement is part of a App::Origin.
bool isOriginFeature() const;

protected:
void setBaseDirection(const Base::Vector3d& dir);

private:
Base::Vector3d baseDir;
};

class AppExport Plane: public App::DatumElement
Expand All @@ -76,6 +83,7 @@ class AppExport Line: public App::DatumElement
PROPERTY_HEADER_WITH_OVERRIDE(App::DatumElement);

public:
Line();
const char* getViewProviderName() const override
{
return "Gui::ViewProviderLine";
Expand All @@ -87,6 +95,7 @@ class AppExport Point: public App::DatumElement
PROPERTY_HEADER_WITH_OVERRIDE(App::DatumElement);

public:
Point();
const char* getViewProviderName() const override
{
return "Gui::ViewProviderPoint";
Expand Down Expand Up @@ -245,11 +254,10 @@ class AppExport LocalCoordinateSystem: public App::GeoFeature
};
static const std::vector<SetupData>& getSetupData();

DatumElement* createDatum(SetupData& data);
DatumElement* createDatum(const SetupData& data);
SetupData getData(const char* role);

void migrateOriginPoint();
void migrateXAxisPlacement();
};

} // namespace App
Expand Down
14 changes: 9 additions & 5 deletions src/Gui/ViewProviderLine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#endif

#include <App/Datums.h>
#include <Gui/Utilities.h>
#include <Gui/ViewParams.h>

#include "ViewProviderLine.h"
Expand All @@ -53,7 +54,8 @@ ViewProviderLine::ViewProviderLine()

ViewProviderLine::~ViewProviderLine() = default;

void ViewProviderLine::attach(App::DocumentObject *obj) {
void ViewProviderLine::attach(App::DocumentObject *obj)
{
ViewProviderDatum::attach(obj);

// Setup label text and line colors
Expand Down Expand Up @@ -82,14 +84,16 @@ void ViewProviderLine::attach(App::DocumentObject *obj) {

static const float size = App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/View")->GetFloat("DatumLineSize", 70.0);

auto line = getObject<App::Line>();
Base::Vector3d dir = line->getBaseDirection();
SbVec3f verts[2];
if (noRole) {
verts[0] = SbVec3f(0, 0, 2 * size);
verts[0] = Base::convertTo<SbVec3f>(dir * 2 * size);
verts[1] = SbVec3f(0, 0, 0);
}
else {
verts[0] = SbVec3f(0, 0, size);
verts[1] = SbVec3f(0, 0, 0.2 * size);
verts[0] = Base::convertTo<SbVec3f>(dir * size);
verts[1] = Base::convertTo<SbVec3f>(dir * 0.2 * size);
}

// indexes used to create the edges
Expand All @@ -108,7 +112,7 @@ void ViewProviderLine::attach(App::DocumentObject *obj) {
sep->addChild ( pLines );

auto textTranslation = new SoTranslation ();
textTranslation->translation.setValue(SbVec3f(0, 0, size * 1.1));
textTranslation->translation.setValue(Base::convertTo<SbVec3f>(dir * 1.1 * size));
sep->addChild ( textTranslation );

auto ps = new SoPickStyle();
Expand Down
11 changes: 8 additions & 3 deletions src/Mod/Part/App/PartFeature.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,15 @@
#include <Base/Placement.h>
#include <Base/Rotation.h>
#include <Base/Stream.h>
#include <Base/Tools.h>
#include <Mod/Material/App/MaterialManager.h>

#include "Geometry.h"
#include "PartFeature.h"
#include "PartFeaturePy.h"
#include "PartPyCXX.h"
#include "TopoShapePy.h"
#include "Base/Tools.h"
#include "Tools.h"

using namespace Part;
namespace sp = std::placeholders;
Expand Down Expand Up @@ -1026,7 +1027,9 @@ static TopoShape _getTopoShape(const App::DocumentObject* obj,
if (linked->isDerivedFrom(App::Line::getClassTypeId())) {
static TopoDS_Shape _shape;
if (_shape.IsNull()) {
BRepBuilderAPI_MakeEdge builder(gp_Lin(gp_Pnt(0, 0, 0), gp_Dir(0, 0, 1)));
auto line = static_cast<App::Line*>(linked);
Base::Vector3d dir = line->getBaseDirection();
BRepBuilderAPI_MakeEdge builder(gp_Lin(gp_Pnt(0, 0, 0), Base::convertTo<gp_Dir>(dir)));
_shape = builder.Shape();
_shape.Infinite(Standard_True);
}
Expand All @@ -1035,7 +1038,9 @@ static TopoShape _getTopoShape(const App::DocumentObject* obj,
else if (linked->isDerivedFrom(App::Plane::getClassTypeId())) {
static TopoDS_Shape _shape;
if (_shape.IsNull()) {
BRepBuilderAPI_MakeFace builder(gp_Pln(gp_Pnt(0, 0, 0), gp_Dir(0, 0, 1)));
auto plane = static_cast<App::Plane*>(linked);
Base::Vector3d dir = plane->getBaseDirection();
BRepBuilderAPI_MakeFace builder(gp_Pln(gp_Pnt(0, 0, 0), Base::convertTo<gp_Dir>(dir)));
_shape = builder.Shape();
_shape.Infinite(Standard_True);
}
Expand Down
6 changes: 3 additions & 3 deletions src/Mod/Test/Document.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,17 +357,17 @@ def testSubObject(self):

res = obj.getSubObject("X_Axis", retType=2)
self.assertEqual(
res[1].multVec(FreeCAD.Vector(0, 0, 1)).getAngle(FreeCAD.Vector(1, 0, 0)), 0.0
res[1].multVec(FreeCAD.Vector(1, 0, 0)).getAngle(FreeCAD.Vector(1, 0, 0)), 0.0
)

res = obj.getSubObject("Y_Axis", retType=2)
self.assertEqual(
res[1].multVec(FreeCAD.Vector(0, 0, 1)).getAngle(FreeCAD.Vector(0, 1, 0)), 0.0
res[1].multVec(FreeCAD.Vector(1, 0, 0)).getAngle(FreeCAD.Vector(0, 1, 0)), 0.0
)

res = obj.getSubObject("Z_Axis", retType=2)
self.assertEqual(
res[1].multVec(FreeCAD.Vector(0, 0, 1)).getAngle(FreeCAD.Vector(0, 0, 1)), 0.0
res[1].multVec(FreeCAD.Vector(1, 0, 0)).getAngle(FreeCAD.Vector(0, 0, 1)), 0.0
)

res = obj.getSubObject("XY_Plane", retType=2)
Expand Down

0 comments on commit 06af866

Please sign in to comment.