Skip to content

Data Exchange, DE_Wrapper - Implement Stream support#663

Merged
dpasukhi merged 41 commits intoOpen-Cascade-SAS:IRfrom
dpasukhi:de_streams
Aug 8, 2025
Merged

Data Exchange, DE_Wrapper - Implement Stream support#663
dpasukhi merged 41 commits intoOpen-Cascade-SAS:IRfrom
dpasukhi:de_streams

Conversation

@dpasukhi
Copy link
Copy Markdown
Member

@dpasukhi dpasukhi commented Aug 7, 2025

No description provided.

dpasukhi added 30 commits August 5, 2025 23:36
- Implemented IsStreamSupported method in DESTL_ConfigurationNode and DEVRML_ConfigurationNode to indicate stream support.
- Enhanced DESTL_Provider and DEVRML_Provider with Read and Write methods that handle stream operations for documents and shapes.
- Updated VrmlAPI_Writer to support writing shapes and documents to streams, including version handling.
- Refactored existing write methods to utilize stream output instead of file output directly.
- Added error handling and warnings for stream operations in both providers.
…P, and DEVRML Providers to accept non-const ReadStreamMap references

- Updated method signatures for Read and Write functions in DEIGES_Provider, DEOBJ_Provider, DEPLY_Provider, DESTL_Provider, DESTEP_Provider, and DEVRML_Provider to take ReadStreamMap by non-const reference.
- Changed usage of FindFromIndex to ChangeFromIndex for stream retrieval to allow modification of the stream map.
- Simplified error handling in DEOBJ_Provider and DEPLY_Provider by directly returning results from Read and Write calls.
- Enhanced error messages for empty stream maps and incorrect configuration nodes in DEVRML_Provider.
- Consolidated redundant code and improved readability across multiple provider implementations.
- Added validation functions for stream maps and configuration nodes to improve error handling and reduce code duplication.
- Introduced SetupApplication and ConfigureReaderFilter functions to streamline application setup and filter configuration.
- Refactored Read and Write methods to utilize the new validation and setup functions, enhancing readability and maintainability.
- Implemented ExtractShapeFromDocument to encapsulate shape extraction logic from documents.
- Improved error messaging for various failure scenarios during reading and writing processes.
- Introduced DE_ValidationUtils for centralized validation functions across multiple providers.
- Replaced individual validation implementations for configuration nodes, documents, and stream maps with calls to DE_ValidationUtils.
- Improved error handling and warning messages for better clarity and consistency.
- Simplified the code structure by removing redundant validation functions and consolidating logic.
- Ensured that all providers now utilize the new validation utility functions for improved maintainability and readability.
…ffer methods for improved stream handling and buffer creation
…ndle stream reading more safely and accurately
- Adjusted parameter alignment in function signatures for consistency in `DESTL_Provider.hxx` and `DEVRML_Provider.hxx`.
- Removed unnecessary blank lines and improved spacing in `RWStl.cxx`, `RWStl.hxx`, and `StlAPI_Writer.cxx` to enhance code clarity.
- Streamlined error handling and validation logic in `DEVRML_Provider.cxx` for better maintainability.
- Updated comments and function definitions in `VrmlAPI_Writer.cxx` and `VrmlAPI_Writer.hxx` to ensure clarity and consistency.
… stream-based read/write operations and error handling
- Changed parameter types from ReadStreamMap and WriteStreamMap to ReadStreamList and WriteStreamList in multiple methods across DEIGES, DESTEP, DESTL, and DEVRML providers.
- Updated validation methods to use the new stream list types.
- Adjusted stream access methods to accommodate the new list structure.
- Modified unit tests to reflect changes in stream handling, ensuring compatibility with the new ReadStreamList and WriteStreamList types.
…Provider and related classes for improved safety and consistency
…ags before seeking, add shape validation, and improve test coverage for wireframe and shaded modes
…ing write operations; introduce comprehensive unit tests for DESTL_Provider functionality
…tion functions for improved parameter handling and optional document setup
…n functions for improved clarity and unit handling; add validation checks for document units during writing operations
…ersonalization occurs after static initialization in Read and Write methods
…stream support methods and clean up code for improved maintainability
…ite methods

- Removed multiple overloaded Read and Write methods in DEXCAF_Provider and DEBREP_Provider that handled streams, simplifying the interface.
- Updated the Read and Write methods to directly work with documents and shapes without the need for session parameters.
- Cleaned up the DEXCAF_ConfigurationNode by removing the IsStreamSupported method, as it was unnecessary.
- Enhanced error handling in the Read and Write methods to ensure proper validation and messaging.
- Consolidated application setup logic for better maintainability and clarity.
- Removed unnecessary blank lines and adjusted spacing in DEVRML_Provider.cxx and DEVRML_Provider.hxx for improved code clarity.
- Standardized parameter formatting in function declarations across DEVRML_Provider class.
- Enhanced readability in DEVRML_Provider_Test by aligning variable declarations and stream operations.
- Ensured consistent formatting in test cases, including spacing and line breaks, to adhere to coding standards.
@dpasukhi dpasukhi added this to the Release 8.0 milestone Aug 7, 2025
@dpasukhi dpasukhi requested a review from Copilot August 7, 2025 00:04
@dpasukhi dpasukhi self-assigned this Aug 7, 2025
@dpasukhi dpasukhi added the 1. Data Exchange Import/Export or iterating of the CAD data label Aug 7, 2025
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements stream support for the Data Exchange (DE) subsystem, enabling providers to read from and write to streams instead of only file paths. The primary focus is on implementing stream operations for VRML, STL, and STEP providers.

