Sketcher: GeoList Improvements

==============================

- Improve GeoList to support GeoElementId input
- Support VertexId to GeoElementId conversion
- GeoListModel gets separate getGeometry and getGeometryFacade naked pointers
- Sketcher: GeoList - make index mappings mutable
- Improve documentation
This commit is contained in:
Abdullah Tahiri
2021-11-28 17:06:19 +01:00
parent 3c43b50608
commit e20039cbbb
2 changed files with 241 additions and 62 deletions

View File

@@ -30,6 +30,7 @@
#include <assert.h>
#include <Base/Vector3D.h>
#include <Base/Exception.h>
#include <Mod/Sketcher/App/GeometryFacade.h>
@@ -45,7 +46,8 @@ GeoListModel<T>::GeoListModel( std::vector<T> && geometrylist,
int intgeocount,
bool ownerT): geomlist(std::move(geometrylist)),
intGeoCount(intgeocount),
OwnerT(ownerT)
OwnerT(ownerT),
indexInit(false)
{
}
@@ -56,7 +58,8 @@ GeoListModel<T>::GeoListModel( const std::vector<T> & geometrylist,
int intgeocount,
bool ownerT): geomlist(geometrylist), // copy constructed here
intGeoCount(intgeocount),
OwnerT(ownerT)
OwnerT(ownerT),
indexInit(false)
{
}
@@ -95,29 +98,66 @@ int GeoListModel<T>::getGeoIdFromGeomListIndex(int index) const
}
template <typename T>
const T GeoListModel<T>::getGeometryFromGeoId(const std::vector<T> & geometrylist, int geoId)
const Part::Geometry * GeoListModel<T>::getGeometryFromGeoId(const std::vector<T> & geometrylist, int geoId)
{
if (geoId >= 0)
return geometrylist[geoId];
else
return geometrylist[geometrylist.size()+geoId];
if constexpr (std::is_same<T,GeometryPtr>()) {
if (geoId >= 0)
return geometrylist[geoId];
else
return geometrylist[geometrylist.size()+geoId];
}
else if constexpr (std::is_same<T,GeometryFacadeUniquePtr>()) {
if (geoId >= 0)
return geometrylist[geoId]->getGeometry();
else
return geometrylist[geometrylist.size()+geoId]->getGeometry();
}
}
template <typename T>
const Sketcher::GeometryFacade * GeoListModel<T>::getGeometryFacadeFromGeoId(const std::vector<T> & geometrylist, int geoId)
{
if constexpr (std::is_same<T,GeometryPtr>()) {
if (geoId >= 0)
return GeometryFacade::getFacade(geometrylist[geoId]).release();
else
return GeometryFacade::getFacade(geometrylist[geometrylist.size()+geoId]).release();
}
else if constexpr (std::is_same<T,GeometryFacadeUniquePtr>()) {
if (geoId >= 0)
return geometrylist[geoId].get();
else
return geometrylist[geometrylist.size()+geoId].get();
}
}
// this function is used to simulate cyclic periodic negative geometry indices (for external geometry)
template <typename T>
const T GeoListModel<T>::getGeometryFromGeoId(int geoId) const
const Part::Geometry * GeoListModel<T>::getGeometryFromGeoId(int geoId) const
{
return GeoListModel<T>::getGeometryFromGeoId(geomlist, geoId);
}
template <typename T>
const Sketcher::GeometryFacade * GeoListModel<T>::getGeometryFacadeFromGeoId(int geoId) const
{
return GeoListModel<T>::getGeometryFacadeFromGeoId(geomlist, geoId);
}
template <typename T>
Base::Vector3d GeoListModel<T>::getPoint(int geoId, Sketcher::PointPos pos) const
{
Part::Geometry * geo = getGeometryFromGeoId(geoId);
const Part::Geometry * geo = getGeometryFromGeoId(geoId);
return getPoint(geo, pos);
}
template <typename T>
Base::Vector3d GeoListModel<T>::getPoint(const GeoElementId & geid) const
{
return getPoint(geid.GeoId, geid.Pos);
}
template <typename T>
Base::Vector3d GeoListModel<T>::getPoint(const Part::Geometry * geo, Sketcher::PointPos pos) const
{
@@ -184,6 +224,83 @@ Base::Vector3d GeoListModel<T>::getPoint(const Part::Geometry * geo, Sketcher::P
return Base::Vector3d();
}
template <typename T>
void GeoListModel<T>::rebuildVertexIndex(void) const
{
VertexId2GeoElementId.clear();
GeoElementId2VertexId.clear();
int geoId=0;
int pointId=0;
auto addGeoElement = [this, &pointId](int geoId, PointPos pos) {
VertexId2GeoElementId.emplace_back(geoId,pos);
GeoElementId2VertexId.emplace(std::piecewise_construct,
std::forward_as_tuple(geoId, pos),
std::forward_as_tuple(pointId++));
};
if (geomlist.size() <= 2)
return;
for (auto it = geomlist.begin(); it != geomlist.end(); ++it, geoId++) {
Base::Type type;
if constexpr (std::is_same<T, Part::Geometry *>::value)
type = (*it)->getTypeId();
else if constexpr (std::is_same<T, std::unique_ptr<const Sketcher::GeometryFacade>>::value)
type = (*it)->getGeometry()->getTypeId();
if ( geoId > getInternalCount())
geoId = -getExternalCount();
if (type == Part::GeomPoint::getClassTypeId()) {
addGeoElement(geoId,PointPos::start);
} else if (type == Part::GeomLineSegment::getClassTypeId() ||
type == Part::GeomBSplineCurve::getClassTypeId()) {
addGeoElement(geoId,PointPos::start);
addGeoElement(geoId,PointPos::end);
} else if (type == Part::GeomCircle::getClassTypeId() ||
type == Part::GeomEllipse::getClassTypeId()) {
addGeoElement(geoId,PointPos::mid);
} else if (type == Part::GeomArcOfCircle::getClassTypeId() ||
type == Part::GeomArcOfEllipse::getClassTypeId() ||
type == Part::GeomArcOfHyperbola::getClassTypeId() ||
type == Part::GeomArcOfParabola::getClassTypeId()) {
addGeoElement(geoId,PointPos::start);
addGeoElement(geoId,PointPos::end);
addGeoElement(geoId,PointPos::mid);
}
}
indexInit = true;
}
template <typename T>
Sketcher::GeoElementId GeoListModel<T>::getGeoElementIdFromVertexId(int vertexId)
{
if(!indexInit) // lazy initialised
rebuildVertexIndex();
return VertexId2GeoElementId[vertexId];
}
template <typename T>
int GeoListModel<T>::getVertexIdFromGeoElementId(const Sketcher::GeoElementId & geoelementId) const
{
if(!indexInit) // lazy initialised
rebuildVertexIndex();
auto found = std::find(VertexId2GeoElementId.begin(), VertexId2GeoElementId.end(), geoelementId);
if( found != VertexId2GeoElementId.end() )
return std::distance(found, VertexId2GeoElementId.begin());
THROWM(Base::IndexError, "GeoElementId not indexed");
}
namespace Sketcher {
// Template specialisations
@@ -193,7 +310,8 @@ GeoListModel<std::unique_ptr< const Sketcher::GeometryFacade>>::GeoListModel(
int intgeocount,
bool ownerT) : geomlist(std::move(geometrylist)),
intGeoCount(intgeocount),
OwnerT(false)
OwnerT(false),
indexInit(false)
{
// GeometryFacades hold the responsibility for releasing the resources.
//
@@ -212,7 +330,8 @@ GeoListModel<std::unique_ptr< const Sketcher::GeometryFacade>>::GeoListModel(
const std::vector<std::unique_ptr< const Sketcher::GeometryFacade>> & geometrylist,
int intgeocount,
bool ownerT): intGeoCount(intgeocount),
OwnerT(false)
OwnerT(false),
indexInit(false)
{
// GeometryFacades are movable, but not copiable, so they need to be reconstructed (shallow copy of vector)
// Under the Single Responsibility Principle, these will not take over a responsibility that shall be enforced
@@ -233,24 +352,6 @@ GeoListModel<std::unique_ptr<const Sketcher::GeometryFacade>>::~GeoListModel()
}
template < >
const std::unique_ptr<const Sketcher::GeometryFacade>
GeoListModel<std::unique_ptr<const Sketcher::GeometryFacade>>::getGeometryFromGeoId
(const std::vector<std::unique_ptr<const Sketcher::GeometryFacade>> & geometrylist, int geoId)
{
if (geoId >= 0)
return Sketcher::GeometryFacade::getFacade(geometrylist[geoId]->getGeometry());
else
return Sketcher::GeometryFacade::getFacade(geometrylist[geometrylist.size()+geoId]->getGeometry());
}
template < >
Base::Vector3d GeoListModel<std::unique_ptr<const Sketcher::GeometryFacade>>::getPoint(int geoId, Sketcher::PointPos pos) const
{
const Part::Geometry * geo = getGeometryFromGeoId(geoId)->getGeometry();
return getPoint(geo, pos);
}
// instantiate the types so that other translation units can access template constructors
template class GeoListModel<Part::Geometry *>;

View File

@@ -31,6 +31,7 @@
#include <vector>
#include <memory>
#include <Mod/Sketcher/App/GeoEnum.h>
#include <Mod/Sketcher/App/GeometryFacade.h>
namespace Base {
@@ -43,22 +44,31 @@ namespace Part {
}
namespace Sketcher {
enum class PointPos : int;
class GeometryFacade;
}
namespace Sketcher {
// TODO: This class is half-cooked and needs to be reviewed. Specially the const/non-const aspect
// as well as the ability to take ownership of deepcopied vectors.
/** @brief Class for managing internal and external geometry as a single object
* @details
* Internal and external geometries are present in a single geometry vector one after the other.
*
* This class follows the format used by solver facade (sketch.cpp) in:
* getSolvedSketch().extractGeometry() and by SketchObject in getCompleteGeometry(). Care should be
* taken that the former may provide the list with a deep copy of the geometry pointers, whereas the
* second just provides a shallow copy of the pointers.
*
* This class is templated to allow instantiations with list elements being legacy naked pointers
* (Part::Geometry *) and GeometryFacade smart pointer objects. Convenience typedefs are provided:
*
* using GeoList = GeoListModel<GeometryPtr>;
* using GeoListFacade = GeoListModel<GeometryFacadeUniquePtr>;
*
* with:
* using GeometryPtr = Part::Geometry *;
* using GeometryFacadeUniquePtr = std::unique_ptr<const Sketcher::GeometryFacade>;
*
* N.B.: Note that the index of the geomlist (all layers) and the GeoId can be converted
* from each other at needed using the member functions (and sometimes the statics).
* from each other as needed using the member fuctions (and sometimes the static functions).
*/
template <typename T>
class GeoListModel {
@@ -66,76 +76,136 @@ class GeoListModel {
protected:
/**
* Constructs the object from a list of geometry in geomlist format and the number of internal
* geometries (non external) present in the list.
*
* @param geometrylist: the geometry in geomlist format (external after internal in a single vector).
* @param intgeocount: the number of internal geometries (non external) in the list.
* @param ownerT: indicates whether the GeoListModel takes ownership of the elements of the std::vector<T> (for pointers)
*/
/** @brief
* Constructors are protected, use static methods getGeoListModel() to construct the objects instead.
*
* Constructs the object from a list of geometry in geomlist format and the number of internal
* geometries (non external) present in the list.
*
* @param geometrylist: the geometry in geomlist format (external after internal in a single vector).
* @param intgeocount: the number of internal geometries (non external) in the list.
* @param ownerT: indicates whether the GeoListModel takes ownership of the elements of the std::vector<T> (for pointers)
*/
explicit GeoListModel(std::vector<T> && geometrylist, int intgeocount, bool ownerT = false);
explicit GeoListModel(const std::vector<T> & geometrylist, int intgeocount, bool ownerT = false);
public:
/** @brief Destructor having type dependent behaviour
*
* @warning
* For GeoList, the destructor will destruct the Part::Geometry pointers * only * if it was constructed with ownerT = true.
*
* For GeoListFacade, the smart pointers will be deleted. However, a GeometryFacade does * not * delete the underlying naked pointers
* by default (which is mostly the desired behaviour as the ownership of the pointers belongs to sketchObject). If GeometryFacade is
* to delete the underlying naked pointers (because it is a temporal deep copy), then the GeometryFacade needs to get ownership (see
* setOwner method).
*
*/
~GeoListModel();
// Explicit deletion to show intent (not that it is needed)
// Explicit deletion to show intent (not that it is needed). This is a move only type.
GeoListModel(const GeoListModel &) = delete;
GeoListModel& operator=(const GeoListModel&) = delete;
// enable move syntaxis
// enable move constructor and move assignment. This is a move only type.
GeoListModel(GeoListModel &&) = default;
GeoListModel& operator=(GeoListModel&&) = default;
/**
/** @brief
* GeoListModel manages the lifetime of its internal std::vector. This means that while the actual ownership
* of the T parameter needs to be specified or separately handled, a new vector will be created and the T elements
* shallow copied to the internal vector.
* of the T parameter needs to be specified or separately handled. In the absence of that, a new vector will
* be created and the T elements shallow copied to the internal vector.
*
* The constness of the GeoListModel is tied to the constness of the std::vector from which it is constructed,
* except when the vector is not const, but the user uses the factory method to create a const model.
* The constness of the GeoListModel is tied to the constness of the std::vector from which it is constructed.
*
* @warning
* For GeoListFacade ownership at GeoListModel level cannot be taken (ownerT cannot be true). An assertion is raised
* if this happens. The ownership needs to be specified on the GeoListFacade objects themselves (setOwner method).
*/
static GeoListModel<T> getGeoListModel(std::vector<T> && geometrylist, int intgeocount, bool ownerT = false);
static const GeoListModel<T> getGeoListModel(const std::vector<T> & geometrylist, int intgeocount, bool ownerT = false);
/**
/** @brief
* returns the geometry given by the GeoId
*/
const T getGeometryFromGeoId(int geoId) const;
const Part::Geometry * getGeometryFromGeoId(int geoId) const;
/**
/** @brief returns a geometryfacade
* @warning If the underlying model of the list is a naked pointed (Part::Geometry *), i.e. a GeoList instantiation, the
* client (the user) bears responsibility for releasing the GeometryFacade pointer!!
*
* This is not a problem when the model of the list is a std::unique_ptr<Sketcher::GeometryFacade>, because the lifetime is tied to
* the GeometryFacade. It will destruct the pointer if it is the owner.
*/
const Sketcher::GeometryFacade * getGeometryFacadeFromGeoId(int geoId) const;
/** @brief
* returns the GeoId index from the index in the geometry in geomlist format with which it was constructed.
*
* @param index: the index of the list of geometry in geomlist format.
*/
int getGeoIdFromGeomListIndex(int index) const;
/**
/** @brief
* returns the geometry given by the GeoId in the geometrylist in geomlist format provided as a parameter.
*
* @param geometrylist: the geometry in geomlist format (external after internal in a single vector).
*
* @param index: the index of the list of geometry in geomlist format.
*/
static const T getGeometryFromGeoId(const std::vector<T> & geometrylist, int geoId);
static const Part::Geometry * getGeometryFromGeoId(const std::vector<T> & geometrylist, int geoId);
/** @brief returns a geometry facade
* @warning If the underlying model of the list is a naked pointed (Part::Geometry *), the client (the user) bears responsibility
* for releasing the GeometryFacade pointer!!
*
* This is not a problem when the model of the list is a std::unique_ptr<Sketcher::GeometryFacade>, because the lifetime is tied to
* the model itself.
*/
static const Sketcher::GeometryFacade * getGeometryFacadeFromGeoId(const std::vector<T> & geometrylist, int geoId);
/** @brief
* Obtain a GeoElementId class {GeoId, Pos} given a VertexId.
*
* A vertexId is a positive index of the vertex, where indices of external geometry taken higher positive values than normal geometry.
* It is the same format of vertex numbering used in the Sketcher, Sketch.cpp, and ViewProviderSketch.
*
*/
Sketcher::GeoElementId getGeoElementIdFromVertexId(int vertexId);
/** @brief
* Given an GeoElementId {GeoId, Pos}, it returns the index of the vertex in VertexId format.
*
* A vertexId is a positive index of the vertex, where indices of external geometry taken higher positive values than normal geometry.
* It is the same format of vertex numbering used in the Sketcher, Sketch.cpp, and ViewProviderSketch.
*
*/
int getVertexIdFromGeoElementId(const Sketcher::GeoElementId & geoelementId) const;
/** @brief
* Returns a point coordinates given {GeoId, Pos}.
*/
Vector3d getPoint(int geoId, Sketcher::PointPos pos) const;
/**
* returns the amount of internal geometry objects.
/** @brief
* Returns a point coordinates given GeoElementId {GeoId, Pos}.
*/
Vector3d getPoint(const GeoElementId & geid) const;
/** @brief
* returns the amount of internal (normal, non-external) geometry objects.
*/
int getInternalCount() const { return intGeoCount;}
/**
/** @brief
* returns the amount of external geometry objects.
*/
int getExternalCount() const { return int(geomlist.size()) - intGeoCount;}
/**
/** @brief
* return a reference to the internal geometry list vector.
*
* @warning { It returns a reference to the internal list vector. The validity of the
@@ -149,13 +219,21 @@ public:
private:
Vector3d getPoint(const Part::Geometry * geo, Sketcher::PointPos pos) const;
void rebuildVertexIndex() const;
private:
int intGeoCount;
bool OwnerT;
mutable bool indexInit;
mutable std::vector<Sketcher::GeoElementId> VertexId2GeoElementId; // these maps a lazy initialised on first demand.
mutable std::map<Sketcher::GeoElementId, int> GeoElementId2VertexId;
};
using GeoList = GeoListModel<Part::Geometry *>;
using GeoListFacade = GeoListModel<std::unique_ptr<const Sketcher::GeometryFacade>>;
using GeometryPtr = Part::Geometry *;
using GeometryFacadeUniquePtr = std::unique_ptr<const Sketcher::GeometryFacade>;
using GeoList = GeoListModel<GeometryPtr>;
using GeoListFacade = GeoListModel<GeometryFacadeUniquePtr>;
GeoListFacade getGeoListFacade(const GeoList & geolist);