Skip to content

Commit

Permalink
Sketcher: EditModeCoinManager/DrawSkechHandler refactoring
Browse files Browse the repository at this point in the history
======================================================

Creation of EditModeCoinManager class and helpers.

In a nutshell:
- EditModeCoinManager gets most of the content of struct EditData
- Drawing is partly outsourced to EditModeCoinManager
- EditModeCoinManager gets a nested Observer class to deal with parameters
- A struct DrawingParameters is created to store all parameters used for drawing
- EditModeCoinManager assume responsibility for determining the drawing size of the Axes
- Preselection detection responsibility is moved to EditModeCoinManager.
- Generation of constraint nodes and constraint drawing is moved to EditModeCoinManager.
- Constraint generation parameters are refactored into ConstraintParameters.
- Text rendering functions are moved to EditModeCoinManager.
- Move HDPI resolution responsibility from VPSketch to EditModeCoinManager
- Move responsibility to create the scenograph for edit mode to EditModeCoinManager
- Move full updateColor responsibility to EditModeCoinManager
- Allows for mapping N logical layers (LayerId of GeometryFacade) to M coin Layers (M<N). This
is convenient as, unless the representation must be different, there is no point in creating coin
layers (overhead).

Refactoring of geometry drawing:
- Determination of the curve values to draw are outsourced to OCC (SRP and remove code duplications).
- Refactor specific drawing of each geometry type into a single template method, based on classes of geometry.
- Drawing of geometry and constraints made agnostic of scale factors of BSpline weights so that a uniform treatment can be provided.

Refactoring of Overlay Layer:
- A new class EditModeInformationOverlayConverter is a full rewrite of the previous overlay routines.

ViewProviderSketch:
- Major cleanup due to migration of functionalities to EditModeCoinManager
- Reduce public api of ViewProviderSketch due to refactor of DrawSketchHandler
- Major addition of documentation
- ShortcutListener implementation using new ViewProvider Attorney
- Gets a parameter handling nested class to handle all parameters (observer)
- Move rubberband to smart pointer
- Refactor selection and preselection into nested classes
- Removal of SEL_PARAMS macro. This macro was making the code unreadable as it "captured" a local stringstream that appeared unused. Substituted by local private member functions.
- Remove EditData
- Improve documentation
- Refactor Preselection struct to remove magical numbers
- Refactor Selection mechanism to remove hacks

ViewProviderSketchDrawSketchHandlerAttorney:
- new Attorney to limit access to ViewProviderSketch and reduce its public interface
- In order to enforce a certain degree of encapsulation and promote a not too tight coupling, while still allowing well
defined collaboration, DrawSketchHandler accesses ViewProviderSketch via this Attorney class.
-DrawSketchHandler has the responsibility of drawing edit temporal curves and markers necessary to enable visual feedback
to the user, as well as the UI interaction during such edits. This is its exclusive responsibility under the Single
Responsibility Principle.
- A plethora of speciliased handlers derive from DrawSketchHandler for each specialised editing (see for example all the
handlers for creation of new geometry). These derived classes do * not * have direct access to the
ViewProviderSketchDrawSketchHandlerAttorney. This is intentional to keep coupling under control. However, generic
functionality requiring access to the Attorney can be implemented in DrawSketchHandler and used from its derived classes
by virtue of the inheritance. This promotes a concentrating the coupling in a single point (and code reuse).

EditModeCoinManager:
- Refactor of updateConstraintColor
- Multifield - new struct to identify a single element in a multifield field per layer
- Move geometry management to delegate class EditModeCoinGeometryManager
- Remove refactored code that was never used in the original ViewProviderSketch.

CommandSketcherBSpline:
- EditModeCoinManager automatically tracks parameter change and triggers the necessary redraw, rendering an explicit redraw obsolete and unnecessary.

Rebase on top of master:
- Commits added to master to ViewProviderSketch applied to EditModeCoinManager.
- Memory leaks - wmayer
- Constraint Diameter Symbol - OpenBrain
- Minor bugfix to display angle constraints - syres

