Skip to content

Modeling Data - Geom_BezierSurface IsURational and IsVRational documentation matches the example - #1602

Open
gsdali wants to merge 1 commit into
Open-Cascade-SAS:IRfrom
SecondMouseAU:docs/geom-beziersurface-rational-axis
Open

gsdali wants to merge 1 commit into
Open-Cascade-SAS:IRfrom
SecondMouseAU:docs/geom-beziersurface-rational-axis

Conversation

@gsdali

@gsdali gsdali commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Pre-Submission Checks

  • I checked existing issues, pull requests, discussions, and forum topics for related work.
  • I followed the contribution guidance in .github/CONTRIBUTING.md.
  • I used a PR title in the Group - Summary format.

Problem / Motivation

Geom_BezierSurface.hxx says

//! Returns False if the weights are identical in the U direction,
//! ...
//! Example :
//! |1.0, 1.0, 1.0|
//! if Weights =  |0.5, 0.5, 0.5|   returns False
//! |2.0, 2.0, 2.0|
Standard_EXPORT bool IsURational() const;

The weights in that matrix are not identical in the U direction: they run 1.0, 0.5, 2.0 down a column. They are identical in the V direction. The prose and the example contradict each other, and the example is the one that matches the code. IsVRational has the mirrored problem.

Geom_BezierSurface.cxx sets Urational in the static Rational() from

Urational = (std::abs(Weights(I, J) - Weights(I, J + 1)) > Epsilon(std::abs(Weights(I, J))));

inside a loop over I that walks J, so it compares neighbours in the column index. Urational is false exactly when every row of the weight matrix is constant, and Vrational is false exactly when every column is. Each flag names the axis opposite the one it compares along, which is easy to misread from the prose: a cylinder converted to a Geom_BSplineSurface reports (IsURational, IsVRational) = (0, 1), which reads as an inverted conversion if the prose is taken literally.

Geom_BSplineSurface.hxx already says this correctly, with the same two example matrices ("Returns False if for each row of weights all the weights are identical."), so the two headers also disagree with each other.

Proposed Solution

Give IsURational and IsVRational of Geom_BezierSurface the wording Geom_BSplineSurface has ("for each row of weights" and "for each column of weights"). The example matrices and the tolerance sentence are untouched. Documentation only: no code, no behaviour change, so no GTest is added.

Validation

A probe linked against a local build of current IR, four explicit 3 x 3 weight matrices on a Geom_BezierSurface:

weights IsURational IsVRational
each row constant (the example of IsURational) 0 1
each column constant (the example of IsVRational) 1 0
all equal 0 0
varying along both 1 1

This is what the new wording says. TKG3d was rebuilt with the changed header (Release, macOS arm64, clang) with no warning.

Checks performed:

  • Relevant tests were added or updated when applicable. (Not applicable: comment-only change.)
  • Relevant local tests were run. (Probe above; TKG3d builds.)

clang-format 18.1.8 with the repository .clang-format, the license check and the include cleanup of the formatting job report no change, and the file contains no non-ASCII character.

Review Notes

Comment-only change in one header, independent of the other open pull requests.

…ntation matches the example

The text says IsURational returns False if the weights are identical in the U
direction, but its own example matrix has weights 1.0, 0.5, 2.0 down a column
(they vary along U and are identical along V) and returns False. The static
Rational() compares Weights(I, J) with Weights(I, J + 1), so Urational is false
exactly when every row of the weight matrix is constant, and Vrational is false
exactly when every column is.

Use the wording Geom_BSplineSurface already has for IsURational, and the
matching one for IsVRational. Documentation only, no behaviour change, so no
test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

1 participant