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

[Problem] Shape.transformGeometry(matrix) fails on OCC 7.7.x #9651

Open
2 tasks done
Jolbas opened this issue May 24, 2023 · 21 comments
Open
2 tasks done

[Problem] Shape.transformGeometry(matrix) fails on OCC 7.7.x #9651

Jolbas opened this issue May 24, 2023 · 21 comments

Comments

@Jolbas
Copy link
Contributor

Jolbas commented May 24, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Version

0.21 (Development)

Full version info

OS: macOS 12.6.3
Word size of FreeCAD: 64-bit
Version: 0.21.0.33263 (Git)
Build type: Release
Branch: master
Hash: 864f99bf984c237ef38d9451fd6f3a5ec34e6eb4
Python 3.11.3, Qt 5.15.8, Coin 4.0.0, Vtk 9.2.2, OCC 7.7.1
Locale: C/Default (C)
Installed mods: 
  * kugghjul.py
  * dxf-library
  * fcgear 1.0.0
  * __pycache__
  * sheetmetal 0.2.61
  * tida.py
  * Curves 0.6.8
  * Assembly4 0.12.7

Subproject(s) affected?

None

Problem description

Running this python code:

obj = App.ActiveDocument.addObject("Part::Box","Box")
obj.recompute()
obj.Shape.transformGeometry(App.Matrix())

Gives this error in python console but nothing in report view:
<class 'Part.OCCError'>: gp_GTrsf::Trsf() - non-orthogonal GTrsf

The error originates from https://github.com/Open-Cascade-SAS/OCCT/blob/5d8b1a4076398556095c71c093ca517cde77d4d8/src/gp/gp_GTrsf.hxx#L446 in OCCT, which is called from https://github.com/Open-Cascade-SAS/OCCT/blob/5d8b1a4076398556095c71c093ca517cde77d4d8/src/BRepTools/BRepTools_GTrsfModification.cxx#L314
It seems like an OCCT problem but I'm not sure how to proceed. From other posts in forum here it seems to only affect OCCT 7.7

Anything else?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@adrianinsaval
Copy link
Member

where in our code are we calling this? perhaps there's new parameters that we need to pass or we need to move to a different occt function.
@FreeCAD/maintainers shall we target this for 0.21? I vote yes, even if we stick to 7.6 for the release it would be good to have it compatible with 7.7 IMO

@Jolbas
Copy link
Contributor Author

Jolbas commented May 24, 2023

@adrianinsaval
Copy link
Member

maybe we need to call mat.setForm() before making the transformation?

@Jolbas
Copy link
Contributor Author

Jolbas commented May 24, 2023

it doesn't help. It is changed back to non-orthogonal later in OCC. Also it has been and should be able to handle non-orthogonal matrices too.

@adrianinsaval
Copy link
Member

The line to throw an error if it was gp_Other was already there in 7.6, something must have changed in how the matrix is treated. Might have been introduced by Open-Cascade-SAS/OCCT@33c8a72

it also looks to me like mat is not used directly but rather the values are copied over to a new gp_GTrsf so calling setForm() does noting anyway. I still don't quite get what's the sequence followed for the transformation but I'm only looking at this through the web interface now.

@Jolbas
Copy link
Contributor Author

Jolbas commented May 24, 2023

If I run the following c++ code I don't get the error and the gp_GTrsf.Form does not change from gp_Identity to gp_Other:

    BRepPrimAPI_MakeBox aBox(10.0, 10.0, 10.0);
    TopoDS_Shape aShape = aBox.Shape();
    gp_GTrsf m;
    BRepBuilderAPI_GTransform aTransform(m);
    aTransform.Perform(aShape, false);

So ther seem to be something in _Shape that is different.

@adrianinsaval
Copy link
Member

there's a couple of differences, we don't pass an identity matrix we use setValue many times on it, the first call already changes Form to gp_Other if I'm not mistaken

gp_GTrsf mat;
mat.SetValue(1,1,rclTrf[0][0]);
mat.SetValue(2,1,rclTrf[1][0]);
mat.SetValue(3,1,rclTrf[2][0]);
mat.SetValue(1,2,rclTrf[0][1]);
mat.SetValue(2,2,rclTrf[1][1]);
mat.SetValue(3,2,rclTrf[2][1]);
mat.SetValue(1,3,rclTrf[0][2]);
mat.SetValue(2,3,rclTrf[1][2]);
mat.SetValue(3,3,rclTrf[2][2]);
mat.SetValue(1,4,rclTrf[0][3]);
mat.SetValue(2,4,rclTrf[1][3]);
mat.SetValue(3,4,rclTrf[2][3]);

then we directly do

BRepBuilderAPI_GTransform mkTrf(this->_Shape, mat, copy);
return mkTrf.Shape();

we don't use Perform

@Jolbas
Copy link
Contributor Author

Jolbas commented May 24, 2023

BRepBuilderAPI_GTransform aTransform(mat);
aTransform.Perform(aShape, false);

is equivalent to

