View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0033317Open CASCADEOCCT:Data Exchangepublic2023-01-16 19:072023-03-17 00:29
Reporterdpasukhi Assigned Todpasukhi  
PrioritynormalSeverityminor 
Status verifiedResolutionfixed 
Product Version7.7.0 
Target Version7.7.1 
Summary0033317: Data Exchange, Step Export - Ignoring color attached to the reference shape label
DescriptionRegression since 7.2
Exported step file has not any color on 7.7 or 7.6 version.
Color attached to the reference label that moved shape part.
Steps To Reproducepload ALL
XOpen d:/solids_7_7_0.xml D_old
WriteStep D_old d:/solids_master.stp
ReadStep D_new d:/solids_master.stp
TagsNo tags attached.
Test case number

Attached Files

  • solids_7_2_0.xml (4,137 bytes)
  • solids_7_7_0.xml (4,376 bytes)
  • tree.png (101,498 bytes)

Activities

dpasukhi

2023-01-16 19:08

administrator   ~0112828

OCAF xml document to export:
solids_7_2_0.xml (4,137 bytes)

dpasukhi

2023-01-16 19:10

administrator   ~0112829

Updated OCAF to 7.7 version
solids_7_7_0.xml (4,376 bytes)

dpasukhi

2023-01-16 19:12

administrator   ~0112830

tree.png (101,498 bytes)

git

2023-01-26 18:24

administrator   ~0113002

Branch CR33317 has been created by dpasukhi.

SHA-1: de9445b6798d7995a80a81c916750722c04580f3


Detailed log of new commits:

Author: dpasukhi
Date: Thu Jan 26 15:24:07 2023 +0000

    0033317: Data Exchange, Step Export - Ignoring color attached to the reference shape label
    
    Fixed problem with pure referencing.
      To export reference label to step we convert it to the assembly with one part.
      All attributes attached to the label should be moved to the new part.
      For working with it new map contains only pure reference labels, that converted to the part
      was implemented to the STEPCAFControl_Writer.

git

2023-01-26 21:59

administrator   ~0113003

Branch CR33317 has been updated forcibly by dpasukhi.

SHA-1: eb279ef7c7d9205ef59c23ea9d3dd1fe0cdbf061

git

2023-01-27 12:26

administrator   ~0113005

Branch CR33317 has been updated forcibly by dpasukhi.

SHA-1: 42d6fdb42886d291e9f1c304f8838a636f94d3af

git

2023-01-27 16:59

administrator   ~0113009

Branch CR33317 has been updated forcibly by dpasukhi.

SHA-1: a1b9efb6b5fc6825b323742f6ec840d58c907738

dpasukhi

2023-01-30 18:17

administrator   ~0113021

Dear ika,
please review OCCT CR33317

All touched test cases are OK, no regressions, see:
http://jenkins-test-10.nnov.opencascade.com/view/CR33317-master-dpasukhi/view/COMPARE/

I fixed problem with case which we create an assembly for reference label. Previously we just attach all attributes to the assembly entity, but it was incorrect.
Now I bind and process reference node as a part of assembly. A name of assembly have the same name as a single part.
The color problem was the reason of incorrect matching label and entity.

ika

2023-01-31 20:05

manager   ~0113033

Dear dpasukhi,

First of all, could you please close the documents after the test script work, I think this approach is safer because of a lot of very different test scenarios.
And could you also please add a test case for names, if you changed their export in the same way as colors?
-----------------------------
in WriteColors
+ Handle(XCAFDoc_ColorTool) aCTool = XCAFDoc_DocumentTool::ColorTool(aLabel);
+ Handle(XCAFDoc_VisMaterialTool) aMatTool = XCAFDoc_DocumentTool::VisMaterialTool(aLabel);
+ Handle(XCAFDoc_ShapeTool) aSTool = XCAFDoc_DocumentTool::ShapeTool(aLabel);
+ if (aSTool.IsNull() || aCTool.IsNull())
+ {
+ continue;
+ }
in WriteNames
+ Handle(XCAFDoc_ShapeTool) aSTool = XCAFDoc_DocumentTool::ShapeTool(aLabel);

could you please explain the reason for retrieving all the tools in every step of the loop, as for me, it just worsens the performance?
-----------------------------
I see that you tried to fix the code style in the updated code, what about these?
+ if (!aSTool->GetComponents(aLabel, compLabels))
+ Handle(StepRepr_RepresentationContext) Context = Styles.FindContext(aShape);
+ Handle(TCollection_HAsciiString) ReprName = new TCollection_HAsciiString("");
+ oldLengthlen = anOldItems->Length();
+ anInvItem.SetValue(styledItm);
and
  STEPConstruct_Styles Styles(theWS);
  STEPConstruct_DataMapOfAsciiStringTransient DPDCs;
  STEPConstruct_DataMapOfPointTransient ColRGBs;
-----------------------------
-#ifdef OCCT_DEBUG
- std::cout << "Warning: Cannot find SDR for " << S.TShape()->DynamicType()->Name() << std::endl;
-#endif
I am not sure that just removing the debug output is a good practice.

dpasukhi

2023-01-31 20:19

administrator   ~0113034

Dear @ika
>>Could you please close the documents after the test script work
Yes, It will be done.
>> And could you also please add a test case for names, if you changed their export in the same way as colors?
There are 5 attached test cases with pure reference label. I will add information about them on the next node.
>>could you please explain the reason for retrieving all the tools in every step of the loop, as for me, it just worsens the performance?
Yes, it impact on the performance, but it not powerful influence.
Moreover, it is necessary thing, because we provide interface with LabelSequense input. No one can guarantee that nobody do not try to use label from the different documents.
I understand, that this ability is not support and tested, but it is a fact that it is exist.
>>I see that you tried to fix the code style in the updated code, what about these?
Oh, I missed them, thank you, thank you)
>>I am not sure that just removing the debug output is a good practice.
You are right, I will update it as a "Messager::SendTrace() <<" that can print only in the special setting environmental.
Debug macros is not best practices too, we need to rebuild all solution to use this feature. So, the "SendTrack()" for the debug input is the best option, isn't it?

ika

2023-01-31 20:33

manager   ~0113035

Dear @dpasukhi,

>> Moreover, it is necessary thing, because we provide interface with LabelSequense input. No one can guarantee that nobody do not try to use label from the different documents.
Sounds interesting, but we have several similar methods WriteNames, Colors, Layers, ExternFiles, Props, PMIs... all of them have the same principle, based on that all the given labels are inside one document, you updated only two of them, please make all of them working in one way old or new, for your choice.

>> So, the "SendTrack()" for the debug input is the best option, isn't it?
Ok, let it be So.

git

2023-02-01 20:16

administrator   ~0113037

Branch CR33317 has been updated by dpasukhi.

SHA-1: 45ad194d2281082b882685e72ea4edfbfb56ad4b


Detailed log of new commits:

Author: dpasukhi
Date: Wed Feb 1 17:16:21 2023 +0000

    // fixing remarks
    Updated code style of the STEPCAFControl_Writer
    Improved ability to export labels from different documents
    Removed OCCT_Debug macros to print in Trace gravity

git

2023-02-01 20:21

administrator   ~0113038

Branch CR33317 has been updated by dpasukhi.

SHA-1: eb3149ceb62f82039efebe9f1e936d021c99be42


Detailed log of new commits:

Author: dpasukhi
Date: Wed Feb 1 17:21:36 2023 +0000

    // reverting printing info message for writeNames

git

2023-02-01 22:28

administrator   ~0113039

Branch CR33317 has been updated by dpasukhi.

SHA-1: 204904a8745cd8b488f2eeee5d4ba475e03ae127


Detailed log of new commits:

Author: dpasukhi
Date: Wed Feb 1 19:27:51 2023 +0000

    // fixed problem with layers

git

2023-02-02 12:37

administrator   ~0113042

Branch CR33317_1 has been created by dpasukhi.

SHA-1: c01f3a8da0b5af2d6c530d71149b56ad9d3797cc


Detailed log of new commits:

Author: dpasukhi
Date: Thu Jan 26 15:24:07 2023 +0000

    0033317: Data Exchange, Step Export - Ignoring color attached to the reference shape label
    
    Fixed problem with pure referencing.
      To export reference label to step we convert it to the assembly with one part.
      All attributes attached to the label should be moved to the new part.
      For working with it new map contains only pure reference labels, that converted to the part
      was implemented to the STEPCAFControl_Writer.
    Updated code style of the STEPCAFControl_Writer
    Improved ability to export labels from different documents
    Removed OCCT_Debug macros to print in Trace gravity

dpasukhi

2023-02-02 12:39

administrator   ~0113043

Dear ika,
please review OCCT: CR33317_1
All changes you can see within CR33317

All tests are OK, see
http://jenkins-test-10.nnov.opencascade.com/view/CR33317-master-dpasukhi/view/COMPARE/

I make a little code style updating of StepCafControl_Writer classes.
Additionally, the ability to work with labels from different documents improved.
All info printing moved to the print with 0 gravity using common interface

git

2023-02-02 16:05

administrator   ~0113045

Branch CR33317_1 has been updated forcibly by dpasukhi.

SHA-1: 656bf2ab210c342be7487ddf791e1a27778bee43

dpasukhi

2023-02-02 16:16

administrator   ~0113046

Test with checking names of pure ref label:
 bugs xde bug1669
bugs modalg_8 bug33165

ika

2023-02-02 20:39

manager   ~0113050

Dear dpasukhi,

some more missed in coding rules changing
name in
+ Standard_EXPORT Standard_Boolean ExternFile(const Standard_CString name,
+ Handle(STEPCAFControl_ExternFile)& theExtFile) const;

mi (it should be a sequence iterator, shouldn't it?) in
+ for (Standard_Integer mi = 1; mi <= aNewseqRI.Length(); mi++)

+ if (!getProDefinitionOfNAUO(theWS, aUUSh, nullPD, UUNAUO, Standard_True) ||
+ !getProDefinitionOfNAUO(theWS, aNUSh, aRelatedPD, NUNAUO, Standard_False))

+ aModel->AddWithRefs(SAR);

+ TDF_LabelSequence aaDGTLS;

+ Standard_Integer NbDR = aDatumLabels.Length();
------------------------------------------

+Standard_Boolean STEPCAFControl_Writer::transfer(STEPControl_Writer& theWriter,
+ const TDF_LabelSequence& theLabels,
+ const STEPControl_StepModelType theMode,
+ const Standard_CString theIsmMulti, <--------- misprint
+ const Standard_Boolean theIsExternFile,
+ const Message_ProgressRange& theProgress)
---------------------------------------------
+ theWriter.WS()->ComputeGraph(Standard_True);// added by skl 03.11.2003 since we use
+ // writer.Transfer() without compute graph
the second piece of comment now looks like a separate comment, please make this place readable
----------------------------------------------
- // write validation props
-// if ( multi && ap ==3 ) {
-// Interface_Static::SetCVal ("write.step.schema", "AP214DIS");
-// }
- if ( GetPropsMode() )
- WriteValProps ( writer.WS(), sublabels, multi );
+ if (GetPropsMode())
+ writeValProps(theWriter.WS(), aSubLabels, theIsmMulti);
 
- Interface_Static::SetIVal ("write.step.schema", ap);
+ Interface_Static::SetIVal("write.step.schema", aStepSchema);

I cannot understand the goal of setting schema here, it seems like an artifact of the old approach with changing schema to write props.
----------------------------------
If you made such significant work in code styling, could you please also fix these misprints?
+ // search for PSA of Monifold solid
+ // check that it is a stiled item for monifold solid brep

git

2023-02-03 00:28

administrator   ~0113051

Branch CR33317_1 has been updated by dpasukhi.

SHA-1: 3eae59574d95056c961d152f7b97544f657b2af5


Detailed log of new commits:

Author: dpasukhi
Date: Thu Feb 2 21:28:40 2023 +0000

    // fixing remarks

dpasukhi

2023-02-03 00:32

administrator   ~0113053

Last edited: 2023-02-03 00:33

 >>I cannot understand the goal of setting schema here, it seems like an artifact of the old approach with changing schema to write props.
I too. I try to find any internally setting, but i find nothing (no one changing of this parameter in any classes). I think it is an artefact. So, I removed returning(setting) of this interface parameter.

git

2023-02-03 02:09

administrator   ~0113054

Branch CR33317_2 has been created by dpasukhi.

SHA-1: 905290246ddd319038ae397a0dea039641c4aa70


Detailed log of new commits:

Author: dpasukhi
Date: Thu Jan 26 15:24:07 2023 +0000

    0033317: Data Exchange, Step Export - Ignoring color attached to the reference shape label
    
    Fixed problem with pure referencing.
      To export reference label to step we convert it to the assembly with one part.
      All attributes attached to the label should be moved to the new part.
      For working with it new map contains only pure reference labels, that converted to the part
      was implemented to the STEPCAFControl_Writer.
    Updated code style of the STEPCAFControl_Writer
    Improved ability to export labels from different documents
    Removed OCCT_Debug macros to print in Trace gravity

dpasukhi

2023-02-03 02:09

administrator   ~0113055

Dear ika,
please review CR33317_2 ( all changes within CR33317_1)

All tests are OK, see
http://jenkins-test-10.nnov.opencascade.com/view/CR33317-master-dpasukhi/view/COMPARE/

All remarks have been done.

ika

2023-02-03 14:22

manager   ~0113062

No more remarks.

Dear bugmaster,
please integrate
OCCT - CR33317_2

vglukhik

2023-02-05 16:31

administrator   ~0113074

IR-2023-02-03

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Ubuntu2004-64:
OCCT
Total CPU difference: 18415.850000000257 / 19028.28000000029 [-3.22%]
Products
Total CPU difference: 12109.420000000147 / 11752.84000000013 [+3.03%]
Windows-64-VC142:
OCCT
Total CPU difference: 20947.078125 / 20944.328125 [+0.01%]
Products
Total CPU difference: 13490.453125 / 13069.734375 [+3.22%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2023-02-05 16:34

administrator   ~0113077

Branch CR33317 has been deleted by mnt.

SHA-1: 204904a8745cd8b488f2eeee5d4ba475e03ae127

git

2023-02-05 16:34

administrator   ~0113078

Branch CR33317_1 has been deleted by mnt.

SHA-1: 3eae59574d95056c961d152f7b97544f657b2af5

git

2023-02-05 16:34

administrator   ~0113079

Branch CR33317_2 has been deleted by mnt.

SHA-1: 905290246ddd319038ae397a0dea039641c4aa70

Related Changesets

occt: master 47263fa6

2023-01-26 18:24:07

dpasukhi


Committer: vglukhik Details Diff		 0033317: Data Exchange, Step Export - Ignoring color attached to the reference shape label

Fixed problem with pure referencing.
To export reference label to step we convert it to the assembly with one part.
All attributes attached to the label should be moved to the new part.
For working with it new map contains only pure reference labels, that converted to the part
was implemented to the STEPCAFControl_Writer.
Updated code style of the STEPCAFControl_Writer
Improved ability to export labels from different documents
Removed OCCT_Debug macros to print in Trace gravity		 Affected Issues
0033317
mod - src/STEPCAFControl/STEPCAFControl_Writer.cxx Diff File
mod - src/STEPCAFControl/STEPCAFControl_Writer.hxx Diff File
mod - src/XCAFDoc/XCAFDoc_ColorTool.cxx Diff File
mod - src/XCAFDoc/XCAFDoc_ColorTool.hxx Diff File
mod - src/XCAFDoc/XCAFDoc_DimTolTool.cxx Diff File
mod - src/XCAFDoc/XCAFDoc_DimTolTool.hxx Diff File
mod - src/XCAFDoc/XCAFDoc_LayerTool.cxx Diff File
mod - src/XCAFDoc/XCAFDoc_LayerTool.hxx Diff File
mod - src/XCAFDoc/XCAFDoc_MaterialTool.cxx Diff File
mod - src/XCAFDoc/XCAFDoc_MaterialTool.hxx Diff File
mod - src/XCAFDoc/XCAFDoc_VisMaterialTool.cxx Diff File
mod - src/XCAFDoc/XCAFDoc_VisMaterialTool.hxx Diff File
add - tests/bugs/step/bug33317 Diff File

Issue History

Date Modified Username Field Change
2023-01-16 19:07 dpasukhi New Issue
2023-01-16 19:07 dpasukhi Assigned To => ichesnokova
2023-01-16 19:08 dpasukhi Note Added: 0112828
2023-01-16 19:08 dpasukhi File Added: solids_7_2_0.xml
2023-01-16 19:10 dpasukhi Note Added: 0112829
2023-01-16 19:10 dpasukhi File Added: solids_7_7_0.xml
2023-01-16 19:12 dpasukhi Note Added: 0112830
2023-01-16 19:12 dpasukhi File Added: tree.png
2023-01-16 19:17 dpasukhi Steps to Reproduce Updated
2023-01-16 19:17 dpasukhi Steps to Reproduce Updated
2023-01-17 14:55 dpasukhi Target Version 7.8.0 => 7.7.1
2023-01-26 17:45 dpasukhi Assigned To ichesnokova => dpasukhi
2023-01-26 18:24 git Note Added: 0113002
2023-01-26 19:03 dpasukhi Status new => assigned
2023-01-26 21:59 git Note Added: 0113003
2023-01-27 12:26 git Note Added: 0113005
2023-01-27 16:59 git Note Added: 0113009
2023-01-30 18:17 dpasukhi Assigned To dpasukhi => ika
2023-01-30 18:17 dpasukhi Status assigned => resolved
2023-01-30 18:17 dpasukhi Note Added: 0113021
2023-01-31 20:05 ika Assigned To ika => dpasukhi
2023-01-31 20:05 ika Status resolved => assigned
2023-01-31 20:05 ika Note Added: 0113033
2023-01-31 20:19 dpasukhi Note Added: 0113034
2023-01-31 20:33 ika Note Added: 0113035
2023-02-01 20:16 git Note Added: 0113037
2023-02-01 20:21 git Note Added: 0113038
2023-02-01 22:28 git Note Added: 0113039
2023-02-02 12:37 git Note Added: 0113042
2023-02-02 12:39 dpasukhi Assigned To dpasukhi => ika
2023-02-02 12:39 dpasukhi Status assigned => resolved
2023-02-02 12:39 dpasukhi Note Added: 0113043
2023-02-02 16:05 git Note Added: 0113045
2023-02-02 16:16 dpasukhi Note Added: 0113046
2023-02-02 20:39 ika Note Added: 0113050
2023-02-02 20:39 ika Assigned To ika => dpasukhi
2023-02-02 20:39 ika Status resolved => assigned
2023-02-03 00:28 git Note Added: 0113051
2023-02-03 00:32 dpasukhi Note Added: 0113053
2023-02-03 00:33 dpasukhi Note Edited: 0113053
2023-02-03 02:09 git Note Added: 0113054
2023-02-03 02:09 dpasukhi Assigned To dpasukhi => ika
2023-02-03 02:09 dpasukhi Status assigned => resolved
2023-02-03 02:09 dpasukhi Note Added: 0113055
2023-02-03 14:22 ika Assigned To ika => bugmaster
2023-02-03 14:22 ika Status resolved => reviewed
2023-02-03 14:22 ika Note Added: 0113062
2023-02-05 16:31 vglukhik Note Added: 0113074
2023-02-05 16:32 vglukhik Changeset attached => occt master 47263fa6
2023-02-05 16:32 dpasukhi Assigned To bugmaster => dpasukhi
2023-02-05 16:32 dpasukhi Status reviewed => verified
2023-02-05 16:32 dpasukhi Resolution open => fixed
2023-02-05 16:34 git Note Added: 0113077
2023-02-05 16:34 git Note Added: 0113078
2023-02-05 16:34 git Note Added: 0113079