MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0027970Open CASCADE[OCCT] OCCT:Application Frameworkpublic2016-10-17 11:572017-03-24 11:59
Reporterszy 
Assigned Toszy 
PrioritynormalSeverityminor 
StatusassignedResolutionopen 
PlatformOSOS Version
Product Version 
Target Version[OCCT] 7.2.0*Fixed in Version 
Summary0027970: Improvement of standard attributes usability - containers
DescriptionCurrent OCAF approach allows to set on the same label only one Attribute of the same type, for example only one TDataStd_RealArray attribute.
It is proposed to remove this limitation by adding so called 'user defined' feature to the attribute. I.e. it is proposed to keep GUID (identifying the attribute) as internal field.

The next list of attributes is supposed to be modified:

TDataStd_RealArray
TDataStd_IntegerArray
TDataStd_BooleanArray
TDataStd_ReferenceArray
TDataStd_ByteArray
TDataStd_ExtStringArray

TDataStd_RealList
TDataStd_IntegerList
TDataStd_BooleanList
TDataStd_ExtStringList
TDataStd_ReferenceList
TDataStd_ListOfByte

TDataStd_IntPackedMap
Steps To ReproduceReview it, please.
TagsNo tags attached.
Test case numbercaf basic
Attached Files7z file icon data_for_27970.7z (1,079 bytes) 2017-03-07 16:03

- Relationships
related to 0028446assignedszy Could not retrieve just kept document with Integer attribute 
parent of 0028563assignedszy Fixing Document version for improved Containers 
Not all the children of this issue are yet resolved or closed.

-  Notes
(0063249)
git (administrator)
2017-01-31 12:43

Branch CR27970 has been created by szy.

SHA-1: b75aafacb3c349252da8479c0a9d8fc240306faf


Detailed log of new commits:

Author: szy
Date: Tue Jan 31 12:43:05 2017 +0300

    0027970: Improvement of standard attributes usability - containers.
(0063494)
git (administrator)
2017-02-07 15:03

Branch CR27970 has been updated by szy.

SHA-1: 5e0317d1ae2a44c4e228292208166ac325eb27ee


Detailed log of new commits:

Author: szy
Date: Tue Feb 7 15:03:03 2017 +0300

    27970: Improvement of standard attributes usability - containers.
    
    New testing scripts integration (test/caf/basic).

(0063703)
mpv (developer)
2017-02-08 17:02

Please, test.
(0063709)
git (administrator)
2017-02-08 17:59

Branch CR27970 has been updated forcibly by apv.

SHA-1: dc2eba3eef3c57d56ec5d8735ec9cd795ab2f45e
(0063710)
apv (tester)
2017-02-08 17:59

Branch CR27970 has been rebased on the current master
(0063711)
git (administrator)
2017-02-08 18:00

Branch CR27970 has been updated forcibly by mkv.

SHA-1: ee5cc8d5747380db4b2658e897c75cf5faa1a2d4
(0063739)
apv (tester)
2017-02-09 17:06
edited on: 2017-02-09 17:06

Dear BugMaster,

Branch CR27970 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: ee5cc8d5747380db4b2658e897c75cf5faa1a2d4

Number of compiler warnings:
occt component:
   Linux: 0 (0 on master)
   Windows: 0 (0 on master)
   MasOS: 0 (0 on master)
products component:
   Linux: 64
   Windows: 0
   MacOS: 1205

Regressions/Differences:
http://occt-tests/CR27970-master-OCCT/Debian70-64/summary.html [^]
http://occt-tests/CR27970-master-OCCT/Windows-64-VC10/summary.html [^]
caf basic W1, W2, W3, W4, W5, W6, W7, W8, W9, W10, W11
gdt dimensions A1, A2, A3, A4, A5, A6, A7, A8, A9
gdt export A9, B1, B2
gdt import A1, A2, A3, A4, A5
gdt tolerances A1, A2
http://occt-tests/CR27970-master-Products/Debian70-64/summary.html [^]
http://occt-tests/CR27970-master-Products/Windows-64-VC10/summary.html [^]
pmivis import places, select