BRepBuilderAPI_GTransform aTransform(aShape, mat, false);

I don't get the error for this code directly after `mat.SetValue(2,4,rclTrf[1][3]);

BRepPrimAPI_MakeBox aBox(10.0, 10.0, 10.0);
TopoDS_Shape aShape = aBox.Shape();
BRepBuilderAPI_GTransform aTransform(aShape, mat, false);

With this shape the execution never reaches the function where the error is thrown. It returns before because the triangulation is null. I don't understand what that means.
https://github.com/Open-Cascade-SAS/OCCT/blob/5d8b1a4076398556095c71c093ca517cde77d4d8/src/BRepTools/BRepTools_GTrsfModification.cxx#L278

@Jolbas
Copy link
Contributor Author

Jolbas commented May 24, 2023

And the error is still thrown if I remove every mat.setValue(...) line.

@adrianinsaval
Copy link
Member

adrianinsaval commented May 24, 2023

maybe it's related to tessellation? I'm gonna assume when doing the transformation on the mesh it's doing something different. It was most likely introduced by that commit I mentioned then, since it introduced all this triangulation related stuff.

@adrianinsaval
Copy link
Member

adrianinsaval commented May 24, 2023

yeah it looks like it's about tessellation https://dev.opencascade.org/doc/refman/html/class_poly___triangulation.html#details

in your example probably aBox is not tessellated but _Shape is

@Jolbas
Copy link
Contributor Author

Jolbas commented May 24, 2023

In this python code the first call to transformGeometry() works but the second call fails
So the recompute on a document object somehow creates tesselation?

boxshape = Part.makeBox(1,1,1)
transformedBoxshape = boxshape.transformGeometry(App.Matrix())
boxobj = App.ActiveDocument.addObject("Part::Feature", "Box")
boxobj.Shape = boxshape
boxobj.recompute()
transformedBoxshape2 = boxobj.Shape.transformGeometry(App.Matrix())

@Jolbas
Copy link
Contributor Author

Jolbas commented May 24, 2023

The recompute isn't necessary. The assignment is enough

boxshape = Part.makeBox(1,1,1)
transformedBoxshape = boxshape.transformGeometry(App.Matrix())
# no error
boxobj = App.ActiveDocument.addObject("Part::Feature", "Box")
boxobj.Shape = boxshape
transformedBoxshape2 = boxobj.Shape.transformGeometry(App.Matrix())
# error

@Jolbas
Copy link
Contributor Author

Jolbas commented May 25, 2023

This means we may work around this by cleaning the shape before transforming!

shape_a = Part.makeBox(1,1,1)
transformed_a = shape_a.transformGeometry(App.Matrix())
# no error
part_feature = App.ActiveDocument.addObject("Part::Feature", "Box")
part_feature.Shape = shape_a
shape_b = part_feature.Shape
transformed_b = shape_b.transformGeometry(App.Matrix())
# error
shape_c = shape_b.cleaned()
transformed_b = shape_c.transformGeometry(App.Matrix())
# no error!

@adrianinsaval
Copy link
Member

We could add a workaround for occt >=7.7 for the moment, we might need to submit a bug report upstream and if possible a patch to fix it

@Jolbas
Copy link
Contributor Author

Jolbas commented May 25, 2023

A fix for OCCT may be to replace the linked lines with a simple

    theTriangulation->ComputeNormals();

https://github.com/Open-Cascade-SAS/OCCT/blob/5d8b1a4076398556095c71c093ca517cde77d4d8/src/BRepTools/BRepTools_TrsfModification.cxx#L174-L179

But I'm not sure at all that won't have side effects

@adrianinsaval
Copy link
Member

@bgbsww this is another occt 7.7 bug

@bgbsww
Copy link
Contributor

bgbsww commented Jan 8, 2024

Looks to be fixed in 7.8.0 @adrianinsaval

obj = App.ActiveDocument.addObject("Part::Box","Box")
obj.recompute()
True
obj.Shape.transformGeometry(App.Matrix())
<Shape object at 0x55f19b6636f0>

OS: Ubuntu 22.04.3 LTS (ubuntu:GNOME/ubuntu)
Word size of FreeCAD: 64-bit
Version: 0.22.0dev.35500 (Git)
Build type: Debug
Branch: bgbsww-occt7.8.0
Hash: 2a6fa0d9832920dd50d8f661e639d14c4eff9a04
Python 3.10.12, Qt 5.15.3, Coin 4.0.0, Vtk 7.1.1, OCC 7.8.0
Locale: English/United States (en_US)
Installed mods: 
  * Curves 0.6.21

@adrianinsaval
Copy link
Member

Awesome, then once the 7.8 PR is merged is probably a good idea to skip 7.7 and move our distributions to that

@adrianinsaval
Copy link
Member

adrianinsaval commented Mar 3, 2024

potentially fixed by Open-Cascade-SAS/OCCT@bcfc5f0 which was backported to 7.7.2: Open-Cascade-SAS/OCCT@c60e501

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants