Coding - Add conversion utilities for STEP geometrical and visual enumerations by dpasukhi · Pull Request #545 · Open-Cascade-SAS/OCCT

dpasukhi · 2025-05-17T18:47:15Z

Introduced RWStepGeom_RWTransitionCode for converting StepGeom_TransitionCode to/from string representations.
Refactored RWStepGeom_RWTrimmedCurve to utilize RWStepGeom_RWTrimmingPreference for trimming preference conversions.
Created RWStepGeom_RWTrimmingPreference for handling StepGeom_TrimmingPreference enumerations.
Updated RWStepGeom_RWUniformCurve and related classes to use RWStepGeom_RWBSplineCurveForm for B-spline curve form conversions.
Added RWStepGeom_RWUniformSurface and related classes to use RWStepGeom_RWBSplineSurfaceForm for B-spline surface form conversions.
Implemented RWStepShape_RWBooleanOperator for boolean operator conversions in STEP shapes.
Refactored RWStepShape_RWBooleanResult to utilize RWStepShape_RWBooleanOperator for boolean operator handling.
Introduced RWStepVisual_RWCentralOrParallel for central or parallel projection type conversions.
Added RWStepVisual_RWSurfaceSide for surface side enumeration conversions.
Updated RWStepVisual_RWSurfaceStyleUsage to use RWStepVisual_RWSurfaceSide for handling surface side.
Created RWStepVisual_RWTextPath for text path enumeration conversions.
Refactored RWStepVisual_RWTextLiteral to utilize RWStepVisual_RWTextPath for text path handling.
Updated RWStepVisual_RWViewVolume to use RWStepVisual_RWCentralOrParallel for projection type conversions.

Copilot

Pull Request Overview

This PR replaces inline enumeration mapping with centralized conversion utilities, improving consistency and reducing boilerplate across STEP readers and writers.

Replaced manual BSplineCurveForm string constants and switches with RWStepGeom_RWBSplineCurveForm conversions
Introduced and applied RWStepBasic_RWSiPrefix and RWStepBasic_RWSiUnitName for all SI unit reader/writer classes
Swapped out local enum mappings in RWStepBasic for RWStepBasic_RWSource and RWStepBasic_RWAheadOrBehind

Reviewed Changes

Copilot reviewed 56 out of 63 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/DataExchange/TKDESTEP/RWStepGeom/RWStepGeom_RWBSplineCurve.cxx	Use `RWStepGeom_RWBSplineCurveForm` for `curve_form` conversion
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitAndVolumeUnit.cxx	Read/write SI unit prefix and name via new `ConvertToEnum`/`ConvertToString` utilities
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitAndTimeUnit.cxx	Applied `RWStepBasic` enumeration utilities for time units
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitAndThermodynamicTemperatureUnit.cxx	Refactored thermodynamic temperature unit enum handling to use conversion utilities
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitAndSolidAngleUnit.cxx	Updated solid angle unit mapping to new shared converters
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitAndRatioUnit.cxx	Switched ratio unit read/write to `RWStepBasic_RWSiPrefix` and `RWStepBasic_RWSiUnitName`
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitAndPlaneAngleUnit.cxx	Replaced plane angle unit enum logic with centralized converters
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitAndMassUnit.cxx	Applied common SI enum converters for mass units
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitAndLengthUnit.cxx	Length unit mapping now uses shared prefix/name conversion
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitAndAreaUnit.cxx	Area unit reader/writer updated to use SI prefix/name utilities
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnit.cxx	Removed inline enum maps, delegating prefix/name conversions to `RWStepBasic_RWSiPrefix` and `RWStepBasic_RWSiUnitName`
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWProductDefinitionFormationWithSpecifiedSource.cxx	Simplified `Source` enum handling via `RWStepBasic_RWSource`
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWCoordinatedUniversalTimeOffset.cxx	Replaced `AheadOrBehind` inline mapping with `RWStepBasic_RWAheadOrBehind`

Files not reviewed (7)

src/DataExchange/TKDESTEP/RWStepBasic/FILES.cmake: Language not supported
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWAheadOrBehind.pxx: Language not supported
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiPrefix.pxx: Language not supported
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnit.pxx: Language not supported
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitName.pxx: Language not supported
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSource.pxx: Language not supported
src/DataExchange/TKDESTEP/RWStepGeom/FILES.cmake: Language not supported