Key changes:

  • Adds stream-based read/write methods to multiple data exchange providers
  • Refactors underlying APIs (VrmlAPI_Writer, StlAPI_Writer, RWStl) to support stream operations
  • Implements comprehensive test coverage for stream functionality
  • Adds validation utilities for improved error handling

Reviewed Changes

Copilot reviewed 40 out of 40 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/perf/de/bug26338_2 Updates performance test threshold for STL file size validation
VrmlAPI_Writer.hxx/.cxx Adds stream-based write methods and refactors to use streams internally
DEVRML_Provider.hxx/.cxx Implements stream support with comprehensive validation and error handling
StlAPI_Writer.hxx/.cxx Adds stream-based write method for STL format
RWStl.hxx/.cxx Adds comprehensive stream support for both ASCII and binary STL formats
DESTEP_Provider.hxx/.cxx Implements stream support for STEP format with proper validation
Multiple test files Adds comprehensive test coverage for stream operations across all providers
Comments suppressed due to low confidence (1)

src/DataExchange/TKDEVRML/VrmlAPI/VrmlAPI_Writer.cxx:358

  • [nitpick] Empty line removal creates inconsistent spacing. Consider maintaining consistent spacing around conditional blocks for better readability.
      if (!aTri.IsNull())

aByteCount += 4;
aDataChunk[aByteCount] = 0;
aDataChunk[aByteCount + 1] = 0;
aByteCount += 2;
Copy link

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Array indexing issue: accessing aByteCount + 1 without incrementing aByteCount first, then incrementing by 2. This should be aDataChunk[aByteCount] = 0; aByteCount++; aDataChunk[aByteCount] = 0; aByteCount++; to match the original logic.

Copilot uses AI. Check for mistakes.
{
// Try to detect ASCII vs Binary format by peeking at the first few bytes
std::streampos originalPos = theStream.tellg();
char header[6];
Copy link

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic number 6 for header size should be defined as a named constant for better maintainability and to explain why 6 bytes are needed for format detection.

Suggested change
char header[6];
char header[THE_STL_HEADER_SIZE];

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please replace magic number with named constant as requested.

myProvider = new DESTL_Provider(aNode);

// Create triangulated shape for testing (STL format requires triangulated data)

Copy link

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Incomplete comment line has no implementation following it. The comment suggests creating a triangulated shape but the actual creation happens later at line 54.

Suggested change
// Create triangulated shape for testing (STL format requires triangulated data)

Copilot uses AI. Check for mistakes.
Comment thread tests/perf/de/bug26338_2 Outdated
dchrono s stop counter writestl
set aFileSize [file size $imagedir/${casename}-ascii.stl]
if {$aFileSize < 490000000} {
if {$aFileSize < 480000000} {
Copy link

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic number 480000000 should be documented or use a named constant to explain why this specific file size threshold is expected for the performance test.

Suggested change
if {$aFileSize < 480000000} {
# Expected minimum file size for 2M triangle ASCII STL export (performance test threshold)
set EXPECTED_MIN_STL_FILE_SIZE 480000000
if {$aFileSize < $EXPECTED_MIN_STL_FILE_SIZE} {

Copilot uses AI. Check for mistakes.
@dpasukhi dpasukhi requested a review from a team August 7, 2025 07:47
…ze threshold in performance test for accurate validation
@dpasukhi dpasukhi marked this pull request as ready for review August 7, 2025 10:36
Copy link
Copy Markdown
Collaborator

@AtheneNoctuaPt AtheneNoctuaPt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix these remarks.

Comment thread src/DataExchange/TKDE/DE/DE_ValidationUtils.cxx Outdated
Comment thread src/DataExchange/TKDE/DE/DE_ValidationUtils.cxx Outdated
Comment thread src/DataExchange/TKDE/DE/DE_ValidationUtils.cxx Outdated
Comment thread src/DataExchange/TKDE/DE/DE_ValidationUtils.cxx Outdated
Comment thread src/DataExchange/TKDE/DE/DE_ValidationUtils.cxx
Comment thread src/DataExchange/TKDE/DE/DE_ValidationUtils.cxx
Comment thread src/DataExchange/TKDEIGES/DEIGES/DEIGES_Provider.cxx Outdated
{
// Try to detect ASCII vs Binary format by peeking at the first few bytes
std::streampos originalPos = theStream.tellg();
char header[6];
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please replace magic number with named constant as requested.

Comment thread src/DataExchange/TKDEVRML/DEVRML/DEVRML_Provider.cxx Outdated
@github-project-automation github-project-automation bot moved this from Todo to In Progress in Maintenance Aug 8, 2025
…ationUtils, DEIGES_Provider, and RWStl for better readability and performance
…riable declarations in DE_ValidationUtils and RWStl
@github-project-automation github-project-automation bot moved this from In Progress to Integration in Maintenance Aug 8, 2025
@dpasukhi dpasukhi merged commit 711fbc4 into Open-Cascade-SAS:IR Aug 8, 2025
23 checks passed
@github-project-automation github-project-automation bot moved this from Integration to Done in Maintenance Aug 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1. Data Exchange Import/Export or iterating of the CAD data

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants