View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0022751Open CASCADEOCCT:Visualizationpublic2011-10-04 18:122012-03-29 17:26
ReportersanAssigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
PlatformAOSL 
Product Version6.5.2 
Target Version6.5.3Fixed in Version6.5.3 
Summary0022751: Issues around Prs3d_TextAspect::Print()
Description1. The code below that comes from Prs3d_TextAspect::Print() will result in memory corruption when this function is called and should be corrected:

  Standard_CString FontName;

  strcpy((char*)FontName,(char*)F);

2. Another question: do we need all this "printing stuff" in Prs3d, Graphic3d and OpenGl packages at all? Probably this is to be removed.
Additional information
and documentation updates		Print() methods from Prs3d package are “potentially helpful” for debugging, but aren’t currently used. Therefore, they were removed.
    And the code related to the unused "roles" from OpenGl package (see OpenGl_tsm.hxx: Print, PickTraverse and Inquire) was removed as well as from higher-level packages (Graphic3d, Visual3d, V3d).
TagsNo tags attached.
Test case numberTest is not requred

Activities

abv

2011-10-04 18:26

manager   ~0018320

I vote for removing printing stuff

san

2011-10-05 11:29

developer   ~0018322

I agree with that.

However, I want to assign this issue to a new probationer (DLN), so I suggest he treat it step-by-step.

First of all, he could correct the local memory management error. An intermediate patch could be prepared and integrated at this point.

Then, he could study OCCT visualization architecture and the way the "printing stuff" is implemented (most likely this will take some time). Finally, he should clean carefully Prs3d, OpenGl and maybe some other packages from the printing code.

san

2011-10-14 11:31

developer   ~0018361

Last edited: 2011-10-14 11:32

 I have some considerations related to problem 2.

After looking into Prs3d_Drawer::Print(), I think it is "potentially helpful" for debugging. An application can obtain a drawer instance and dump its contents to the console. Thus Print() methods of all Prs3d_*Aspect classes should also be kept...

On the other hand, I have never used this debugging technique when dealing with Prs3d_Drawer. Neither it is used by any DRAW command. Console dump is not adopted for debugging in other high-level visualization packages but Prs3d.
So "potentially helpful" for me means rather "purely theoretically helpful".

And from practice we know that the most common error about the drawer is that one never knows if he deals with an individual drawer instance for some interactive object or with a reference to the global drawer. And Print() method is of no use here. Moreover, keeping Print() method requires certain maintenance efforts when some aspect's API is changed.

So my conclusion is that it makes sense to remove Print() method from Prs3d_Drawer and aspect classes, and there is no need in correction of the local memory usage error (uninitialized Standard_CString).

Another issue is related to unused printing stuff in OpenGl package.
Currently, each element type (graphic primitive or visual attribute) should implement several functions with the following "roles" (see OpenGl_tsm.hxx):
- PickTraverse
- DisplayTraverse
- Add
- Delete
- Print
- Inquire

However, it seems that only Display, Add and Delete are really used.
PickTraverse and Print are really obsolete, while I have some doubts about Inquire (there are some "Inquire*" methods in OpenGl_GraphicDriver.cdl).
This point should be analyzed carefully and the conclusion should be posted here in order to take the final decision whether to keep or remove roles PickTraverse, Print and Inquire as well as corresponding functions in all OpenGl "element types".

san

2012-02-20 12:59

developer   ~0019663

Dear DLN,

Please, check if the current trunk already incorporates changes similar to those introduced by you.
If necessary, create a new SVN branch from the current trunk's HEAD and add missing corrections and finally assign the issue to me for reviewing.

kgv

2012-02-20 16:46

developer   ~0019675

Please consider OPENGL_NS_PICK enumeration value for removal too.

dln

2012-03-07 12:42

developer   ~0019891

For redesigned version of OpenGl driver were made similar changes.

Full list of changes:

  All Print() methods were removed from Prs3d package. This concerns the following files:

    Prs3d_AngleAspect.cdl;
    Prs3d_ArrowAspect.cdl;
    Prs3d_DatumAspect.cdl;
    Prs3d_IsoAspect.cdl;
    Prs3d_LengthAspect.cdl;
    Prs3d_LineAspect.cdl;
    Prs3d_PointAspect.cdl;
    Prs3d_TextAspect.cdl;
    Prs3d_Drawer.cdl.
 
  Also some unused methods were deleted from OpenGl package as well as from higher-level packages (Graphic3d, Visual3d, V3d). The touched files are listed below.

  - from OpenGl_GraphicDriver.hxx:

     Standard_Boolean ElementExploration( ... );
     Graphic3d_TypeOfPrimitive ElementType( ... );
     void DumpGroup(const Graphic3d_CGroup& ACGroup);
     void DumpView(const Graphic3d_CView& ACView);
     void DumpStructure( const Graphic3d_CStructure& ACStructure );
     void InitPick ();
     void Pick (Graphic3d_CPick& ACPick);
     void PickId (const Graphic3d_CGroup& ACGroup);

    And the same methods were deleted from Graphic3d_GraphicDriver.cdl.
 
  - from Graphic3d_Structure.cdl:

    Graphic3d_TypeOfPrimitive Graphic3d_Structure::Type( ... );
    Standard_Boolean Exploration( ... );
    void Exploration();

 - from Graphic3d_Group.cdl:

    void Exploration();
    Standard_Integer PickId();
    void RemovePickId();
    void SetPickId(const Standard_Integer Id);

 - from Visual3d_View.cdl:

    void Exploration();

 - from Visual3d_ViewManager.cdl:

    Visual3d_PickDescriptor Pick( ... );

 - from V3d_PositionLight.cdl:

    V3d_TypeOfPickLight Pick( ... );

