Add BRep_Tool.CurveOnSurface_r returning (pcurve, first, last) (#167) - #203
Open
GabrielJMS wants to merge 1 commit into
Open
Add BRep_Tool.CurveOnSurface_r returning (pcurve, first, last) (#167)#203GabrielJMS wants to merge 1 commit into
GabrielJMS wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
…ery#167) CurveOnSurface returns a non-void Handle and reports its parameter range through Standard_Real& reference args, which are unreachable from Python (pybind11 cannot write back immutable types). The existing call-by-reference tuple mechanism only triggers for void-returning methods, so this case is missed. Add a targeted, non-breaking workaround via the rules mechanism: a new static method CurveOnSurface_r(E, F) that returns (pcurve, first, last). The original CurveOnSurface_s binding is left untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #167.
BRep_Tool::CurveOnSurface returns a non-void Handle(Geom2d_Curve) and reports the
edge's parameter range through reference arguments (Standard_Real& First, Standard_Real& Last).
Because Standard_Real maps to an immutable Python float, those outputs cannot be written
back to the caller, so the parameter range is unreachable from Python — which blocks the
parametric-space (UV) workflows described in the issue.
The existing call-by-reference → tuple mechanism (is_byref in pywrap) only triggers for
void-returning methods, so a non-void method like this one is missed.
Change
A targeted, non-breaking workaround via the rules mechanism (as suggested in the issue
thread — "add workaround method in the binding manually or via rules"). It adds a new static
method and leaves the original CurveOnSurface_s binding untouched:
Usage:
The _r suffix denotes "reference outputs returned as a tuple". Only the edge-on-face
overload is wrapped here.
Questions for the maintainers
@adam-urbanczyk
Does this targeted approach look acceptable? It mirrors the existing
additional_static_methods precedents and changes no existing signatures.
Would you prefer a more general implementation instead? The void call-by-reference
tuple handling could be generalized to also cover non-void methods whose outputs come
back through immutable-type references (Standard_Real/Standard_Integer/Standard_Boolean),
auto-generating these tuple variants — i.e. the same treatment void functions already get.
That would be a change in pywrap (relaxing the void-only gate in is_byref and
extending the byref lambda in template_sub.j2 to prepend the return value to the tuple),
not in this repo. Happy to open that as a separate pywrap PR if you'd rather solve it
generally.