Testing cases:
caf basic - FAILED
http://occt-tests/CR27970-master-OCCT/Debian70-64/summary.html#caf-basic [^]
http://occt-tests/CR27970-master-OCCT/Windows-64-VC10/summary.html#caf-basic [^]

Testing on Linux:
Total MEMORY difference: 92176493 / 91224989 [+1.04%]
Total CPU difference: 19847.09000000038 / 19650.730000000327 [+1.00%]

Testing on Windows:
Total MEMORY difference: 57126085 / 57124391 [+0.00%]
Total CPU difference: 18276.25035469861 / 18512.045866198645 [-1.27%]

There are differences in images found by testdiff:
http://occt-tests/CR27970-master-Products/Debian70-64/diff-Debian70-64.html [^]
http://occt-tests/CR27970-master-Products/Windows-64-VC10/diff-Windows-64-VC10-image.html [^]
pmivis import nist_ct_08_inch
pmivis import nist_ct_06_inch
pmivis import places
pmivis import duplicate
pmivis import select
pmivis import nist_ct_08_mm
pmivis import nist_ct_06_mm
pmivis import nist_ct_09_mm
pmivis import nist_ctc_04
pmivis import nist_ct_09_inch
pmivis import xcaf
pmivis import nist_ctc_01
pmivis import nist_ctc_03

(0063740)
apv (tester)
2017-02-09 17:08

Dear Sergey,

Branch CR27970 has been rejected due to:
- regressions/differences/improvements
- failed test cases for issue
- differences in images
(0064177)
szy (developer)
2017-03-07 16:05

Added additional data (see the attached file data_for_27970.7z) - to be used for failed cases "caf basic W1, W2, W3, W4, W5, W6, W7, W8, W9, W10, W11".
(0064178)
git (administrator)
2017-03-07 16:15

Branch CR27970_1 has been created by szy.

SHA-1: ac35c4f2061b334612b7888ecebfd0ebbb258715


Detailed log of new commits:

Author: szy
Date: Tue Mar 7 16:15:17 2017 +0300

    27970: Improvement of standard attributes usability - containers.
    
    Branch with initial data (27970) plus current corrections.
(0064179)
szy (developer)
2017-03-07 16:18

Test it, please.
Use the attached archive with data (*.7z) necessary for testing cases "caf / basic / W1 - W11.
(0064233)
git (administrator)
2017-03-10 12:36

Branch CR27970_1 has been updated by apv.

SHA-1: 954a25f0c35706b0d6153be45b4fe10c7540af55


Detailed log of new commits:

Author: apv
Date: Fri Mar 10 12:36:20 2017 +0300

    Minor corrections in new test cases for the issue

(0064234)
apv (tester)
2017-03-10 13:51
edited on: 2017-03-15 13:46

Dear BugMaster,

Branch CR27970_1 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: ac35c4f2061b334612b7888ecebfd0ebbb258715

Number of compiler warnings:
occt component:
   Linux: 0 (0 on master)
   Windows: 0 (0 on master)
   MasOS: 0 (0 on master)
products component:
   Linux: 64
   Windows: 0
   MacOS: 1187

Regressions/Differences:
http://occt-tests/CR27970_1-master-OCCT/Debian70-64/summary.html [^]
http://occt-tests/CR27970_1-master-OCCT/Windows-64-VC10/summary.html [^]
bugs caf bug24164_2

Testing cases:
caf basic - OK
http://occt-tests/CR27970_1-master-OCCT/Debian70-64/summary.html#caf-basic [^]
http://occt-tests/CR27970_1-master-OCCT/Windows-64-VC10/summary.html#caf-basic [^]

Testing on Linux:
Total MEMORY difference: 93018395 / 92345109 [+0.73%]
Total CPU difference: 20017.850000000177 / 19840.8000000004 [+0.89%]

Testing on Windows:
Total MEMORY difference: 57672371 / 57663643 [+0.02%]
Total CPU difference: 18678.95133609855 / 18635.5206576986 [+0.23%]

(0064235)
apv (tester)
2017-03-10 13:52

Dear Sergey,

Branch CR27970_1 has been rejected due to:
- regressions/differences/improvements
(0064285)
git (administrator)
2017-03-13 17:53

Branch CR27970_1 has been updated by szy.

SHA-1: 5c5931c460616fe20613b22f3177e1efe336f1ba


Detailed log of new commits:

Author: szy
Date: Mon Mar 13 17:53:07 2017 +0300

    27970: Improvement of standard attributes usability - containers.
    
    Minor fix of the Draw command - DDataStd_SetRefArray.

(0064286)
szy (developer)
2017-03-13 17:55

Fixed (minor fix).
test it, please.
(0064338)
apv (tester)
2017-03-15 13:45

Dear BugMaster,

Branch CR27970_1 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: 5c5931c460616fe20613b22f3177e1efe336f1ba

Number of compiler warnings:
occt component:
   Linux: 0 (0 on master)
   Windows: 0 (0 on master)
   MasOS: 0 (0 on master)
products component:
   Linux: 64
   Windows: 0
   MacOS: 1194

Regressions/Differences:
Not detected

Testing cases:
caf basic - OK
http://occt-tests/CR27970_1-master-OCCT/Debian70-64/summary.html#caf-basic [^]
http://occt-tests/CR27970_1-master-OCCT/Windows-64-VC10/summary.html#caf-basic [^]

Testing on Linux:
Total MEMORY difference: 92961018 / 92350545 [+0.66%]
Total CPU difference: 20030.77000000019 / 19840.8900000004 [+0.96%]

Testing on Windows:
Total MEMORY difference: 57670239 / 57663643 [+0.01%]
Total CPU difference: 18567.067418898383 / 18635.5206576986 [-0.37%]
(0064410)
git (administrator)
2017-03-17 18:45

Branch CR27970_2 has been created by szy.

SHA-1: c6d3bbed952c3c150d775da4dcd9eac91cc998af


Detailed log of new commits:

Author: szy
Date: Fri Mar 17 18:45:27 2017 +0300

    27970: Improvement of standard attributes usability - containers.
    
    Consolidated patch. Contains also document version fix.
(0064411)
szy (developer)
2017-03-17 18:48

Reviewed.
Contains in addition to the version CR27970_1 additional fix of document version.
Test it, please once again.
(0064535)
apv (tester)
2017-03-20 15:51

Dear BugMaster,

Branch CR27970_2 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: c6d3bbed952c3c150d775da4dcd9eac91cc998af

Number of compiler warnings:
occt component:
   Linux: 0 (0 on master)
   Windows: 0 (0 on master)
   MasOS: 0 (0 on master)
products component:
   Linux: 64
   Windows: 0
   MacOS: 1190

Regressions/Differences:
Not detected

Testing cases:
caf basic - OK
http://occt-tests/CR27970_2-master-OCCT/Debian70-64/summary.html#caf-basic [^]
http://occt-tests/CR27970_2-master-OCCT/Windows-64-VC10/summary.html#caf-basic [^]

Testing on Linux:
Total MEMORY difference: 92917488 / 92535815 [+0.41%]
Total CPU difference: 20176.990000000165 / 20187.16000000011 [-0.05%]

Testing on Windows:
Total MEMORY difference: 57783059 / 57782265 [+0.00%]
Total CPU difference: 18700.838276398525 / 18782.551600198512 [-0.44%]
(0064656)
msv (developer)
2017-03-23 15:30

Please replace tabs with spaces in all newly added code. See coding rules:
https://dev.opencascade.org/doc/overview/html/occt_dev_guides__coding_rules.html#occt_coding_rules_3 [^]
(0064667)
git (administrator)
2017-03-23 18:27

Branch CR27970_3 has been created by szy.

SHA-1: 0670fc74d52bdbfdb752ad72392f1f6d28ec358a


Detailed log of new commits:

Author: szy
Date: Thu Mar 23 18:27:25 2017 +0300

    27970: Improvement of standard attributes usability - containers.
    
    Correction of formatting only.
(0064668)
szy (developer)
2017-03-23 18:33

Only formatting was changed. It is not necessary to retest.
Compilation is checked.
(0064674)
msv (developer)
2017-03-24 10:53

More remarks to overall development:

src\DDataStd\DDataStd_BasicCommands.cxx
- There is enormous code duplication. Total 22 new commands like the following were added with this patch:
  SetUDIntArray
  GetUDIntArray
At this, there are corresponding commands without 'UD' in the name:
  SetIntArray
  GetIntArray
Two versions differ only by added argument 'guid'. The implementation of new commands is the copy with addition of code processing of new argument.
It is very bad practice. It is better to leave the same set of commands, but add optional argument, like the following.
Was:
  SetIntArray DF entry isDelta From To [elmt1 elmt2 ...]
To do:
  SetIntArray DF entry [-guid <GUID>] isDelta From To [elmt1 elmt2 ...]
This will make usage of commands more clear and uniform.

src\StdLPersistent\StdLPersistent_Data.cxx
- The code of the method FixID() (at 43) looks not optimal. Moreover, it is not needed at all. Such approach is prone to errors: when a new attribute will become having user guid it will be an additional complexity for the developer to update this piece of code. Please use OOP approach. In this case the optimal way is to initialize the field myID of each attribute in its constructor.
- 83-84: why this code is not under condition 'if (aPAttrib)', as it was before the fix? With new code, access violation exception is possible.

src\TDataStd\TDataStd_BooleanArray.cxx
and other attributes cxx files (total 11 files)
- The methods 'Set(label, lower, upper)' and 'Set(label, guid, lower, upper)' have the same implementation with exception of guid. Please make the first method simply calling the second one:
  Set(label, GetID(), lower, upper);
(0064675)
oan (developer)
2017-03-24 11:48

DC,

I have just had a look at the commit for the pure interest only.

I have noticed that there are quite similar code parts in each driver class, e.g. BinMDataStd_BooleanArrayDriver:

//-------------------------------------------------------------------------
if(BinMDataStd::DocumentVersion() > 9) { // process user defined guid
  const Standard_Integer& aPos = theSource.Position();
  Standard_GUID aGuid;
  Standard_Boolean ok = theSource >> aGuid;
  if (!ok) {
    theSource.SetPosition(aPos);
    anAtt->SetID(TDataStd_BooleanArray::GetID());
    ok = Standard_True;
  } else {
    anAtt->SetID(aGuid);
  }
} else
   anAtt->SetID(TDataStd_BooleanArray::GetID());
//-------------------------------------------------------------------------

...and...

//-------------------------------------------------------------------------
// process user defined guid
if(anAtt->ID() != TDataStd_BooleanArray::GetID())
  theTarget << anAtt->ID();
//-------------------------------------------------------------------------

I suggest to create a template methods of base class or separate template functions implementing common functionality and taking type of container as template parameter, i.e. something like this:

template<class ContainerType>
Standard_Boolean processUDGuid(const BinObjMgt_Persistent& theSource, Handle(T)& anAtt)
{
Standard_Boolean ok = Standard_True;
if(BinMDataStd::DocumentVersion() > 9) { // process user defined guid
  const Standard_Integer& aPos = theSource.Position();
  Standard_GUID aGuid;
  ok = theSource >> aGuid;
  if (!ok) {
    theSource.SetPosition(aPos);
    anAtt->SetID(ContainerType::GetID());
    ok = Standard_True;
  } else {
    anAtt->SetID(aGuid);
  }
} else
   anAtt->SetID(ContainerType::GetID());

return ok;
}

template<class ContainerType>
Standard_Boolean processUDGuid(BinObjMgt_Persistent& theTarget, Handle(T)& anAtt)
{
if(anAtt->ID() != ContainerType::GetID())
  theTarget << anAtt->ID();
}


I hope this could help to simplify code.

Best regards.
(0064676)
oan (developer)
2017-03-24 11:59

For container classes from TDataStd it would probably be useful to introduce intermediate class-wrapper for TDF_Attribute defining "Standard_GUID myID;" field with related common operations. Then use this class as a parent for containers instead of TDF_Attribute.

This also could help to eliminate code duplications and ease further maintenance.

- Issue History
Date Modified Username Field Change
2016-10-17 11:57 szy New Issue
2016-10-17 11:57 szy Assigned To => szy
2016-10-17 11:58 szy Status new => assigned
2016-11-09 11:03 abv Target Version 7.1.0 => 7.2.0*
2017-01-31 12:43 git Note Added: 0063249
2017-02-07 15:03 git Note Added: 0063494
2017-02-07 15:04 szy Assigned To szy => mpv
2017-02-07 15:04 szy Status assigned => resolved
2017-02-07 15:04 szy Steps to Reproduce Updated View Revisions
2017-02-08 17:02 mpv Note Added: 0063703
2017-02-08 17:02 mpv Assigned To mpv => bugmaster
2017-02-08 17:02 mpv Status resolved => reviewed
2017-02-08 17:48 mkv Assigned To bugmaster => mkv
2017-02-08 17:53 apv Assigned To mkv => apv
2017-02-08 17:59 git Note Added: 0063709
2017-02-08 17:59 apv Note Added: 0063710
2017-02-08 18:00 git Note Added: 0063711
2017-02-09 13:00 apv Test case number => caf basic
2017-02-09 14:23 abv Relationship added related to 0028446
2017-02-09 17:06 apv Note Added: 0063739
2017-02-09 17:06 apv Assigned To apv => szy
2017-02-09 17:06 apv Status reviewed => assigned
2017-02-09 17:06 apv Note Edited: 0063739 View Revisions
2017-02-09 17:08 apv Note Added: 0063740
2017-03-07 16:03 szy File Added: data_for_27970.7z
2017-03-07 16:05 szy Note Added: 0064177
2017-03-07 16:15 git Note Added: 0064178
2017-03-07 16:16 szy Status assigned => resolved
2017-03-07 16:18 szy Note Added: 0064179
2017-03-07 16:18 szy Assigned To szy => bugmaster
2017-03-07 16:18 szy Status resolved => reviewed
2017-03-07 19:08 apv Assigned To bugmaster => apv
2017-03-10 12:36 git Note Added: 0064233
2017-03-10 13:51 apv Note Added: 0064234
2017-03-10 13:51 apv Assigned To apv => szy
2017-03-10 13:51 apv Status reviewed => assigned
2017-03-10 13:52 apv Note Added: 0064235
2017-03-13 17:53 git Note Added: 0064285
2017-03-13 17:54 szy Status assigned => resolved
2017-03-13 17:55 szy Note Added: 0064286
2017-03-13 17:55 szy Assigned To szy => apv
2017-03-13 17:55 szy Status resolved => reviewed
2017-03-15 13:45 apv Note Added: 0064338
2017-03-15 13:45 apv Assigned To apv => bugmaster
2017-03-15 13:45 apv Status reviewed => tested
2017-03-15 13:46 apv Note Edited: 0064234 View Revisions
2017-03-16 17:20 bugmaster Assigned To bugmaster => szy
2017-03-16 17:20 bugmaster Status tested => assigned
2017-03-17 08:19 abv Relationship added parent of 0028563
2017-03-17 18:45 git Note Added: 0064410
2017-03-17 18:46 szy Status assigned => resolved
2017-03-17 18:48 szy Note Added: 0064411
2017-03-17 18:48 szy Assigned To szy => apv
2017-03-17 18:48 szy Status resolved => reviewed
2017-03-20 15:51 apv Note Added: 0064535
2017-03-20 15:51 apv Assigned To apv => bugmaster
2017-03-20 15:51 apv Status reviewed => tested
2017-03-23 15:30 msv Note Added: 0064656
2017-03-23 15:30 msv Assigned To bugmaster => szy
2017-03-23 15:30 msv Status tested => assigned
2017-03-23 18:27 git Note Added: 0064667
2017-03-23 18:33 szy Note Added: 0064668
2017-03-23 18:33 szy Assigned To szy => msv
2017-03-23 18:33 szy Status assigned => resolved
2017-03-24 10:53 msv Note Added: 0064674
2017-03-24 10:53 msv Assigned To msv => szy
2017-03-24 10:53 msv Status resolved => assigned
2017-03-24 11:48 oan Note Added: 0064675
2017-03-24 11:59 oan Note Added: 0064676


Copyright © 2000 - 2017 MantisBT Team
Powered by Mantis Bugtracker