Deleted files:
   V3d_Camera.cdl

dln

2012-03-07 14:02

developer   ~0019892

Last edited: 2012-03-07 14:02

 Dear SAN,

All changes in branch: http://svn/svn/occt/branches/OCC22751_compare

san

2012-03-07 15:59

developer   ~0019894

Last edited: 2012-03-07 16:10

 The SVN branch reviewed with the following remarks.

src/OpenGl/OpenGl_NamedStatus.hxx
========================================
What is the purpose of 2-bit gap between OPENGL_NS_HIDE and OPENGL_NS_HIGHLIGHT?

#define OPENGL_NS_HIDE (1<<0)
#define OPENGL_NS_HIGHLIGHT (1<<2)

I suggest each OPENGL_NS_ flag is always shifted by 1 bit with respect to the previous one.

src/Prs3d/Prs3d_AngleAspect.cxx
src/Prs3d/Prs3d_DatumAspect.cxx
src/Prs3d/Prs3d_Drawer.cxx
src/Prs3d/Prs3d_IsoAspect.cxx
src/Prs3d/Prs3d_LengthAspect.cxx
src/Prs3d/Prs3d_LineAspect.cxx
src/Prs3d/Prs3d_TextAspect.cxx
========================================
Please, remove the empty line at the end of the files.
Note that a single "end-of-line" character should be present at the end of each file, this remark is related to multiple EOLs only.

src/Visual3d/Visual3d_Layer.cxx
========================================
Line 18: Why Graphic3d_TypeOfPrimitive.hxx has been included? Is it needed?

san

2012-03-12 17:28

developer   ~0019938

The SVN branch reviewed without remarks, ready for testing.

mkv

2012-03-14 18:37

tester   ~0019974

Dear BugMaster,
SVN branch http://svn/svn/occt/branches/OCC22751_compare has been moved to GIT CR22751 one.
Workbench KAS:dev:mkv-22751-occt was created from git branch CR22751
(and mkv-22751-products from svn trunk) and compiled on Linux platform.

There are not regressions in mkv-22751-products regarding to KAS:dev:products-20120306-opt

See results in /QADisk/occttests/results/KAS/dev/mkv-22751-products_13032012/lin
See reference results in /QADisk/occttests/results/KAS/dev/products-20120306-opt_07032012/lin
See test cases in /QADisk/occttests/tests/ED
N.B. In order to launch testing case you can make use the following instructions
http://doc/doku.php?id=occt:certification

bugmaster

2012-03-15 19:09

administrator   ~0020000

Integrated into master

Related Changesets

occt: master 8ca7beb8

2012-03-13 10:11:29

dln


