Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bspline stage1b 2017 thirddeliverable 2 #664

Conversation

abdullahtahiriyo
Copy link
Contributor

Just rebased to run mergability and test and checks.

Outstanding issues in thread:
https://forum.freecadweb.org/viewtopic.php?f=10&t=9364&start=380

I would gladly add a commit to fix that in the error messages, FreeCAD exception thrown disappears:
https://forum.freecadweb.org/viewtopic.php?f=10&t=9364&sid=93fefc161a6a2c17cc73d4a1b11608b1&start=370#p168075

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

Very important remark:

Until now, points could not be construction = true.

For now on, if a point is construction = true, the solver will ignore it.
Internal geometry knot points, which were added as fixed parameters to the solver according to a previous commit, are tracked in the corresponding bspline as solver level,
without being a parameter to the solver, and upon solving, the position thereof is updated by means of OCC functionality.

This allows to show the knot points and solidarily move them when moving a bspline.
=======================================================================

bool checkBothExternal(int GeoId1, int GeoId2);
void getIdsFromName(const std::string &name, const Sketcher::SketchObject* Obj, int &GeoId, Sketcher::PointPos &PosId);
bool inline isVertex(int GeoId, Sketcher::PointPos PosId);
bool inline isEdge(int GeoId, Sketcher::PointPos PosId);
bool isSimpleVertex(const Sketcher::SketchObject* Obj, int GeoId, Sketcher::PointPos PosId);
bool IsPointAlreadyOnCurve(int GeoIdCurve, int GeoIdPoint, Sketcher::PointPos PosIdPoint, Sketcher::SketchObject* Obj);
…e any potential index miscalculation and memory overflow
===============================================

New convenience for the user. If the first pole is radius length constraint, then any newly exposed internal geometry is created with equality constraint to the first pole.

This is really convenient in the case the user is working with polynomic bsplines, as any operation involving losing/gaining a pole (for example increase in multiplicity of a knot).
@@ -217,7 +217,8 @@ Py::Boolean GeometryPy::getConstruction(void) const

void GeometryPy::setConstruction(Py::Boolean arg)
{
getGeometryPtr()->Construction = arg;
if(getGeometryPtr()->getClassTypeId() != Part::GeomPoint::getClassTypeId())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it not allowed to set the construction flag for a point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is under epigraph: a) Modify the api so that no unintended use is made.

If I am not wrong (I think I remember it from an old discussion), there is no point in making a point of type construction. If I am wrong and generally in FreeCAD it is useful to make a point construction=true. Then this shall not be merged AND the sketcher code will work under the other epigraph: b) "document so that developers are aware of the consequences of setting a point of type construction".

This BSpline deliverable introduces a fourth type of geometry type of sketcher geometry (other than normal, construction, external), the "construction point" (making use of the perceived idea that a construction point did not make sense in FreeCAD). A construction point is programmability created to indicate the solver that such point has fixed parameters and that it shall not try to move it. The idea behind this is that very complex geometry uses OCC functions for handling this geometry outside the solver.

I do not have strong feelings about this commit, if you decide towards a) we merged if you decide for b) I document better the issue in the code. Just let me know.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is under epigraph: a) Modify the api so that no unintended use is made.

This is again only true in the context of the sketcher. But there might be examples where it is useful (although I can't tell you one at the moment). So, I wouldn't restrict this inside the Geometry class but only there where it's needed, i.e. in the sketcher code.

If I am not wrong (I think I remember it from an old discussion), there is no point in making a point of type construction.

Yes, in a sketch I don't know of an example where it would make sense to have a point as real geometry. If it's a "free" point I guess you can't create a wire from it and if it's a "bound" point then it's actually obsolete because the curve's points can be used instead.

@wwmayer
Copy link
Contributor

wwmayer commented Apr 8, 2017

Merged.

@wwmayer wwmayer closed this Apr 8, 2017
@abdullahtahiriyo abdullahtahiriyo deleted the bspline_stage1b_2017_thirddeliverable_2 branch November 3, 2020 07:15
kwahoo2 pushed a commit to kwahoo2/FreeCAD that referenced this pull request Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants