Data Exchange - Update Readers with ShapeHealing parameters #247
Conversation
AtheneNoctuaPt
added
2. Enhancement
…ade-SAS#247 All instances of using XSAlgo_AlgoContainer are replaced with XSAlgo_ShapeProcessor. Parameters for XSAlgo_ShapeProcessor operations are now can be passes via the updated interface of respective classes.
AtheneNoctuaPt
force-pushed
the
CR0033811
branch
from
3cde7a9 to
022a887
Compare
There was a problem hiding this comment.
I still in process of review.
But the main point - do we really need in depended static containers of internal map(yes, they are always default and use only content from stucture to fill up, but still use some type of hidden magic for the user or developer)?
My idea was have single input and share that map as a smart pointer and if that smart pointer null - fill up.
As for a copying and propagating - the smart pointer it is always planned to be deep copibable (in healing process we use only own copy).
Moreover, in normal scenario map is always initialized by high level of classes. It means, only single map used for while transfer circle. Once generated, used for single process. BUT in case of normal formats (NOT IGES), we can have multiple processes (in parallel threads) that can have different healing parameters.
For example Step reader always will fill up container. The diffrence is only how it will be filled up. So, as a result 3 possible ways that must to be clear for the user
- fill up map by special method that have input map and fill up it from Static_Interface
- Reading with native ShapeHealing structure as a parameter that used for filling up map
- Prepared by user healing map AND the structure. Why both? - because idea to give option for freedom map it ONLY extend the not existed in structure parameters or values.
_
I still in progress of review, but I want to have you feedback about my logic.
I would avoid internal "special" definition of map. I even want prefer to make them hidden as much as possible if no other way.
|
|
||
| IGESControl_ActorWrite::ParameterMap IGESControl_ActorWrite::GetDefaultParams() | ||
| { | ||
| static ParameterMap aParams; |
There was a problem hiding this comment.
The method is not thread safety.
There 2 options:
- rely on static initialization (making some special class-holder that have some constructor)
- use std::call_once
- use mutex (less recommended)
| { | ||
| static ParameterMap aParams; | ||
| if (aParams.empty()) | ||
| { |
There was a problem hiding this comment.
The method is not thread safety.
There 2 options:
rely on static initialization (making some special class-holder that have some constructor)
use std::call_once
use mutex (less recommended)
There was a problem hiding this comment.
Additionally looks so close to IGESControl_ActorWrite.
They both used for writing. Probably need to combine somehow (or not)
| //! @return the default parameters for shape processing. | ||
| Standard_EXPORT static ParameterMap GetDefaultParams(); | ||
|
|
||
| private: | ||
| Handle(Transfer_FinderProcess) myTP; |
There was a problem hiding this comment.
just want to say that these pyramid looks pretty.
| //======================================================================= | ||
| //function : EncodeRegul | ||
| //purpose : INTERNAL to encode regularity on edges | ||
| //======================================================================= |
There was a problem hiding this comment.
new function can use the same description style as in header, but with // as a comment beginngin
Example
// INTERNAL to encode regularity on edges
//"@"param theShape ...
//"@"returns ...
"@" used because GitHub making reference to person
_
Or you can just have line with single line descrption.
Or no description.
But please do not use old template
//===
//function
//purpose
//====
It is not useful.
There was a problem hiding this comment.
Sorry, I see that you just move the code. You can not update in case if it is just moving.
| static ParameterMap aParams; | ||
| if (aParams.empty()) | ||
| { | ||
| DE_ShapeFixParameters aShapeFixParameters; |
There was a problem hiding this comment.
The same comment as for 2 previous IGES params.
Looks very interesting.
I hope the main idea - if someone on the top have map - it will be propagated and never will be used from the low level.
All instances of using XSAlgo_AlgoContainer are replaced with XSAlgo_ShapeProcessor.
Parameters for XSAlgo_ShapeProcessor operations are now can be passes via the updated interface of respective classes.