View Issue Details

IDProjectCategoryView StatusLast Update
0033788Open CASCADEOCCT:Data Exchangepublic2024-08-10 13:53
Reporterichesnokova Assigned Tobugmaster  
PrioritynormalSeverityminor 
Status verifiedResolutionfixed 
Target Version7.8.1Fixed in Version7.9.0 
Summary0033788: Data Exchange, DE Wrapper - Shape Healing configuration node
DescriptionNeed to create a new class to keep Shape healing details as a base class inherited from DE_ConfigurationNode.
New class will be used as a parent for STEP, IGES, HOOPS, DXF, Parasolid Configuration Nodes component.
Current task is create a Healing parameters in separated classes and define specific class enumeration for them. (3 possible value: -1, 0, 1). Needs to decide need we these 3 or just bool.
This issue is not about deep integration of using this classes. Result for that issue is only update existed classes’s parent and create new class to inherit from.
The new class should be close for example to STEPCafControl_ConfigurationNode. The main idea is keep ShapeHealing parameters. But additionally needs to be able to extract information from StaticInteface with with some method.
Task to upgrade ShapeHealing logic is separated and will be created based on current implementation.
TagsNo tags attached.
Test case numbernone

Activities

git

2024-08-05 10:20

administrator   ~0116363

Branch CR33788 has been created by ichesnokova.

SHA-1: 74a953d4fc194e19c653f8d15422781d0710ef84


Detailed log of new commits:

Author: ichesnok
Date: Sun Aug 4 20:53:36 2024 +0100

    0033788: Data Exchange, DE Wrapper - Shape Healing configuration node
    
    DE_ShapeFixConfigurationNode parent class added for STEP, IGES, XT, DXF configuration nodes.
    DE_ShapeFixParameters class added for shape healing parameters storage.

ichesnokova

2024-08-05 10:46

developer   ~0116365

Dear dpasukhi,
please review CR33788 branches.

dpasukhi

2024-08-06 12:34

administrator   ~0116376

Last edited: 2024-08-06 12:37

In that case better to avoid creating a new dependency.
Better to avoid lining TKXDEBase, that means we will not need to reset parameters from Static_Interface for now.
Additionally, no needs to variables that keep information about format or other deteails to be used for Static_interface.
Probably for now better to not touch any other ConfigNodes. Create only single one and do not use it inside that ticket. Please remove PROD branch and revert you changes for STEP and IGES

git

2024-08-07 12:48

administrator   ~0116404

Branch CR33788 has been updated forcibly by ichesnokova.

SHA-1: af1d06fd35c829d3da45e15e09949649072d75a6

ichesnokova

2024-08-07 12:59

developer   ~0116405

Dear dpasukhi,
please review CR33788 branch again

dpasukhi

2024-08-07 15:16

administrator   ~0116406

DE_ShapeFixParameters -> convert to struct without cxx and any constructors.

No needs for any overriding methods and other options
  //! Creates new provider for the own format
  //! @return new created provider
  Standard_EXPORT virtual Handle(DE_Provider) BuildProvider() Standard_OVERRIDE;

  //! Gets CAD format name of associated provider
  //! @return provider CAD format
  Standard_EXPORT virtual TCollection_AsciiString GetFormat() const Standard_OVERRIDE;

  //! Gets provider's vendor name of associated provider
  //! @return provider's vendor name
  Standard_EXPORT virtual TCollection_AsciiString GetVendor() const Standard_OVERRIDE;

  //! Gets list of supported file extensions
  //! @return list of extensions
  Standard_EXPORT virtual TColStd_ListOfAsciiString GetExtensions() const Standard_OVERRIDE;

Only that methods are necessary:
  //! Initializes all field by default
  Standard_EXPORT DE_ShapeFixConfigurationNode();

  //! Copies values of all fields
  //! @param[in] theConfigurationNode object to copy
  Standard_EXPORT DE_ShapeFixConfigurationNode(const Handle(DE_ShapeFixConfigurationNode)& theConfigurationNode);

  //! Updates values according the resource
  //! @param[in] theResource input resource to use
  //! @return True if Load was successful
  Standard_EXPORT virtual bool Load(const Handle(DE_ConfigurationContext)& theResource) Standard_OVERRIDE;

  //! Writes configuration to the string
  //! @return result resource string
  Standard_EXPORT virtual TCollection_AsciiString Save() const Standard_OVERRIDE;

  //! Copies values of all fields
  //! @return new object with the same field values
  Standard_EXPORT virtual Handle(DE_ConfigurationNode) Copy() const Standard_OVERRIDE;