Comments suppressed due to low confidence (2)

src/DataExchange/TKDESTEP/RWStepGeom/RWStepGeom_RWBSplineCurve.cxx:82

The error message refers to 'b_spline_curve_form' in lowercase; consider matching the exact enumeration identifier (e.g. 'B_SPLINE_CURVE_FORM') or including the parameter name (#4) for clarity.

ach->AddFail("Enumeration b_spline_curve_form has not an allowed value");

src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitAndVolumeUnit.cxx:57

Consider adding unit tests to cover RWStepBasic_RWSiPrefix::ConvertToEnum and RWStepBasic_RWSiUnitName::ConvertToEnum to ensure all prefixes and unit names are correctly mapped.

hasAprefix            = RWStepBasic_RWSiPrefix::ConvertToEnum(text, aPrefix);

Copilot

Pull Request Overview

This PR introduces a set of conversion utility classes for STEP geometrical, basic, and visual enumerations, and refactors several RWStep* files to use these new conversion functions.

Introduced conversion utilities (e.g., RWStepGeom_RWBSplineCurveForm, RWStepBasic_RWSiPrefix, RWStepBasic_RWSiUnitName, etc.) for enum‐to‐string and string‐to‐enum conversions.
Refactored multiple files to remove custom decoding logic and use the new conversion routines.
Updated error handling in enum conversion scenarios for improved consistency.

Reviewed Changes

Copilot reviewed 56 out of 63 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/DataExchange/TKDESTEP/RWStepGeom/RWStepGeom_RWBSplineCurve.cxx	Replaced custom enum mapping with conversion utility RWBSplineCurveForm.
src/DataExchange/TKDESTEP/RWStepBasic/*	Refactored RWStepBasic_RWSiUnitAnd... files to use RWStepBasic_RWSiPrefix and RWStepBasic_RWSiUnitName.
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWProductDefinitionFormationWithSpecifiedSource.cxx	Updated source enum conversion using RWStepBasic_RWSource utility.
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWCoordinatedUniversalTimeOffset.cxx	Replaced manual Ahead/Behind conversion with RWStepBasic_RWAheadOrBehind utility.
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnit.cxx	Removed custom Decode/Encode functions in favor of new conversion methods.

Files not reviewed (7)

src/DataExchange/TKDESTEP/RWStepBasic/FILES.cmake: Language not supported
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWAheadOrBehind.pxx: Language not supported
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiPrefix.pxx: Language not supported
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnit.pxx: Language not supported
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitName.pxx: Language not supported
src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSource.pxx: Language not supported
src/DataExchange/TKDESTEP/RWStepGeom/FILES.cmake: Language not supported

Comments suppressed due to low confidence (5)

src/DataExchange/TKDESTEP/RWStepGeom/RWStepGeom_RWBSplineCurve.cxx:80

Ensure that the ConvertToEnum function in RWStepGeom_RWBSplineCurveForm handles all possible curve form enum values and returns a clear error for unrecognized inputs.

if (!RWStepGeom_RWBSplineCurveForm::ConvertToEnum(text, aCurveForm))

src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitAndTimeUnit.cxx:60

Verify that RWStepBasic_RWSiPrefix::ConvertToEnum covers all valid si_prefix cases to avoid potential mismatches across different unit files.

hasAprefix            = RWStepBasic_RWSiPrefix::ConvertToEnum(text, aPrefix);

src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnit.cxx:48

The removal of custom DecodePrefix/DecodeName functions improves maintainability. Ensure that the new conversion functions fully replicate the original behavior for all enum values.

hasAprefix            = RWStepBasic_RWSiPrefix::ConvertToEnum(text, aPrefix);

src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWProductDefinitionFormationWithSpecifiedSource.cxx:66

Confirm that RWStepBasic_RWSource::ConvertToEnum correctly distinguishes between all valid source enum values to maintain consistent behavior.

if (!RWStepBasic_RWSource::ConvertToEnum(text, aMakeOrBuy))

src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWCoordinatedUniversalTimeOffset.cxx:63

Verify that the conversion utility RWStepBasic_RWAheadOrBehind accurately covers all enum variants for time offset sense, ensuring consistency with the updated conversion approach.

if (!RWStepBasic_RWAheadOrBehind::ConvertToEnum(text, aSense))

…merations - Introduced RWStepGeom_RWTransitionCode for converting StepGeom_TransitionCode to/from string representations. - Refactored RWStepGeom_RWTrimmedCurve to utilize RWStepGeom_RWTrimmingPreference for trimming preference conversions. - Created RWStepGeom_RWTrimmingPreference for handling StepGeom_TrimmingPreference enumerations. - Updated RWStepGeom_RWUniformCurve and related classes to use RWStepGeom_RWBSplineCurveForm for B-spline curve form conversions. - Added RWStepGeom_RWUniformSurface and related classes to use RWStepGeom_RWBSplineSurfaceForm for B-spline surface form conversions. - Implemented RWStepShape_RWBooleanOperator for boolean operator conversions in STEP shapes. - Refactored RWStepShape_RWBooleanResult to utilize RWStepShape_RWBooleanOperator for boolean operator handling. - Introduced RWStepVisual_RWCentralOrParallel for central or parallel projection type conversions. - Added RWStepVisual_RWSurfaceSide for surface side enumeration conversions. - Updated RWStepVisual_RWSurfaceStyleUsage to use RWStepVisual_RWSurfaceSide for handling surface side. - Created RWStepVisual_RWTextPath for text path enumeration conversions. - Refactored RWStepVisual_RWTextLiteral to utilize RWStepVisual_RWTextPath for text path handling. - Updated RWStepVisual_RWViewVolume to use RWStepVisual_RWCentralOrParallel for projection type conversions.

Co-authored-by: Copilot <[email protected]> Signed-off-by: Pasukhin Dmitry <[email protected]>

ikochetkova

Please have a look at a couple of small comments.

ikochetkova · 2025-06-23T16:04:50Z

@@ -0,0 +1,81 @@
+// Created on: 1995-12-01


File looks like a new one but with the very old header

ikochetkova · 2025-06-23T16:12:55Z

+static constexpr char spPeta[]  = ".PETA.";
+static constexpr char spDeci[]  = ".DECI.";
+static constexpr char spKilo[]  = ".KILO.";
+static constexpr char spDeca[]  = ".DECA.";


That was the sequence of the sorting? Looks random, it can easily lead to missing some prefixes.

I just used what was before.
It is the same order as before, I'm not sure that it was ordered somehow.
I was afraid to change something, because I was assuming it was sortied by some hidden logic. But looks like it is just random :)

Copilot

Pull Request Overview

This PR refactors the STEP enumeration conversion logic by introducing dedicated conversion utilities (in .pxx files) for various enumerations in STEP visual, shape, geom, and basic modules. Key changes include removing redundant inline static strings and switch-case conversions in reader/writer classes, and replacing them with centralized conversion functions to improve consistency and maintainability.

Reviewed Changes

Copilot reviewed 63 out of 63 changed files in this pull request and generated 1 comment.

File	Description
RWStepVisual_RWViewVolume.cxx	Replaces inline enum comparisons with conversion functions for projection type.
RWStepShape_RWBooleanResult.cxx, RWStepShape_RWBooleanOperator.pxx	Refactors boolean operator conversion using a dedicated conversion file.
RWStepGeom/* files (e.g., RWStepGeom_RWBSplineSurfaceForm.pxx, RWStepGeom_RWTrimmingPreference.pxx)	Consolidates conversion logic for various geometric enums into centralized utilities.
RWStepBasic/* files (e.g., RWStepBasic_RWSiUnitName.pxx, RWStepBasic_RWSiPrefix.pxx)	Updates SI unit conversion to use shared conversion functions.

Comments suppressed due to low confidence (2)

src/DataExchange/TKDESTEP/RWStepVisual/RWStepVisual_RWViewVolume.cxx:124

The use of the conversion utility improves maintainability; however, ensure that ConvertToString reliably returns a valid string (i.e. not nullptr) for all valid enum values to prevent potential runtime issues during serialization.

  SW.SendEnum(RWStepVisual_RWCentralOrParallel::ConvertToString(ent->ProjectionType()));

src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitAndVolumeUnit.cxx:109

Refactoring to use conversion functions for SI unit names enhances consistency; consider adding a fallback or explicit error handling if ConvertToString returns nullptr.

  SW.SendEnum(RWStepBasic_RWSiUnitName::ConvertToString(ent->Name()));

Copilot · 2025-06-23T17:59:09Z

+    case StepGeom_bssfQuadricSurf:
+      return bssfQuadricSurf;
+  }
+  return nullptr;


Returning nullptr for an unrecognized enumeration may lead to silent failures during WriteStep; consider either logging an error or providing a default string value to improve error visibility during debugging.

Suggested change

return nullptr;

std::cerr << "Error: Unrecognized StepGeom_BSplineSurfaceForm enumeration value encountered." << std::endl;

return ".UNKNOWN_SURF_FORM.";

ikochetkova

No more remarks

dpasukhi self-assigned this May 17, 2025

dpasukhi added 2. Enhancement New feature or request 1. Coding Coding rules, trivial changes and misprints labels May 17, 2025

github-project-automation Bot added this to Maintenance May 17, 2025

github-project-automation Bot moved this to Todo in Maintenance May 17, 2025

dpasukhi requested a review from Copilot May 17, 2025 18:49

Copilot AI reviewed May 17, 2025

View reviewed changes

Comment thread src/DataExchange/TKDESTEP/RWStepBasic/RWStepBasic_RWSiUnitAndVolumeUnit.cxx

dpasukhi requested a review from Copilot May 17, 2025 18:53

Copilot AI reviewed May 17, 2025

View reviewed changes

dpasukhi requested a review from Copilot May 17, 2025 18:55

This comment was marked as outdated.

Sign in to view

dpasukhi requested review from a team and removed request for a team May 17, 2025 18:56

dpasukhi added 3 commits June 21, 2025 10:16

formatting

58d1ad3

Refactor conversion functions to inline for improved performance

09f5b46

dpasukhi force-pushed the stp_update_enums branch from 26eb2c3 to 09f5b46 Compare June 21, 2025 09:16

dpasukhi requested review from a team and Copilot June 21, 2025 09:16

This comment was marked as outdated.

Sign in to view

dpasukhi added this to the Release 8.0 milestone Jun 23, 2025

Update src/DataExchange/TKDESTEP/RWStepGeom/FILES.cmake

9422617

Co-authored-by: Copilot <[email protected]> Signed-off-by: Pasukhin Dmitry <[email protected]>

ikochetkova reviewed Jun 23, 2025

View reviewed changes

Update copyright information in RWStepGeom_RWKnotType.pxx

8a6a59e

dpasukhi requested review from Copilot and ikochetkova June 23, 2025 17:58

Copilot AI reviewed Jun 23, 2025

View reviewed changes

ikochetkova approved these changes Jun 25, 2025

View reviewed changes

github-project-automation Bot moved this from Todo to Integration in Maintenance Jun 25, 2025

dpasukhi merged commit ed7a447 into Open-Cascade-SAS:IR Jun 25, 2025
34 checks passed

dpasukhi deleted the stp_update_enums branch June 25, 2025 10:40

github-project-automation Bot moved this from Integration to Done in Maintenance Jun 25, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Coding - Add conversion utilities for STEP geometrical and visual enumerations#545

Coding - Add conversion utilities for STEP geometrical and visual enumerations#545
dpasukhi merged 5 commits intoOpen-Cascade-SAS:IRfrom
dpasukhi:stp_update_enums

dpasukhi commented May 17, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

ikochetkova left a comment

Uh oh!

ikochetkova Jun 23, 2025

Uh oh!

ikochetkova Jun 23, 2025

Uh oh!

dpasukhi Jun 23, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jun 23, 2025

Uh oh!

ikochetkova left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	return nullptr;
	std::cerr << "Error: Unrecognized StepGeom_BSplineSurfaceForm enumeration value encountered." << std::endl;
	return ".UNKNOWN_SURF_FORM.";

Conversation

dpasukhi commented May 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

ikochetkova left a comment

Choose a reason for hiding this comment

Uh oh!

ikochetkova Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

ikochetkova Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

dpasukhi Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

ikochetkova left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dpasukhi Jun 23, 2025 •

edited

Loading