[Error] Sketcher - tangency constrain

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
User avatar
paddle
Veteran
Posts: 1364
Joined: Mon Feb 03, 2020 4:47 pm

[Error] Sketcher - tangency constrain

Post by paddle »

Hi,

CommandConstraints.cpp
Tangency constrain

Code: Select all

void CmdSketcherConstrainTangent::applyConstraint(std::vector<SelIdPair> &selSeq, int seqIndex)
{
    .....

    int GeoId1 = GeoEnum::GeoUndef, GeoId2 = GeoEnum::GeoUndef, GeoId3 = GeoEnum::GeoUndef;
    Sketcher::PointPos PosId1 = Sketcher::PointPos::none, PosId2 = Sketcher::PointPos::none, PosId3 = Sketcher::PointPos::none;

    // check if the edge already has a Block constraint
    if ( areBothPointsOrSegmentsFixed(Obj,GeoId1,GeoId2) ) {
      showNoConstraintBetweenFixedGeometry();
      return;
    }

    switch (seqIndex) {
    case 0: // {SelEdge, SelEdgeOrAxis}
    case 1: // {SelEdgeOrAxis, SelEdge}
    case 2: // {SelEdge, SelExternalEdge}
    case 3: // {SelExternalEdge, SelEdge}
    {
        GeoId1 = selSeq.at(0).GeoId; GeoId2 = selSeq.at(1).GeoId;
        .....
areBothPointsOrSegmentsFixed(Obj,GeoId1,GeoId2) is called when GeoId1 and GeoId2 are GeoUndef. So this IF is useless.
I don't know if this IF has some use but the misplacement might cause some bad behaviour?
chrisb
Veteran
Posts: 53786
Joined: Tue Mar 17, 2015 9:14 am

Re: [Error] Sketcher - tangency constrain

Post by chrisb »

I don't know if Abdullah is scanning this forum, let's ping him:
Abdullah wrote:ping
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
openBrain
Veteran
Posts: 9031
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: [Error] Sketcher - tangency constrain

Post by openBrain »

Actually the check can just be removed because the selection mechanism already prevents selecting 2 external edges.
Do you want me to proceed and push a PR ?
User avatar
paddle
Veteran
Posts: 1364
Joined: Mon Feb 03, 2020 4:47 pm

Re: [Error] Sketcher - tangency constrain

Post by paddle »

openBrain wrote: Thu Jan 20, 2022 2:47 pm Actually the check can just be removed because the selection mechanism already prevents selecting 2 external edges.
Do you want me to proceed and push a PR ?
Yes please go ahead !
User avatar
adrianinsaval
Veteran
Posts: 5534
Joined: Thu Apr 05, 2018 5:15 pm

Re: [Error] Sketcher - tangency constrain

Post by adrianinsaval »

openBrain wrote: Thu Jan 20, 2022 2:47 pm Actually the check can just be removed because the selection mechanism already prevents selecting 2 external edges.
Do you want me to proceed and push a PR ?
I won't pretend to understand sketcher code but are you sure it's not needed? The comment mentions the block constraint, is this how external geometry are called in the code? or is this if relevant for the (GUI) block constraint?

Edit: checked the code https://github.com/FreeCAD/FreeCAD/blob ... 4108-L4116

Code: Select all

getIdsFromName(SubNames[0], Obj, GeoId1, PosId1);
I have very little C++ knowledge but I thought you had to pass pointers as arguments if you wanted the called function to modify the values of the variables so this is confusing me, it seems here the values for GeoId are obtained with this function by passing them as arguments.

Anyways, so CmdSketcherConstrainTangent::activated already checks for "blocked" geometries (external, block constrained and bspline knots) it's then safe to assume this portion of code will always be executed before CmdSketcherConstrainTangent::applyConstraint so no check is needed inside that, is this correct?
Might be worth looking at how the rest of the constraints are doing this check to see if they have similar errors.
User avatar
paddle
Veteran
Posts: 1364
Joined: Mon Feb 03, 2020 4:47 pm

Re: [Error] Sketcher - tangency constrain

Post by paddle »

openBrain wrote: Thu Jan 20, 2022 2:47 pm Actually the check can just be removed because the selection mechanism already prevents selecting 2 external edges.
Do you want me to proceed and push a PR ?
The same is at CmdSketcherConstrainPerpendicular::applyConstraint
openBrain
Veteran
Posts: 9031
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: [Error] Sketcher - tangency constrain

Post by openBrain »

adrianinsaval wrote: Thu Jan 20, 2022 3:25 pm I won't pretend to understand sketcher code but are you sure it's not needed? The comment mentions the block constraint, is this how external geometry are called in the code? or is this if relevant for the (GUI) block constraint?
A naked eye is sometime needed. :) You're totally right. There is no issue deleting the check regarding external geometries, but Block constraints aren't handled by the selection and still need to be checked. I'll have a deeper look.
https://github.com/FreeCAD/FreeCAD/blob ... 4108-L4116

Code: Select all

getIdsFromName(SubNames[0], Obj, GeoId1, PosId1);
I have very little C++ knowledge but I thought you had to pass pointers as arguments if you wanted the called function to modify the values of the variables so this is confusing me, it seems here the values for GeoId are obtained with this function by passing them as arguments.
C++ (compared to C) not only has pointer to reference a variable, but also so called 'reference' (which roughly is a non-null pointer to a known type variable). It is denoted with an ampersand '&' in the function prototype but right that when using it, you hardly see the difference with a 'by value' argument. See function prototype :

Code: Select all

void getIdsFromName(const std::string &name, const Sketcher::SketchObject* Obj, int &GeoId, Sketcher::PointPos &PosId);
User avatar
adrianinsaval
Veteran
Posts: 5534
Joined: Thu Apr 05, 2018 5:15 pm

Re: [Error] Sketcher - tangency constrain

Post by adrianinsaval »

thanks for the explanation! As you have probably guessed I learned some C years ago and was told C++ added a bunch of fancy things but was essentially the same, not sure to what extent that's true but many of my assumptions still come from (also limited) C knowledge
openBrain
Veteran
Posts: 9031
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: [Error] Sketcher - tangency constrain

Post by openBrain »

https://github.com/FreeCAD/FreeCAD/pull/5402

EDIT : Doing so I saw that Perpendicular constraint doesn't support the vertex/vertex mode in continuous mode.
Will fix it when above PR is merged. ;)
chrisb
Veteran
Posts: 53786
Joined: Tue Mar 17, 2015 9:14 am

Re: [Error] Sketcher - tangency constrain

Post by chrisb »

openBrain wrote: Thu Jan 20, 2022 4:54 pm Doing so I saw that Perpendicular constraint doesn't support the vertex/vertex mode in continuous mode.
That sheds some light on the usage of this mode, because I think that is the most frequently used variant of the perpendicular constraint. Or perhaps this mode is used only by the smart guys, who know about Polyline and the power of the Mystery M.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
Post Reply