As for a Load or Save. Needs to realize reading and saving operation. But when you writing a type of operation. Needs to use a GetVendor and GetFormat as a methods calls.
I mean, need to implement full support of dump and read ShapeHealing parameters. If you need to increase estimation - please do. But write a reason - for example, needs extra time to implement dump functionality of parameters

git

2024-08-08 12:52

administrator   ~0116419

Branch CR33788 has been updated forcibly by ichesnokova.

SHA-1: 59c9e9c246709cd69f8a087c62f8dca85432c051

git

2024-08-08 12:56

administrator   ~0116420

Branch CR33788 has been updated forcibly by ichesnokova.

SHA-1: 1fda511007b65cba23812900f0f4664df5ea7e4f

ichesnokova

2024-08-08 12:58

developer   ~0116421

Remarks were fixed, please review.

git

2024-08-08 16:51

administrator   ~0116427

Branch CR33788 has been updated forcibly by ichesnokova.

SHA-1: cd71828ab31ecbfdeb131ebb42cfa3830cc2ea28

dpasukhi

2024-08-08 21:01

administrator   ~0116446

Dear @ichesnokova
There are some small remarks.

Big remark. Load not consistent with Save. Your commands are not same. Needs to have the equal parameters name for both operation.
 176   aResult += "!\n";
 177   aResult += "!Defines the maximum allowable tolerance\n";
 178   aResult += "!Default value: 1.e-6. Available values: any real positive (non null) value\n";
 179   aResult += aScope + "Tolerance3d :\t " + HealingParameters.Tolerance3d + "\n";
 180   aResult += "!\n";

VS

  57   HealingParameters.MaxTolerance3d = theResource->RealVal("max.tolerance3d", HealingParameters.MaxTolerance3d, aScope);

max.tolerance3d != Tolerance3d


Separator is not necessary, please remove the next lines (I start them from !!!!!:
 165 TCollection_AsciiString DE_ShapeFixConfigurationNode::Save() const
 166 {
 167   TCollection_AsciiString aResult;
!!!!!!!!!! 168   aResult += "!*****************************************************************************\n";
!!!!!!!!!! 169   aResult = aResult + "!Configuration Node " + " Vendor: " + GetVendor() + " Format: " + GetFormat() + "\n";
 170   TCollection_AsciiString aScope = THE_CONFIGURATION_SCOPE() + "." + GetFormat() + "." + GetVendor() + ".healing";


Needs to inherit from "unsigned char"
  22   //! Enum, classifying a type of value for parameters
  23   enum class FixValue : char
  24   {

Looks like string not need anymore
 15 #define _DE_ShapeFixParameters_HeaderFile
  16 
  17 #include <TCollection_AsciiString.hxx>
  18 
  19 //! Struct for shape healing parameters storage

Recommended to use single style of commenting (especially not overuse articles in the development comments)
Comments should be consistent
  25     FixOrNot = -1, //!< procedure will be called or not called, depending on the situation
  26     NotFix = 0,    //!< the procedure is not executed
  27     Fix = 1        //!< the procedure is executed anyway

Order of includes is not correct
// Header
 17 #include <DE_ConfigurationNode.hxx>
  18 #include <TColStd_ListOfAsciiString.hxx>
  19 #include <DE_ShapeFixParameters.hxx>

// Source
  16 #include <DE_PluginHolder.hxx>
  17 #include <DE_Wrapper.hxx>
  18 #include <DE_ConfigurationContext.hxx>

git

2024-08-09 10:29

administrator   ~0116465

Branch CR33788 has been updated forcibly by ichesnokova.

SHA-1: 10d3a2a7c33b0581fe0c253714c1629e5b083f08

git

2024-08-09 10:31

administrator   ~0116466

Branch CR33788 has been updated forcibly by ichesnokova.

SHA-1: 4f3f1809f9a12c59795c2e131d568f3968e1fc09

ichesnokova

2024-08-09 10:33

developer   ~0116467

Remarks were fixed.
@dpasukhi, please review branch.

dpasukhi

2024-08-09 11:28

administrator   ~0116468

Last edited: 2024-08-09 11:29

Dear @ichesnokova
There are a few short remarks:

Please update you git message to avoid format specific data. Please update commit message accordingly your changes.

I feel that name is not matched with the goal of enum.
I recommend to rename into FixMode or any other, I'm welcome to discussion.
To rename enum it will be enough to click RMB(ПКМ) and choose rename and it will rename anything related with this enum name.
  20   //! Enum, classifying a type of value for parameters
  21   enum class FixValue : char

git

2024-08-09 11:46

administrator   ~0116469

Branch CR33788 has been updated forcibly by ichesnokova.

SHA-1: 43615194ad2f27c3168deb049c29ed7642c1a4b0

dpasukhi

2024-08-09 12:20

administrator   ~0116473

Dear bugmaster, please integrate
OCCT: CR33788
PROD: -

Issue History

Date Modified Username Field Change
2024-08-05 09:54 ichesnokova New Issue
2024-08-05 09:54 ichesnokova Assigned To => ichesnokova
2024-08-05 10:20 git Note Added: 0116363
2024-08-05 10:46 ichesnokova Assigned To ichesnokova => dpasukhi
2024-08-05 10:46 ichesnokova Status new => resolved
2024-08-05 10:46 ichesnokova Note Added: 0116365
2024-08-06 12:34 dpasukhi Assigned To dpasukhi => ichesnokova
2024-08-06 12:34 dpasukhi Status resolved => assigned
2024-08-06 12:34 dpasukhi Note Added: 0116376
2024-08-06 12:36 dpasukhi Note Edited: 0116376
2024-08-06 12:37 dpasukhi Note Edited: 0116376
2024-08-06 12:37 dpasukhi Project Internal => Open CASCADE
2024-08-07 12:48 git Note Added: 0116404
2024-08-07 12:59 ichesnokova Assigned To ichesnokova => dpasukhi
2024-08-07 12:59 ichesnokova Status assigned => resolved
2024-08-07 12:59 ichesnokova Note Added: 0116405
2024-08-07 15:16 dpasukhi Note Added: 0116406
2024-08-07 15:16 dpasukhi Assigned To dpasukhi => ichesnokova
2024-08-07 15:16 dpasukhi Status resolved => assigned
2024-08-08 12:52 git Note Added: 0116419
2024-08-08 12:56 git Note Added: 0116420
2024-08-08 12:58 ichesnokova Assigned To ichesnokova => dpasukhi
2024-08-08 12:58 ichesnokova Status assigned => resolved
2024-08-08 12:58 ichesnokova Note Added: 0116421
2024-08-08 16:51 git Note Added: 0116427
2024-08-08 21:01 dpasukhi Assigned To dpasukhi => ichesnokova
2024-08-08 21:01 dpasukhi Status resolved => assigned
2024-08-08 21:01 dpasukhi Note Added: 0116446
2024-08-09 10:29 git Note Added: 0116465
2024-08-09 10:31 git Note Added: 0116466
2024-08-09 10:33 ichesnokova Assigned To ichesnokova => dpasukhi
2024-08-09 10:33 ichesnokova Status assigned => resolved
2024-08-09 10:33 ichesnokova Note Added: 0116467
2024-08-09 11:28 dpasukhi Note Added: 0116468
2024-08-09 11:29 dpasukhi Assigned To dpasukhi => ichesnokova
2024-08-09 11:29 dpasukhi Status resolved => assigned
2024-08-09 11:29 dpasukhi Note Edited: 0116468
2024-08-09 11:46 git Note Added: 0116469
2024-08-09 12:19 ichesnokova Assigned To ichesnokova => dpasukhi
2024-08-09 12:19 ichesnokova Status assigned => resolved
2024-08-09 12:20 dpasukhi Assigned To dpasukhi => bugmaster
2024-08-09 12:20 dpasukhi Status resolved => reviewed
2024-08-09 12:20 dpasukhi Note Added: 0116473
2024-08-10 13:53 dpasukhi Status reviewed => verified
2024-08-10 13:53 dpasukhi Resolution open => fixed
2024-08-10 13:53 dpasukhi Fixed in Version => 7.9.0
2024-08-10 13:53 dpasukhi Test case number => none