Retrieving weights of a rational BSpline surface causes crashing

[russelmann]

Calls to method const TColStd_Array2OfReal* Geom_BSplineSurface::Weights() const crash while the same operation runs fine using the alternative signature void Geom_BSplineSurface::Weights(TColStd_Array2OfReal& W) const.

from OCCT.Geom import Geom_BSplineSurface, Geom_RectangularTrimmedSurface, Geom_SphericalSurface
from OCCT.GeomConvert import GeomConvert
from OCCT.TColStd import TColStd_Array2OfReal
from OCCT.gp import gp_Ax3

# Prepare some geometry to initialize a rational BSpline surface.
spherical_surface = Geom_SphericalSurface(gp_Ax3(), 1.)
surface = Geom_RectangularTrimmedSurface(spherical_surface, 0., 1., 0., 1.)

# Second loop won't be executed due to the crash.
reproduce = True
for k in range(2):
    bspline_surface: Geom_BSplineSurface = GeomConvert.SurfaceToBSplineSurface_(surface)
    if reproduce:  # causes crashing
        weights = bspline_surface.Weights()
    else:  # a lengthy workaround
        bspline_surface.Weights(weights := TColStd_Array2OfReal(1, bspline_surface.NbUPoles(), 1, bspline_surface.NbVPoles()))
    values = [[weights(i, j) for j in range(1, 1 + weights.NbColumns())] for i in range(1, 1 + weights.NbRows())] # not essential
    print(k)

This might be caused by the fact that the return type is a pointer unlike with other similar methods, such as const TColgp_Array2OfPnt& Geom_BSplineSurface::Poles() const which return a reference and do not cause any problems.
I think the same problem occurs with rational BSpline curves. Let me know if a code snippet for this case would be useful too.

[trelau]

yes it is likely to do with the pointer as outlined here https://pybind11.readthedocs.io/en/stable/advanced/smart_ptrs.html#smart-pointers (the part where they point out returning a raw pointer that breaks the code). although, i thought there were other places in the pyOCCT codebase that returned raw pointers without issue. Might have to find some of these places and if they all crash, then we'll need to find a way to fix returning raw pointers in general.

For this specific case, we could alter the source code to this:

cls_Geom_BSplineSurface.def("Weights", [](Geom_BSplineSurface &self) {TColStd_Array2OfReal weights(1, self.NbUPoles(), 1, self.NbVPoles()); self.Weights(weights); return weights; });

This just calls the other method behind the scenes, but with the same signature as the one causing the issue.

[trelau]

The conservative thing to do might be just skip any function that returns a pointer for now and handle this type of thing on a case-by-case basis until a more general and proper solution can be found 🤔

[russelmann]

I think that in the case of weights it would be nice to replicate a behavior similar to what happens in C++, i.e. it returns None if the curve/surface is non-rational and returns an TColStd_Array2OfReal (perhaps, by reference, to avoid unnecessary data copying?) if it is rational. This should be doable using std::optional from C++17. A drawback is that the reference will not know if the curve/surface has been deleted together with the TColStd_Array2OfReal containing the weights.
I will try to build pyocct from sources to experiment with this.

[russelmann]

I have a suggestion in accordance with my prior comment which does not require C++17:

cls_Geom_BSplineSurface.def("Weights", [](Geom_BSplineSurface& self) -> py::object {return self.Weights() ? py::cast(*self.Weights()) : py::none();});

[trelau]

@russelmann thanks, that seems to do the trick for this case. Opened #84