From ac07a56b754aa814405015284647d0745080b923 Mon Sep 17 00:00:00 2001 From: Benjamin Nauck Date: Fri, 20 Jun 2025 13:38:18 +0200 Subject: [PATCH] Sketcher: Add support for more than 3 points While this adds a new way, it keeps the old to make it easier to merge. This will allow other work to be merge before without rebase issues --- src/Mod/Sketcher/App/Constraint.cpp | 270 +++++++++++++++++++++++----- src/Mod/Sketcher/App/Constraint.h | 46 +++-- src/Mod/Sketcher/App/PreCompiled.h | 3 + 3 files changed, 261 insertions(+), 58 deletions(-) diff --git a/src/Mod/Sketcher/App/Constraint.cpp b/src/Mod/Sketcher/App/Constraint.cpp index e9f7dae4c6..0fc5fe0a2e 100644 --- a/src/Mod/Sketcher/App/Constraint.cpp +++ b/src/Mod/Sketcher/App/Constraint.cpp @@ -24,9 +24,16 @@ #ifndef _PreComp_ #include #include +#include #include +#include +#include +#include +#include #endif +#include + #include #include #include @@ -75,19 +82,24 @@ Constraint* Constraint::copy() const temp->Type = this->Type; temp->AlignmentType = this->AlignmentType; temp->Name = this->Name; - temp->First = this->First; - temp->FirstPos = this->FirstPos; - temp->Second = this->Second; - temp->SecondPos = this->SecondPos; - temp->Third = this->Third; - temp->ThirdPos = this->ThirdPos; temp->LabelDistance = this->LabelDistance; temp->LabelPosition = this->LabelPosition; temp->isDriving = this->isDriving; temp->InternalAlignmentIndex = this->InternalAlignmentIndex; temp->isInVirtualSpace = this->isInVirtualSpace; temp->isActive = this->isActive; + temp->elements = this->elements; // Do not copy tag, otherwise it is considered a clone, and a "rename" by the expression engine. + +#if SKETCHER_CONSTRAINT_USE_LEGACY_ELEMENTS + temp->First = this->First; + temp->FirstPos = this->FirstPos; + temp->Second = this->Second; + temp->SecondPos = this->SecondPos; + temp->Third = this->Third; + temp->ThirdPos = this->ThirdPos; +#endif + return temp; } @@ -145,19 +157,42 @@ void Constraint::Save(Writer& writer) const << "InternalAlignmentIndex=\"" << InternalAlignmentIndex << "\" "; } writer.Stream() << "Value=\"" << Value << "\" " - << "First=\"" << First << "\" " - << "FirstPos=\"" << (int)FirstPos << "\" " - << "Second=\"" << Second << "\" " - << "SecondPos=\"" << (int)SecondPos << "\" " - << "Third=\"" << Third << "\" " - << "ThirdPos=\"" << (int)ThirdPos << "\" " << "LabelDistance=\"" << LabelDistance << "\" " << "LabelPosition=\"" << LabelPosition << "\" " << "IsDriving=\"" << (int)isDriving << "\" " << "IsInVirtualSpace=\"" << (int)isInVirtualSpace << "\" " - << "IsActive=\"" << (int)isActive << "\" />" + << "IsActive=\"" << (int)isActive << "\" "; - << std::endl; + // Save elements + { + // Ensure backwards compatibility with old versions + writer.Stream() << "First=\"" << getElement(0).GeoId << "\" " + << "FirstPos=\"" << getElement(0).posIdAsInt() << "\" " + << "Second=\"" << getElement(1).GeoId << "\" " + << "SecondPos=\"" << getElement(1).posIdAsInt() << "\" " + << "Third=\"" << getElement(2).GeoId << "\" " + << "ThirdPos=\"" << getElement(2).posIdAsInt() << "\" "; +#if SKETCHER_CONSTRAINT_USE_LEGACY_ELEMENTS + auto elements = std::views::iota(size_t {0}, this->elements.size()) + | std::views::transform([&](size_t i) { + return getElement(i); + }); +#endif + auto geoIds = elements | std::views::transform([](const GeoElementId& e) { + return e.GeoId; + }); + auto posIds = elements | std::views::transform([](const GeoElementId& e) { + return e.posIdAsInt(); + }); + + const std::string ids = fmt::format("{}", fmt::join(geoIds, " ")); + const std::string positions = fmt::format("{}", fmt::join(posIds, " ")); + + writer.Stream() << "ElementIds=\"" << ids << "\" " + << "ElementPositions=\"" << positions << "\" "; + } + + writer.Stream() << "/>\n"; } void Constraint::Restore(XMLReader& reader) @@ -166,10 +201,6 @@ void Constraint::Restore(XMLReader& reader) Name = reader.getAttribute("Name"); Type = reader.getAttribute("Type"); Value = reader.getAttribute("Value"); - First = reader.getAttribute("First"); - FirstPos = reader.getAttribute("FirstPos"); - Second = reader.getAttribute("Second"); - SecondPos = reader.getAttribute("SecondPos"); if (this->Type == InternalAlignment) { AlignmentType = reader.getAttribute("InternalAlignmentType"); @@ -182,12 +213,6 @@ void Constraint::Restore(XMLReader& reader) AlignmentType = Undef; } - // read the third geo group if present - if (reader.hasAttribute("Third")) { - Third = reader.getAttribute("Third"); - ThirdPos = reader.getAttribute("ThirdPos"); - } - // Read the distance a constraint label has been moved if (reader.hasAttribute("LabelDistance")) { LabelDistance = (float)reader.getAttribute("LabelDistance"); @@ -208,19 +233,90 @@ void Constraint::Restore(XMLReader& reader) if (reader.hasAttribute("IsActive")) { isActive = reader.getAttribute("IsActive"); } + + if (reader.hasAttribute("ElementIds") && reader.hasAttribute("ElementPositions")) { + auto splitAndClean = [](std::string_view input) { + const char delimiter = ' '; + + auto tokens = input | std::views::split(delimiter) + | std::views::transform([](auto&& subrange) { + // workaround due to lack of std::ranges::to in c++20 + std::string token; + auto size = std::ranges::distance(subrange); + token.reserve(size); + for (char c : subrange) { + token.push_back(c); + } + return token; + }) + | std::views::filter([](const std::string& s) { + return !s.empty(); + }); + + return std::vector(tokens.begin(), tokens.end()); + }; + + const std::string elementIds = reader.getAttribute("ElementIds"); + const std::string elementPositions = reader.getAttribute("ElementPositions"); + + const auto ids = splitAndClean(elementIds); + const auto positions = splitAndClean(elementPositions); + + if (ids.size() != positions.size()) { + throw Base::ParserError(fmt::format("ElementIds and ElementPositions do not match in " + "size. Got {} ids and {} positions.", + ids.size(), + positions.size())); + } + + elements.clear(); + for (size_t i = 0; i < std::min(ids.size(), positions.size()); ++i) { + const int geoId {std::stoi(ids[i])}; + const PointPos pos {static_cast(std::stoi(positions[i]))}; + addElement(GeoElementId(geoId, pos)); + } + } + + // Ensure we have at least 3 elements + while (getElementsSize() < 3) { + addElement(GeoElementId(GeoEnum::GeoUndef, PointPos::none)); + } + + // Load deprecated First, Second, Third elements + // These take precedence over the new elements + // Even though these are deprecated, we still need to read them + // for compatibility with old files. + { + constexpr std::array names = {"First", "Second", "Third"}; + constexpr std::array posNames = {"FirstPos", "SecondPos", "ThirdPos"}; + static_assert(names.size() == posNames.size()); + + for (size_t i = 0; i < names.size(); ++i) { + if (reader.hasAttribute(names[i])) { + const int geoId {reader.getAttribute(names[i])}; + const PointPos pos {reader.getAttribute(posNames[i])}; + setElement(i, GeoElementId(geoId, pos)); + } + } + } } void Constraint::substituteIndex(int fromGeoId, int toGeoId) { - if (this->First == fromGeoId) { - this->First = toGeoId; +#if SKETCHER_CONSTRAINT_USE_LEGACY_ELEMENTS + for (size_t i = 0; i < elements.size(); ++i) { + const GeoElementId element = getElement(i); + if (element.GeoId == fromGeoId) { + setElement(i, GeoElementId(toGeoId, element.Pos)); + } } - if (this->Second == fromGeoId) { - this->Second = toGeoId; - } - if (this->Third == fromGeoId) { - this->Third = toGeoId; +#else + for (auto& element : elements) { + if (element.GeoId == fromGeoId) { + element = GeoElementId(toGeoId, element.Pos); + } } +#endif } void Constraint::substituteIndexAndPos(int fromGeoId, @@ -228,18 +324,22 @@ void Constraint::substituteIndexAndPos(int fromGeoId, int toGeoId, PointPos toPosId) { - if (this->First == fromGeoId && this->FirstPos == fromPosId) { - this->First = toGeoId; - this->FirstPos = toPosId; + const GeoElementId from {fromGeoId, fromPosId}; + +#if SKETCHER_CONSTRAINT_USE_LEGACY_ELEMENTS + for (size_t i = 0; i < elements.size(); ++i) { + const GeoElementId element = getElement(i); + if (element == from) { + setElement(i, GeoElementId(toGeoId, toPosId)); + } } - if (this->Second == fromGeoId && this->SecondPos == fromPosId) { - this->Second = toGeoId; - this->SecondPos = toPosId; - } - if (this->Third == fromGeoId && this->ThirdPos == fromPosId) { - this->Third = toGeoId; - this->ThirdPos = toPosId; +#else + for (auto& element : elements) { + if (element == from) { + element = GeoElementId(toGeoId, toPosId); + } } +#endif } std::string Constraint::typeToString(ConstraintType type) @@ -251,3 +351,91 @@ std::string Constraint::internalAlignmentTypeToString(InternalAlignmentType alig { return internalAlignmentType2str[alignment]; } + +bool Constraint::involvesGeoId(int geoId) const +{ +#if SKETCHER_CONSTRAINT_USE_LEGACY_ELEMENTS + auto elements = + std::views::iota(size_t {0}, this->elements.size()) | std::views::transform([&](size_t i) { + return getElement(i); + }); +#endif + return std::ranges::any_of(elements, [geoId](const auto& element) { + return element.GeoId == geoId; + }); +} +/// utility function to check if (`geoId`, `posId`) is one of the points/curves +bool Constraint::involvesGeoIdAndPosId(int geoId, PointPos posId) const +{ +#if SKETCHER_CONSTRAINT_USE_LEGACY_ELEMENTS + auto elements = + std::views::iota(size_t {0}, this->elements.size()) | std::views::transform([&](size_t i) { + return getElement(i); + }); +#endif + return std::ranges::find(elements, GeoElementId(geoId, posId)) != elements.end(); +} + +GeoElementId Constraint::getElement(size_t index) const +{ + if (index >= elements.size()) { + throw Base::IndexError("Constraint::getElement index out of range"); + } + +#if SKETCHER_CONSTRAINT_USE_LEGACY_ELEMENTS + if (index < 3) { + switch (index) { + case 0: + return GeoElementId(First, FirstPos); + case 1: + return GeoElementId(Second, SecondPos); + case 2: + return GeoElementId(Third, ThirdPos); + } + } +#endif + return elements[index]; +} +void Constraint::setElement(size_t index, GeoElementId element) +{ + if (index >= elements.size()) { + throw Base::IndexError("Constraint::getElement index out of range"); + } + + elements[index] = element; + +#if SKETCHER_CONSTRAINT_USE_LEGACY_ELEMENTS + if (index < 3) { + switch (index) { + case 0: + First = element.GeoId; + FirstPos = element.Pos; + break; + case 1: + Second = element.GeoId; + SecondPos = element.Pos; + break; + case 2: + Third = element.GeoId; + ThirdPos = element.Pos; + break; + } + } +#endif +} + +size_t Constraint::getElementsSize() const +{ + return elements.size(); +} + +void Constraint::addElement(GeoElementId element) +{ +#if SKETCHER_CONSTRAINT_USE_LEGACY_ELEMENTS + int i = elements.size(); + elements.resize(i + 1); + setElement(i, element); +#else + elements.push_back(element); +#endif +} diff --git a/src/Mod/Sketcher/App/Constraint.h b/src/Mod/Sketcher/App/Constraint.h index 8989358b75..00491e5bd4 100644 --- a/src/Mod/Sketcher/App/Constraint.h +++ b/src/Mod/Sketcher/App/Constraint.h @@ -33,6 +33,11 @@ #include "GeoEnum.h" +// Flipping this to 0 removes old legazy members First, FirstPos, Second... +// Will be used when everything has been migrated to new api. +#define SKETCHER_CONSTRAINT_USE_LEGACY_ELEMENTS 1 + + namespace Sketcher { /*! @@ -129,24 +134,18 @@ public: || Type == Diameter || Type == Angle || Type == SnellsLaw || Type == Weight; } - /// utility function to swap the index in First/Second/Third of the provided constraint from the + /// utility function to swap the index in elements of the provided constraint from the /// fromGeoId GeoId to toGeoId void substituteIndex(int fromGeoId, int toGeoId); - /// utility function to swap the index and position in First/Second/Third of the provided + /// utility function to swap the index and position in elements of the provided /// constraint from {fromGeoId, fromPosId} to {toGeoId, toPosId}. void substituteIndexAndPos(int fromGeoId, PointPos fromPosId, int toGeoId, PointPos toPosId); /// utility function to check if `geoId` is one of the geometries - bool involvesGeoId(int geoId) const - { - return First == geoId || Second == geoId || Third == geoId; - } + bool involvesGeoId(int geoId) const; + /// utility function to check if (`geoId`, `posId`) is one of the points/curves - bool involvesGeoIdAndPosId(int geoId, PointPos posId) const - { - return (First == geoId && FirstPos == posId) || (Second == geoId && SecondPos == posId) - || (Third == geoId && ThirdPos == posId); - } + bool involvesGeoIdAndPosId(int geoId, PointPos posId) const; std::string typeToString() const { @@ -210,12 +209,6 @@ public: ConstraintType Type {None}; InternalAlignmentType AlignmentType {Undef}; std::string Name; - int First {GeoEnum::GeoUndef}; - PointPos FirstPos {PointPos::none}; - int Second {GeoEnum::GeoUndef}; - PointPos SecondPos {PointPos::none}; - int Third {GeoEnum::GeoUndef}; - PointPos ThirdPos {PointPos::none}; float LabelDistance {10.F}; float LabelPosition {0.F}; bool isDriving {true}; @@ -226,6 +219,25 @@ public: bool isActive {true}; + GeoElementId getElement(size_t index) const; + void setElement(size_t index, GeoElementId element); + size_t getElementsSize() const; + void addElement(GeoElementId element); + +#ifdef SKETCHER_CONSTRAINT_USE_LEGACY_ELEMENTS + // Deprecated, use getElement/setElement instead + int First {GeoEnum::GeoUndef}; + int Second {GeoEnum::GeoUndef}; + int Third {GeoEnum::GeoUndef}; + PointPos FirstPos {PointPos::none}; + PointPos SecondPos {PointPos::none}; + PointPos ThirdPos {PointPos::none}; +#endif + +private: + // New way to access point ids and positions. + // While the old way is still supported, it is recommended to the getters and setters instead. + std::vector elements {GeoElementId(), GeoElementId(), GeoElementId()}; protected: boost::uuids::uuid tag; diff --git a/src/Mod/Sketcher/App/PreCompiled.h b/src/Mod/Sketcher/App/PreCompiled.h index 5bdacf3b44..b51f4033d0 100644 --- a/src/Mod/Sketcher/App/PreCompiled.h +++ b/src/Mod/Sketcher/App/PreCompiled.h @@ -32,12 +32,15 @@ #ifdef _PreComp_ // standard +#include #include #include #include #include #include +#include #include +#include #include // Qt