Data Exchange, DE_Wrapper - Implement Stream support#663
Data Exchange, DE_Wrapper - Implement Stream support#663dpasukhi merged 41 commits intoOpen-Cascade-SAS:IRfrom
Conversation
- 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.
… and configuration nodes
- 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.
…ion nodes, file paths, and streams
- 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.
…able naming in DESTL_Provider
… to stream operations
…stream-based operations
…or clarity and consistency
…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.
…operations and error handling
… 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.
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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.
| { | ||
| // Try to detect ASCII vs Binary format by peeking at the first few bytes | ||
| std::streampos originalPos = theStream.tellg(); | ||
| char header[6]; |
There was a problem hiding this comment.
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.
| char header[6]; | |
| char header[THE_STL_HEADER_SIZE]; |
There was a problem hiding this comment.
Please replace magic number with named constant as requested.
| myProvider = new DESTL_Provider(aNode); | ||
|
|
||
| // Create triangulated shape for testing (STL format requires triangulated data) | ||
|
|
There was a problem hiding this comment.
[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.
| // Create triangulated shape for testing (STL format requires triangulated data) |
| dchrono s stop counter writestl | ||
| set aFileSize [file size $imagedir/${casename}-ascii.stl] | ||
| if {$aFileSize < 490000000} { | ||
| if {$aFileSize < 480000000} { |
There was a problem hiding this comment.
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.
| 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} { |
…ze threshold in performance test for accurate validation
AtheneNoctuaPt
left a comment
There was a problem hiding this comment.
Please fix these remarks.
| { | ||
| // Try to detect ASCII vs Binary format by peeking at the first few bytes | ||
| std::streampos originalPos = theStream.tellg(); | ||
| char header[6]; |
There was a problem hiding this comment.
Please replace magic number with named constant as requested.
…ationUtils, DEIGES_Provider, and RWStl for better readability and performance
…riable declarations in DE_ValidationUtils and RWStl
…dundant condition
No description provided.