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

Scaled cylinder yields invalid shape #615

Closed
aothms opened this issue May 1, 2016 · 4 comments
Closed

Scaled cylinder yields invalid shape #615

aothms opened this issue May 1, 2016 · 4 comments

Comments

@aothms
Copy link

aothms commented May 1, 2016

Hi all,

Please find attached a trivial example program that yields an invalid shape by using TopoDS_Shape::Move() on a shape with a Geom_CylindricalSurface and a gp_Trsf with scale factor.

#include <gp_Trsf.hxx>
#include <Geom_Circle.hxx>

#include <BRepBuilderAPI_MakeWire.hxx>
#include <BRepBuilderAPI_MakeEdge.hxx>
#include <BRepBuilderAPI_MakeFace.hxx>
#include <BRepBuilderAPI_Transform.hxx>

#include <BRepPrimAPI_MakePrism.hxx>

#include <TopoDS_Iterator.hxx>

#include <BRepCheck.hxx>
#include <BRepCheck_Result.hxx>
#include <BRepCheck_Analyzer.hxx>
#include <BRepCheck_ListOfStatus.hxx>
#include <BRepCheck_ListIteratorOfListOfStatus.hxx>

#include <BRepTools.hxx>

int main(int argc, char** argv) {
    Handle(Geom_Circle) circle = new Geom_Circle(gp_Ax2(), 400.);
    BRepBuilderAPI_MakeWire wire;
    wire.Add(BRepBuilderAPI_MakeEdge(circle));
    TopoDS_Shape cylinder = BRepPrimAPI_MakePrism(BRepBuilderAPI_MakeFace(wire, false), gp_Vec(0, 0, 4000)).Shape();

    {
        BRepCheck_Analyzer valid(cylinder);
        bool validity = valid.IsValid();
        std::cout << validity << std::endl;
    }

    gp_Trsf trsf;
    trsf.SetScaleFactor(0.1);
    TopoDS_Shape scaled_cylinder = cylinder.Moved(trsf);

    {
        BRepCheck_Analyzer valid(scaled_cylinder);
        bool validity = valid.IsValid(); 
        std::cout << validity << std::endl;
    }

    scaled_cylinder = BRepBuilderAPI_Transform(cylinder, trsf);

    {
        BRepCheck_Analyzer valid(scaled_cylinder);
        bool validity = valid.IsValid();
        std::cout << validity << std::endl;
    }
}

Results in:

1
0
1

(in other words, the unscaled cylinder is valid, the scaled cylinder with TopoDS_Shape::Move() is not, BRepBuilderAPI_Transform does yield a valid shape on the same operands)

I did some debugging and it boils down to

  1. a BRepCheck_UnorientableShape being issued for the 'cylindrical' TopoDS_Face
  2. This is due to a BRepCheck_InvalidCurveOnSurface being issued for one of its edges
    ste == BRepCheck_InvalidCurveOnSurface ||
  3. This is due to the distance between points on the pcurve and 3d curve being a factor 10 away from each other in the Z-axis. Which seems to indicate the somehow the transformation does get applied to only one of them.
    if (pref.SquareDistance(pother) > Tol2) {

I'm in way over my head here. Appreciate some help. Thanks all.

@a-betenev
Copy link
Contributor

Hello Thomas,

Unfortunately, transformations with non-unit scale factor are not
supported in shape locations.
The reason is that elementary surfaces and curves (such as line or
cylinder) in OCCT are parametrized 'by length'.
When you apply scale factor, parametrization changes, and all the data
that refer to it need to be recomputed (such as p-curves on surfaces).
Though this potentially could be accounted for, it is not done in
practice (would be overly complex).
Thus, just do not use non-unit scales in this way (use Transform to get
new shape instead).

Andrey

On 01.05.2016 14:16, Thomas Krijnen wrote:

Hi all,

Please find attached a trivial example program that yields an invalid
shape by using |TopoDS_Shape::Move()| on a shape with a
|Geom_CylindricalSurface| and a |gp_Trsf| with scale factor.

|#include <gp_Trsf.hxx> #include <Geom_Circle.hxx> #include
<BRepBuilderAPI_MakeWire.hxx> #include <BRepBuilderAPI_MakeEdge.hxx>
#include <BRepBuilderAPI_MakeFace.hxx> #include
<BRepBuilderAPI_Transform.hxx> #include <BRepPrimAPI_MakePrism.hxx>
#include <TopoDS_Iterator.hxx> #include <BRepCheck.hxx> #include
<BRepCheck_Result.hxx> #include <BRepCheck_Analyzer.hxx> #include
<BRepCheck_ListOfStatus.hxx> #include
<BRepCheck_ListIteratorOfListOfStatus.hxx> #include <BRepTools.hxx>
int main(int argc, char** argv) { Handle(Geom_Circle) circle = new
Geom_Circle(gp_Ax2(), 400.); BRepBuilderAPI_MakeWire wire;
wire.Add(BRepBuilderAPI_MakeEdge(circle)); TopoDS_Shape cylinder =
BRepPrimAPI_MakePrism(BRepBuilderAPI_MakeFace(wire, false), gp_Vec(0,
0, 4000)).Shape(); { BRepCheck_Analyzer valid(cylinder); bool validity
= valid.IsValid(); std::cout << validity << std::endl; } gp_Trsf trsf;
trsf.SetScaleFactor(0.1); TopoDS_Shape scaled_cylinder =
cylinder.Moved(trsf); { BRepCheck_Analyzer valid(scaled_cylinder);
bool validity = valid.IsValid(); std::cout << validity << std::endl; }
scaled_cylinder = BRepBuilderAPI_Transform(cylinder, trsf); {
BRepCheck_Analyzer valid(scaled_cylinder); bool validity =
valid.IsValid(); std::cout << validity << std::endl; } } |

Results in:

|1 0 1 |

(in other words, the unscaled cylinder is valid, the scaled cylinder
with |TopoDS_Shape::Move()| is not, |BRepBuilderAPI_Transform| does
yield a valid shape on the same operands)

I did some debugging and it boils down to

  1. a |BRepCheck_UnorientableShape| being issued for the 'cylindrical'
    |TopoDS_Face|
  2. This is due to a |BRepCheck_InvalidCurveOnSurface| being issued
    for one of its edges
    ste == BRepCheck_InvalidCurveOnSurface ||
  3. This is due to the distance between points on the pcurve and 3d
    curve being a factor 10 away from each other in the Z-axis. Which
    seems to indicate the somehow the transformation does get applied
    to only one of them.
    if (pref.SquareDistance(pother) > Tol2) {

I'm in way over my head here. Appreciate some help. Thanks all.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#615

@aothms
Copy link
Author

aothms commented May 1, 2016

Dear Andrey,

Thanks for clarifying this. I wasn't aware of this, although admittedly the fact that the method is called Move() should have made me think longer. Maybe for future ignorant users like me it might be good to throw an exception when a location with gp_Trsf::ScaleFactor() != 1. is encountered, but I'm sure you have your reasons. Thanks again!

Kind regards,
Thomas

@a-betenev
Copy link
Contributor

Dear Thomas,

I agree, now recorded as issue in OCCT Mantis:
http://tracker.dev.opencascade.org/view.php?id=27457

Best Regards,
Andrey

On 01.05.2016 15:00, Thomas Krijnen wrote:

Dear Andrey,

Thanks for clarifying this. I wasn't aware of this, although
admittedly the fact that the method is called |Move()| should have
made me think longer. Maybe for future ignorant users like me it might
be good to throw an exception when a location with
|gp_Trsf::ScaleFactor() != 1.| is encountered, but I'm sure you have
your reasons. Thanks again!

Kind regards,
Thomas


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#615 (comment)

@aothms
Copy link
Author

aothms commented May 1, 2016

Dear Andrey,

Great! Thanks.

Kind regards,
Thomas

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

No branches or pull requests

2 participants