Committer: bugmaster Details Diff		 0022751: Issues around Prs3d_TextAspect::Print() Affected Issues
0022751
mod - src/Graphic3d/FILES Diff File
mod - src/Graphic3d/Graphic3d_GraphicDriver.cdl Diff File
mod - src/Graphic3d/Graphic3d_Group.cdl Diff File
mod - src/Graphic3d/Graphic3d_Group_2.cxx Diff File
mod - src/Graphic3d/Graphic3d_Group_4.cxx Diff File
mod - src/Graphic3d/Graphic3d_Structure.cdl Diff File
mod - src/Graphic3d/Graphic3d_Structure.cxx Diff File
mod - src/OpenGl/FILES Diff File
mod - src/OpenGl/OpenGl_GraphicDriver.hxx Diff File
mod - src/OpenGl/OpenGl_GraphicDriver_4.cxx Diff File
mod - src/OpenGl/OpenGl_GraphicDriver_5.cxx Diff File
mod - src/OpenGl/OpenGl_GraphicDriver_6.cxx Diff File
mod - src/OpenGl/OpenGl_GraphicDriver_7.cxx Diff File
mod - src/OpenGl/OpenGl_NamedStatus.hxx Diff File
mod - src/Prs3d/Prs3d_AngleAspect.cdl Diff File
mod - src/Prs3d/Prs3d_AngleAspect.cxx Diff File
mod - src/Prs3d/Prs3d_ArrowAspect.cdl Diff File
mod - src/Prs3d/Prs3d_ArrowAspect.cxx Diff File
mod - src/Prs3d/Prs3d_DatumAspect.cdl Diff File
mod - src/Prs3d/Prs3d_DatumAspect.cxx Diff File
mod - src/Prs3d/Prs3d_Drawer.cdl Diff File
mod - src/Prs3d/Prs3d_Drawer.cxx Diff File
mod - src/Prs3d/Prs3d_IsoAspect.cdl Diff File
mod - src/Prs3d/Prs3d_IsoAspect.cxx Diff File
mod - src/Prs3d/Prs3d_LengthAspect.cdl Diff File
mod - src/Prs3d/Prs3d_LengthAspect.cxx Diff File
mod - src/Prs3d/Prs3d_LineAspect.cdl Diff File
mod - src/Prs3d/Prs3d_LineAspect.cxx Diff File
mod - src/Prs3d/Prs3d_PointAspect.cdl Diff File
mod - src/Prs3d/Prs3d_PointAspect.cxx Diff File
mod - src/Prs3d/Prs3d_TextAspect.cdl Diff File
mod - src/Prs3d/Prs3d_TextAspect.cxx Diff File
mod - src/V3d/V3d.cdl Diff File
mod - src/V3d/V3d_Camera.cdl Diff File
mod - src/V3d/V3d_Camera.cxx Diff File
mod - src/V3d/V3d_DirectionalLight.cxx Diff File
mod - src/V3d/V3d_PositionalLight.cxx Diff File
mod - src/V3d/V3d_PositionLight.cdl Diff File
mod - src/V3d/V3d_PositionLight.cxx Diff File
mod - src/V3d/V3d_SpotLight.cxx Diff File
mod - src/Visual3d/Visual3d_Layer.cxx Diff File
mod - src/Visual3d/Visual3d_View.cdl Diff File
mod - src/Visual3d/Visual3d_View.cxx Diff File
mod - src/Visual3d/Visual3d_ViewManager.cdl Diff File
mod - src/Visual3d/Visual3d_ViewManager.cxx Diff File

Issue History

Date Modified Username Field Change
2011-10-04 18:12 san New Issue
2011-10-04 18:12 san Assigned To => san
2011-10-04 18:26 abv Note Added: 0018320
2011-10-04 18:26 abv Status new => assigned
2011-10-05 11:29 san Note Added: 0018322
2011-10-07 12:40 san Assigned To san => dln
2011-10-14 11:31 san Note Added: 0018361
2011-10-14 11:31 san Note Edited: 0018361
2011-10-14 11:32 san Note Edited: 0018361
2012-01-19 15:04 dln Additional Information Updated
2012-01-19 15:04 dln Assigned To dln => san
2012-01-19 15:04 dln Status assigned => resolved
2012-02-06 20:22 san Assigned To san => bugmaster
2012-02-06 20:29 san Assigned To bugmaster => san
2012-02-20 12:59 san Note Added: 0019663
2012-02-20 12:59 san Assigned To san => dln
2012-02-20 12:59 san Status resolved => assigned
2012-02-20 16:46 kgv Note Added: 0019675
2012-03-07 12:42 dln Note Added: 0019891
2012-03-07 14:02 dln Note Added: 0019892
2012-03-07 14:02 dln Note Edited: 0019892
2012-03-07 14:02 dln Assigned To dln => san
2012-03-07 14:03 dln Status assigned => resolved
2012-03-07 15:59 san Note Added: 0019894
2012-03-07 15:59 san Assigned To san => dln
2012-03-07 15:59 san Status resolved => assigned
2012-03-07 16:10 san Note Edited: 0019894
2012-03-11 12:33 dln Assigned To dln => san
2012-03-11 12:34 dln Status assigned => resolved
2012-03-12 17:28 san Note Added: 0019938
2012-03-12 17:28 san Assigned To san => bugmaster
2012-03-12 17:28 san Status resolved => reviewed
2012-03-12 20:03 mkv Assigned To bugmaster => mkv
2012-03-14 18:37 mkv Note Added: 0019974
2012-03-14 18:37 mkv Test case number => Test is not requred
2012-03-14 18:37 mkv Assigned To mkv => bugmaster
2012-03-14 18:37 mkv Status reviewed => tested
2012-03-15 19:09 bugmaster Note Added: 0020000
2012-03-15 19:09 bugmaster Status tested => verified
2012-03-15 19:09 bugmaster Resolution open => fixed
2012-03-15 19:09 bugmaster Assigned To bugmaster => dln
2012-03-29 17:26 bugmaster Changeset attached => occt master 8ca7beb8