Architecture Description
=======================

* Encapsulation and collaboration - restricting friendship - reducing public interface

Summary:
- DrawSketchHandler to ViewProviderSketch friendship regulated via attorney.
- ShortcutListener to ViewProviderSketch friendship regulated via attorney.
- EditModeCoinManager (new class) to ViewProviderSketch friendship regulated via attorney.
- ViewProviderSketch public interface is heavily reduced.

In further detail:
While access from ViewProviderSketch to other classes is regulated via their public interface, DrawSketchHandler, ShortcutListener and EditCoinManager (new class) access
to ViewProviderSketch non-public interface via attorneys. Previously, it was an unrestricted access (friend classes). Now this interface is restricted and regulated via attorneys.
This increases the encapsulation of ViewProviderSketch, reduces the coupling between classes and promotes an ordered growth. This I call the "collaboration interface".

At the same time, ViewProviderSketch substantially reduces its public interface. Access from Command draw handlers (deriving from DrawSketchHandler) is intended to be restricted to
the functionality exposed by DrawSketchHandler to its derived classes. However, this is still only partly enforced to keep the refactoring within limits. A further refactoring of
DrawSketchHandler and derivatives is for future discussion.

* Complexity and delegation

Summary:
- Complexity of coin node management is dealt with by delegation to helper classes and specialised objects.

In further detail:

ViewProviderSketch is halved in terms of code size. Higher level ViewProviderSketch functions remain

* Automatic update of parameters - Parameter observer nested classes

Summary:
- ViewProviderSketch and CoinManager get their own observer nested classes to monitor the parameters relevant to them and automatically update on change.

The split enables that each class deals only with parameters within their own responsibilities, effectively isolating the specifics and decoupling the implementations. It is
more convenient as there is no need to leave edit mode to update parameters. It is more compact as it leverages core code.

More information:
https://forum.freecadweb.org/viewtopic.php?p=553257#p553257
  • Loading branch information
abdullahtahiriyo committed Dec 27, 2021
1 parent b9ed85a commit 92e6094
Show file tree
Hide file tree
Showing 25 changed files with 8,122 additions and 5,079 deletions.
14 changes: 14 additions & 0 deletions src/Mod/Sketcher/Gui/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,20 @@ SET(SketcherGui_SRCS
TaskSketcherValidation.h
ShortcutListener.cpp
ShortcutListener.h
EditModeInformationOverlayCoinConverter.cpp
EditModeInformationOverlayCoinConverter.h
EditModeGeometryCoinConverter.cpp
EditModeGeometryCoinConverter.h
EditModeCoinManagerParameters.h
EditModeCoinManagerParameters.cpp
EditModeCoinManager.cpp
EditModeCoinManager.h
EditModeGeometryCoinManager.cpp
EditModeGeometryCoinManager.h
EditModeConstraintCoinManager.cpp
EditModeConstraintCoinManager.h
ViewProviderSketchCoinAttorney.cpp
ViewProviderSketchCoinAttorney.h
ViewProviderSketch.cpp
ViewProviderSketch.h
DrawSketchHandler.cpp
Expand Down
10 changes: 5 additions & 5 deletions src/Mod/Sketcher/Gui/CommandConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -740,9 +740,9 @@ class DrawSketchHandlerGenConstraint: public DrawSketchHandler
SelType newSelType = SelUnknown;

//For each SelType allowed, check if button is released there and assign it to selIdPair
int VtId = sketchgui->getPreselectPoint();
int CrvId = sketchgui->getPreselectCurve();
int CrsId = sketchgui->getPreselectCross();
int VtId = getPreselectPoint();
int CrvId = getPreselectCurve();
int CrsId = getPreselectCross();
if (allowedSelTypes & (SelRoot | SelVertexOrRoot) && CrsId == 0) {
selIdPair.GeoId = Sketcher::GeoEnum::RtPnt;
selIdPair.PosId = Sketcher::PointPos::start;
Expand Down Expand Up @@ -1809,8 +1809,8 @@ class DrawSketchHandlerCoincident: public DrawSketchHandler

virtual bool releaseButton(Base::Vector2d onSketchPos)
{
int VtId = sketchgui->getPreselectPoint();
int CrsId = sketchgui->getPreselectCross();
int VtId = getPreselectPoint();
int CrsId = getPreselectCross();
std::stringstream ss;
int GeoId_temp;
Sketcher::PointPos PosId_temp;
Expand Down
149 changes: 76 additions & 73 deletions src/Mod/Sketcher/Gui/CommandCreateGeo.cpp

Large diffs are not rendered by default.

23 changes: 6 additions & 17 deletions src/Mod/Sketcher/Gui/CommandSketcherBSpline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,11 @@ void ActivateBSplineHandler(Gui::Document *doc,DrawSketchHandler *handler)
}
}

void ShowRestoreInformationLayer(SketcherGui::ViewProviderSketch* vp, char * visibleelementname)
void ShowRestoreInformationLayer(const char * visibleelementname)
{
ParameterGrp::handle hGrp = App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/Mod/Sketcher/General");
bool status = hGrp->GetBool(visibleelementname, true);
hGrp->SetBool(visibleelementname, !status);
vp->showRestoreInformationLayer();
}

// Show/Hide B-spline degree
Expand All @@ -111,9 +110,7 @@ void CmdSketcherBSplineDegree::activated(int iMsg)
{
Q_UNUSED(iMsg);

Gui::Document * doc = getActiveGuiDocument();
SketcherGui::ViewProviderSketch* vp = static_cast<SketcherGui::ViewProviderSketch*>(doc->getInEdit());
ShowRestoreInformationLayer(vp, "BSplineDegreeVisible");
ShowRestoreInformationLayer("BSplineDegreeVisible");
}

bool CmdSketcherBSplineDegree::isActive(void)
Expand Down Expand Up @@ -142,9 +139,7 @@ void CmdSketcherBSplinePolygon::activated(int iMsg)
{
Q_UNUSED(iMsg);

Gui::Document * doc = getActiveGuiDocument();
SketcherGui::ViewProviderSketch* vp = static_cast<SketcherGui::ViewProviderSketch*>(doc->getInEdit());
ShowRestoreInformationLayer(vp, "BSplineControlPolygonVisible");
ShowRestoreInformationLayer("BSplineControlPolygonVisible");
}

bool CmdSketcherBSplinePolygon::isActive(void)
Expand Down Expand Up @@ -173,9 +168,7 @@ void CmdSketcherBSplineComb::activated(int iMsg)
{
Q_UNUSED(iMsg);

Gui::Document * doc = getActiveGuiDocument();
SketcherGui::ViewProviderSketch* vp = static_cast<SketcherGui::ViewProviderSketch*>(doc->getInEdit());
ShowRestoreInformationLayer(vp, "BSplineCombVisible");
ShowRestoreInformationLayer("BSplineCombVisible");
}

bool CmdSketcherBSplineComb::isActive(void)
Expand Down Expand Up @@ -204,9 +197,7 @@ void CmdSketcherBSplineKnotMultiplicity::activated(int iMsg)
{
Q_UNUSED(iMsg);

Gui::Document * doc = getActiveGuiDocument();
SketcherGui::ViewProviderSketch* vp = static_cast<SketcherGui::ViewProviderSketch*>(doc->getInEdit());
ShowRestoreInformationLayer(vp, "BSplineKnotMultiplicityVisible");
ShowRestoreInformationLayer("BSplineKnotMultiplicityVisible");
}

bool CmdSketcherBSplineKnotMultiplicity::isActive(void)
Expand Down Expand Up @@ -235,9 +226,7 @@ void CmdSketcherBSplinePoleWeight::activated(int iMsg)
{
Q_UNUSED(iMsg);

Gui::Document* doc = getActiveGuiDocument();
SketcherGui::ViewProviderSketch* vp = static_cast<SketcherGui::ViewProviderSketch*>(doc->getInEdit());
ShowRestoreInformationLayer(vp, "BSplinePoleWeightVisible");
ShowRestoreInformationLayer("BSplinePoleWeightVisible");
}

bool CmdSketcherBSplinePoleWeight::isActive(void)
Expand Down
12 changes: 6 additions & 6 deletions src/Mod/Sketcher/Gui/CommandSketcherTools.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,7 @@ class DrawSketchHandlerCopy: public DrawSketchHandler
setPositionText(endpoint, text);

EditCurve[1] = endpoint;
sketchgui->drawEdit(EditCurve);
drawEdit(EditCurve);
if (seekAutoConstraint(sugConstr1, endpoint, Base::Vector2d(0.0, 0.0), AutoConstraint::VERTEX)) {
renderSuggestConstraintsCursor(sugConstr1);
return;
Expand All @@ -1362,7 +1362,7 @@ class DrawSketchHandlerCopy: public DrawSketchHandler
virtual bool pressButton(Base::Vector2d)
{
if (Mode == STATUS_SEEK_First) {
sketchgui->drawEdit(EditCurve);
drawEdit(EditCurve);
Mode = STATUS_End;
}
return true;
Expand Down Expand Up @@ -1415,7 +1415,7 @@ class DrawSketchHandlerCopy: public DrawSketchHandler

tryAutoRecomputeIfNotSolve(static_cast<Sketcher::SketchObject *>(sketchgui->getObject()));
EditCurve.clear();
sketchgui->drawEdit(EditCurve);
drawEdit(EditCurve);

// no code after this line, Handler gets deleted in ViewProvider
sketchgui->purgeHandler();
Expand Down Expand Up @@ -1899,7 +1899,7 @@ class DrawSketchHandlerRectangularArray: public DrawSketchHandler
setPositionText(endpoint, text);

EditCurve[1] = endpoint;
sketchgui->drawEdit(EditCurve);
drawEdit(EditCurve);
if (seekAutoConstraint(sugConstr1, endpoint, Base::Vector2d(0.0, 0.0), AutoConstraint::VERTEX))
{
renderSuggestConstraintsCursor(sugConstr1);
Expand All @@ -1913,7 +1913,7 @@ class DrawSketchHandlerRectangularArray: public DrawSketchHandler
virtual bool pressButton(Base::Vector2d)
{
if (Mode == STATUS_SEEK_First) {
sketchgui->drawEdit(EditCurve);
drawEdit(EditCurve);
Mode = STATUS_End;
}
return true;
Expand Down Expand Up @@ -1952,7 +1952,7 @@ class DrawSketchHandlerRectangularArray: public DrawSketchHandler
tryAutoRecomputeIfNotSolve(static_cast<Sketcher::SketchObject *>(sketchgui->getObject()));

EditCurve.clear();
sketchgui->drawEdit(EditCurve);
drawEdit(EditCurve);

// no code after this line, Handler is deleted in ViewProvider
sketchgui->purgeHandler();
Expand Down
96 changes: 88 additions & 8 deletions src/Mod/Sketcher/Gui/DrawSketchHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,56 @@ using namespace SketcherGui;
using namespace Sketcher;


/************************************ Attorney *******************************************/

inline void ViewProviderSketchDrawSketchHandlerAttorney::setPositionText(ViewProviderSketch &vp, const Base::Vector2d &Pos, const SbString &txt)
{
vp.setPositionText(Pos,txt);
}

inline void ViewProviderSketchDrawSketchHandlerAttorney::setPositionText(ViewProviderSketch &vp, const Base::Vector2d &Pos)
{
vp.setPositionText(Pos);
}

inline void ViewProviderSketchDrawSketchHandlerAttorney::resetPositionText(ViewProviderSketch &vp)
{
vp.resetPositionText();
}

inline void ViewProviderSketchDrawSketchHandlerAttorney::drawEdit(ViewProviderSketch &vp, const std::vector<Base::Vector2d> &EditCurve)
{
vp.drawEdit(EditCurve);
}

inline void ViewProviderSketchDrawSketchHandlerAttorney::drawEditMarkers(ViewProviderSketch &vp, const std::vector<Base::Vector2d> &EditMarkers, unsigned int augmentationlevel)
{
vp.drawEditMarkers(EditMarkers, augmentationlevel);
}

inline void ViewProviderSketchDrawSketchHandlerAttorney::setAxisPickStyle(ViewProviderSketch &vp, bool on)
{
vp.setAxisPickStyle(on);
}

inline int ViewProviderSketchDrawSketchHandlerAttorney::getPreselectPoint(const ViewProviderSketch &vp)
{
return vp.getPreselectPoint();
}

inline int ViewProviderSketchDrawSketchHandlerAttorney::getPreselectCurve(const ViewProviderSketch &vp)
{
return vp.getPreselectCurve();
}

inline int ViewProviderSketchDrawSketchHandlerAttorney::getPreselectCross(const ViewProviderSketch &vp)
{
return vp.getPreselectCross();
}

/**************************** DrawSketchHandler *******************************************/


//**************************************************************************
// Construction/Destruction

Expand All @@ -69,8 +119,8 @@ DrawSketchHandler::~DrawSketchHandler() {}
void DrawSketchHandler::quit(void)
{
assert(sketchgui);
sketchgui->drawEdit(std::vector<Base::Vector2d>());
sketchgui->drawEditMarkers(std::vector<Base::Vector2d>());
drawEdit(std::vector<Base::Vector2d>());
drawEditMarkers(std::vector<Base::Vector2d>());
resetPositionText();

Gui::Selection().rmvSelectionGate();
Expand Down Expand Up @@ -314,9 +364,9 @@ int DrawSketchHandler::seekAutoConstraint(std::vector<AutoConstraint> &suggested
Base::Vector3d hitShapeDir = Base::Vector3d(0,0,0); // direction of hit shape (if it is a line, the direction of the line)

// Get Preselection
int preSelPnt = sketchgui->getPreselectPoint();
int preSelCrv = sketchgui->getPreselectCurve();
int preSelCrs = sketchgui->getPreselectCross();
int preSelPnt = getPreselectPoint();
int preSelCrv = getPreselectCurve();
int preSelCrs = getPreselectCross();
int GeoId = GeoEnum::GeoUndef;

Sketcher::PointPos PosId = Sketcher::PointPos::none;
Expand Down Expand Up @@ -691,16 +741,46 @@ void DrawSketchHandler::renderSuggestConstraintsCursor(std::vector<AutoConstrain

void DrawSketchHandler::setPositionText(const Base::Vector2d &Pos, const SbString &text)
{
sketchgui->setPositionText(Pos, text);
ViewProviderSketchDrawSketchHandlerAttorney::setPositionText(*sketchgui, Pos, text);
}


void DrawSketchHandler::setPositionText(const Base::Vector2d &Pos)
{
sketchgui->setPositionText(Pos);
ViewProviderSketchDrawSketchHandlerAttorney::setPositionText(*sketchgui,Pos);
}

void DrawSketchHandler::resetPositionText(void)
{
sketchgui->resetPositionText();
ViewProviderSketchDrawSketchHandlerAttorney::resetPositionText(*sketchgui);
}

void DrawSketchHandler::drawEdit(const std::vector<Base::Vector2d> &EditCurve)
{
ViewProviderSketchDrawSketchHandlerAttorney::drawEdit(*sketchgui, EditCurve);
}

void DrawSketchHandler::drawEditMarkers(const std::vector<Base::Vector2d> &EditMarkers, unsigned int augmentationlevel)
{
ViewProviderSketchDrawSketchHandlerAttorney::drawEditMarkers(*sketchgui, EditMarkers, augmentationlevel);
}

void DrawSketchHandler::setAxisPickStyle(bool on)
{
ViewProviderSketchDrawSketchHandlerAttorney::setAxisPickStyle(*sketchgui, on);
}

int DrawSketchHandler::getPreselectPoint(void) const
{
return ViewProviderSketchDrawSketchHandlerAttorney::getPreselectPoint(*sketchgui);
}

int DrawSketchHandler::getPreselectCurve(void) const
{
return ViewProviderSketchDrawSketchHandlerAttorney::getPreselectCurve(*sketchgui);
}

int DrawSketchHandler::getPreselectCross(void) const
{
return ViewProviderSketchDrawSketchHandlerAttorney::getPreselectCross(*sketchgui);
}
44 changes: 43 additions & 1 deletion src/Mod/Sketcher/Gui/DrawSketchHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,28 @@ namespace SketcherGui {

class ViewProviderSketch;

/**
* In order to enforce a certain degree of encapsulation and promote a not
* too tight coupling, while still allowing well defined collaboration,
* DrawSketchHandler accesses ViewProviderSketch via this Attorney class
*/
class ViewProviderSketchDrawSketchHandlerAttorney {
private:
static inline void setPositionText(ViewProviderSketch &vp, const Base::Vector2d &Pos, const SbString &txt);
static inline void setPositionText(ViewProviderSketch &vp, const Base::Vector2d &Pos);
static inline void resetPositionText(ViewProviderSketch &vp);
static inline void drawEdit(ViewProviderSketch &vp, const std::vector<Base::Vector2d> &EditCurve);
static inline void drawEditMarkers(ViewProviderSketch &vp, const std::vector<Base::Vector2d> &EditMarkers, unsigned int augmentationlevel = 0);
static inline void setAxisPickStyle(ViewProviderSketch &vp, bool on);

static inline int getPreselectPoint(const ViewProviderSketch &vp);
static inline int getPreselectCurve(const ViewProviderSketch &vp);
static inline int getPreselectCross(const ViewProviderSketch &vp);

friend class DrawSketchHandler;
};


// A Simple data type to hold basic information for suggested constraints
struct AutoConstraint
{
Expand All @@ -56,6 +78,17 @@ struct AutoConstraint
/** Handler to create new sketch geometry
* This class has to be reimplemented to create geometry in the
* sketcher while its in editing.
*
* DrawSketchHandler takes over the responsibility of drawing edit temporal curves and
* markers necessary to enable visual feedback to the user, as well as the UI interaction during
* such edits. This is its exclusive responsibility under the Single Responsibility Principle.
*
* A plethora of speciliased handlers derive from DrawSketchHandler for each specialised editing (see
* for example all the handlers for creation of new geometry). These derived classes do * not * have
* direct access to the ViewProviderSketchDrawSketchHandlerAttorney. This is intended to keep coupling
* under control. However, generic functionality requiring access to the Attorney can be implemented
* in DrawSketchHandler and used from its derived classes by virtue of the inheritance. This promotes a
* concentrating the coupling in a single point (and code reuse).
*/
class SketcherGuiExport DrawSketchHandler
{
Expand Down Expand Up @@ -98,7 +131,7 @@ class SketcherGuiExport DrawSketchHandler
/**
* Sets a cursor for 3D inventor view.
* pixmap as a cursor image in device independent pixels.
*
*
* \param autoScale - set this to false if pixmap already scaled for HiDPI
**/
void setCursor(const QPixmap &pixmap, int x,int y, bool autoScale=true);
Expand All @@ -113,6 +146,15 @@ class SketcherGuiExport DrawSketchHandler
qreal devicePixelRatio();
void setCrosshairCursor(const char* svgName);

void drawEdit(const std::vector<Base::Vector2d> &EditCurve);
void drawEditMarkers(const std::vector<Base::Vector2d> &EditMarkers, unsigned int augmentationlevel = 0);
void setAxisPickStyle(bool on);

int getPreselectPoint(void) const;
int getPreselectCurve(void) const;
int getPreselectCross(void) const;


/**
* Returns constraints icons scaled to width.
**/
Expand Down
Loading

0 comments on commit 92e6094

Please sign in to comment.