DiscussionsIssue archiveOCCT:Visualization

Archived issue #0030857

Visualization - using one implementation of Text in graphic group

Open CASCADEOCCT:Visualizationclosed55 public notes

Search issues

Description

The expansion of Text() methods of Graphic3d_Group should last.

It's proposed to define a common structure for passing auxiliary parameters and Text representation modes, and replacing all Text() methods with single one (existing methods to be preserved for a while as deprecated redirections to new method).

Public activity

55 archived notes

Participants are labeled by their role within this record.

01Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Mon Aug 5 07:42:47 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction
    
    # Conflicts:
    # src/Graphic3d/Graphic3d_Group.cxx
02Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Mon Aug 5 08:18:54 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction

03Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Tue Aug 6 07:20:08 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    #compilation correction: Handle(Font_TextFormatter) will come with #0030537

04Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Mon Aug 5 07:42:47 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
05Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Wed Aug 7 08:59:10 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
06Author
Dear Kirill,

please, review proposed modifications.

Job: http://jenkins-test-12.nnov.opencascade.com/view/CR30857-master-NDS/

[signature removed]Natalia
07Commenter 3
+  const NCollection_String& Text() const { return myText; }
...
+  //! Returns text orientation in 3D space.
+  const gp_Ax2& Orientation() const { return myOrientation; }
+
+  Graphic3d_HorizontalTextAlignment HAlignment() const { return myHAlign; }
+  Graphic3d_VerticalTextAlignment VAlignment() const { return myVAlign; }

Setters are missing.
It is better providing setters defining orientation/3d point/anchor logic independent from initialization of other common fields like text, alignment, etc. instead of confusing big Init() _almost_everything_ methods.


   //! Creates the string <theText> at orientation <theOrientation> in 3D space.
+  Standard_DEPRECATED("Deprecated method Text() with obsolete arguments, use AddText() instead of it")
   Standard_EXPORT virtual void Text (const TCollection_ExtendedString&       
... 
+  //! Adds a text for display
+  Standard_EXPORT virtual void AddText (const Handle(Graphic3d_TextParams)& theTextParams,
+                                        const Standard_Boolean theToEvalMinMax = Standard_True);

Please move deprecated Text() methods to dedicated "public:" section at the end of class.

+  Graphic3d_Vertex myPoint;

It doesn't worth keep using this obsolete Graphic3d_Vertex class in new structures.
Please remove this field and store point inside gp_Ax2 field and use gp_Pnt for setters.

+  //! Creates the string <AText> at position <APoint>.
+  //! The 3D point of attachment is projected. The text is
+  //! written in the plane of projection.
+  //! The attributes are given with respect to the plane of
+  //! projection.
+  //! AHeight : Height of text.
+  //! (Relative to the Normalized Projection
+  //! Coordinates (NPC) Space).
+  //! AAngle  : Orientation of the text
+  //! (with respect to the horizontal).
+  Standard_EXPORT Graphic3d_TextParams (const Standard_Real theHeight);

Description does not match method.

+//! This class describes texture parameters.
+class Graphic3d_TextParams : public Standard_Transient

texture?
Graphic3d_TextParams does not look like a text parameters but more like Graphic3d_Text or Graphic3d_TextLabel.

08Author
Thank you for review.

It seems that OpenGl_TextParam in obsolete. What is your opinion, should it be removed in the current issue?

09Commenter 3
yes
10Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Aug 16 13:01:41 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    #issue remarks correction
11Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Aug 16 13:18:12 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
12Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Aug 15 13:17:18 2019 +0300

    0030901: Visualization - OSD_MemInfo moving memory computation out of the constructor
    
    (cherry picked from [revision removed])

13Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Tue Aug 20 09:41:23 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    #correction of warings on Linux, some minor code improve

14Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Tue Aug 20 09:43:32 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    #correction of warings on Linux, some minor code improve

15Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Tue Aug 20 10:12:34 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    Graphic3d_Group::Text() are obsolete, AddText() should be used instead of these methods.
    Introduced new class Graphic3d_Text for parameters necessary to fill OpenGl_Text. All parameters of Graphic3d_Group::Text() are united in this class.
    
    OpenGl_Text constructors with direct position/orientation values are obsolete, constructor with Graphic3d_Text should be used here.
    OpenGl_TextParam is obsolete, using of it is not supported any more.
16Author
Dear Kirill

remarks are corrected.
Please, check it once again.
17Commenter 3
+    aTextParams->SetPosition (gp_Pnt (aPlast.X(),aPlast.Y(),aPlast.Z()));

redundant conversion.

+//! More compilated formatting is available using Font_TextFormatter.
+//!
+class Graphic3d_Text : public Standard_Transien

Unexpected empty line in documentation.

+  //! Destructor.
+  Standard_EXPORT virtual ~Graphic3d_Text() {}

Inconsistent Standard_EXPORT for inline method.

+  //! Sets text value.
+  const NCollection_String& Text() const { return myText; }
+
+  //! Returns text value.
+  void SetText (const NCollection_String& theText) { myText = theText; }

Swapped description.

+  //! Sets text orientation in 3D space.
+  void SetOrientation (const gp_Ax2& theOrientation) { myOrientation = theOrientation; myHasPlane = Standard_True; }
+
+  //! Reset text orientation in 3D space.
+  void ResetOrientation() { myOrientation = gp_Ax2(); myHasPlane = Standard_False; }

Please split one-liners.

+  Handle(Font_TextFormatter) myTextFormatter; //!< text formatter collect information about text letter positions

collects.

+  NCollection_String myText; //!< text value
+  Handle(Font_TextFormatter) myTextFormatter; //!< text formatter collect information about text letter positions
+
+  bool myHasPlane; //!< Check if text have orientation in 3D space.
+  gp_Ax2 myOrientation; //!< Text orientation in 3D space.
+  Standard_Boolean myHasOwnAnchor; //!< flag if text uses position as point of attach
+
+  Standard_Real myHeight; //!< height of text
+  Graphic3d_HorizontalTextAlignment myHAlign; //!< horizontal alignment
+  Graphic3d_VerticalTextAlignment myVAlign; //!< vertical alignment

Please re-arrange fields by their size.

+#ifndef _Graphic3d_TextParams_HeaderFile
+#define _Graphic3d_TextParams_HeaderFile
...
+#endif // _Graphic3d_TextParams_HeaderFile

Should be updated.

+        aTextParams->SetText (aStr.ToCString());

Please add SetText() accepting TCollection_AsciiString.

-  aParams.Height = int ((theHeight < 2.0) ? aStruct->GlDriver()->DefaultTextHeight() : theHeight);
+  if (theTextParams->Height() < 2.0)
+    theTextParams->SetHeight (int (aStruct->GlDriver()->DefaultTextHeight()));

Please add TODO here - this should be handled in different way (throw exception / take default text height without modifying Graphic3d_Text / log warning, etc.).

+  Standard_EXPORT virtual void AddText (const Handle(Graphic3d_Text)& theTextParams,
+                                        const Standard_Boolean theToEvalMinMax = Standard_True) Standard_OVERRIDE;

Please skip default value for theToEvalMinMax in subclass.

  mutable GLdouble myExportHeight;

myExportHeight is used nowhere and can be removed.

+  Handle(Graphic3d_Text) myParams;

Should be first field.

  Standard_Boolean hasAnchorPoint() const { return myParams->HasOwnAnchorPoint() != Standard_False; }

Why "!= Standard_False"?

   theAttachmentPoint.Coord(x,y,z);
...
+  aTextParams->SetPosition (gp_Pnt (x,y,z));
...
+  aTextParams->SetPosition (gp_Pnt (pX.X(), pX.Y(), pX.Z()));
+  aTextParams->SetPosition (gp_Pnt (pY.X(), pY.Y(), pY.Z()));
+  aTextParams->SetPosition (gp_Pnt (pZ.X(), pZ.Y(), pZ.Z()));

Redundant conversion.
18Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Aug 22 07:51:46 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    # remarks correction

19Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Aug 22 08:05:53 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    Graphic3d_Group::Text() are obsolete, AddText() should be used instead of these methods.
    Introduced new class Graphic3d_Text for parameters necessary to fill OpenGl_Text. All parameters of Graphic3d_Group::Text() are united in this class.
    
    OpenGl_Text constructors with direct position/orientation values are obsolete, constructor with Graphic3d_Text should be used here.
    OpenGl_TextParam is obsolete, using of it is not supported any more.
20Author
Dear Kirill,

please, check the new branch for the issue.

Some remarks:
- Adding SetText(TCollection_AsciiString) gives errors if we do not cast (for example Standard_CString) to exact type (ambiguous call to overloaded function). Had I correctly understand your recoomendation - adding method, not replacing existing with NCollection_String?

- throwing an exception in if (theTextParams->Height() < 2.0) will give use different result in relation to the previous implementation. Do you validate it?

[signature removed]Natalia
21Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Aug 22 08:42:08 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    # warnings correction on Debian

22Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Aug 22 09:13:20 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    Graphic3d_Group::Text() are obsolete, AddText() should be used instead of these methods.
    Introduced new class Graphic3d_Text for parameters necessary to fill OpenGl_Text. All parameters of Graphic3d_Group::Text() are united in this class.
    
    OpenGl_Text constructors with direct position/orientation values are obsolete, constructor with Graphic3d_Text should be used here.
    OpenGl_TextParam is obsolete, using of it is not supported any more.
23Author
please, use CR30857_7
24Commenter 3
> - Adding SetText(TCollection_AsciiString) gives errors if we do not cast
> (for example Standard_CString) to exact type
> (ambiguous call to overloaded function).
> Had I correctly understand your recoomendation - adding method,
> not replacing existing with NCollection_String?
Ideally, NCollection_String/TCollection_AsciiString usage should become transparent to user, but this would be possible only after #0025308.

The proposed change is based on understanding that majority of OCCT users would use TCollection_AsciiString for manipulating strings instead of NCollection_String - so that this use case (TCollection_AsciiString input) should be more straightforward to user. At the same time, formatter uses NCollection_String also for concatenating final string, so that changing it's logic to use TCollection_AsciiString is probably suboptimal.

I see that just adding SetText with TCollection_AsciiString argument would lead to ambiguity for "const char*" input. So that to workaround it you'll have to add also SetText() with "const char*" input redirecting to NCollection_String.

> - throwing an exception in if (theTextParams->Height() < 2.0)
> will give use different result in relation to the previous implementation.
> Do you validate it?
For the moment, I do not propose changing behavior but to add TODO for another bug.
The question here is to identify why the code using DefaultTextHeight() has been introduced in the first place and where this scenario is currently used to decide how to solve the issue smoothly.
25Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Aug 22 11:32:02 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    # remarks correction

26Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Aug 22 11:40:29 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    Graphic3d_Group::Text() are obsolete, AddText() should be used instead of these methods.
    Introduced new class Graphic3d_Text for parameters necessary to fill OpenGl_Text. All parameters of Graphic3d_Group::Text() are united in this class.
    
    OpenGl_Text constructors with direct position/orientation values are obsolete, constructor with Graphic3d_Text should be used here.
    OpenGl_TextParam is obsolete, using of it is not supported any more.
27Author
Corrected.
28Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Aug 22 13:56:16 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    #remarks correction

29Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Aug 22 15:25:12 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    #remarks correction - Init with text parameters is obsolete

30Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Aug 22 15:41:07 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    Graphic3d_Group::Text() are obsolete, AddText() should be used instead of these methods.
    Introduced new class Graphic3d_Text for parameters necessary to fill OpenGl_Text. All parameters of Graphic3d_Group::Text() are united in this class.
    
    OpenGl_Text constructors with direct position/orientation values are obsolete, constructor with Graphic3d_Text should be used here.
    OpenGl_TextParam is obsolete, using of it is not supported any more.
31Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Aug 23 07:37:13 2019 +0300

    0030537: Visualization - wrapping text in font text formatter
    
    # remarks correction.

32Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Aug 23 08:07:40 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    Graphic3d_Group::Text(...) are obsolete, AddText() should be used instead of these methods.
    Graphic3d_Text is a new class for parameters necessary to fill OpenGl_Text. All parameters of Graphic3d_Group::Text() are moved into this class.
    
    OpenGl_TextParam is removed, these fields were moved into Graphic3d_Text.
    OpenGl_Text constructors/Init with OpenGl_TextParam parameter were removed. Constructor with Graphic3d_Text should be used instead of it.
    Using OpenGl_Text Init() with OpenGl_TextParam should be now replaced on two cases. The first case is setting values into Graphic3d_Text and the second case is calling Reset() after. As example, look at modification in OpenGl_FrameStatsPrs.
33Author
Dear Kirill,

it is corrected, please review.
Jenkins Job: http://jenkins-test-12.nnov.opencascade.com/view/CR30857-master-NDS/

[signature removed]Natalia
34Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Aug 23 13:52:33 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    #remarks correction.

35Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Aug 23 13:55:50 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    #remarks correction.

36Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Aug 23 14:23:23 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    #remarks correction.

37Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Aug 23 14:28:04 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    Graphic3d_Group::Text(...) are obsolete, AddText() should be used instead of these methods.
    Graphic3d_Text is a new class for parameters necessary to fill OpenGl_Text. All parameters of Graphic3d_Group::Text() are moved into this class.
    
    OpenGl_TextParam is removed, these fields were moved into Graphic3d_Text.
    OpenGl_Text constructors/Init with OpenGl_TextParam parameter were removed. Constructor with Graphic3d_Text should be used instead of it.
    Using OpenGl_Text Init() with OpenGl_TextParam should be now replaced on two cases. The first case is setting values into Graphic3d_Text and the second case is calling Reset() after. As example, look at modification in OpenGl_FrameStatsPrs.
38Author
Dear Kirill,

Remarks are corrected.
The same job is started.

[signature removed]Natalia
39Commenter 1
Combination -
OCCT branch : [archived branch]
master SHA - [revision removed]
[revision removed]
Products branch : [archived branch] SHA - [revision removed]
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 16079.660000000105 / 16105.290000000065 [-0.16%]
Products
Total CPU difference: 10477.77000000006 / 10484.700000000048 [-0.07%]
Windows-64-VC14:
OCCT
Total CPU difference: 18284.6875 / 18149.296875 [+0.75%]
Products
Total CPU difference: 12046.4375 / 12045.34375 [+0.01%]

Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
40Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Sat Aug 24 15:20:07 2019 +0300

    0029451: Information Message Alert to debug an algorithm or object functionality
    
    Dump/Init implementation in OCCT object and parsing it in VInspector (partially)

41Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Aug 15 13:17:18 2019 +0300

    0030901: Visualization - OSD_MemInfo moving memory computation out of the constructor
    
    (cherry picked from [revision removed])
    (cherry picked from [revision removed])

42Commenter 2
Branch [archived branch] has been deleted by Commenter 1.

[revision removed]
43Commenter 2
Branch [archived branch] has been deleted by Commenter 1.

[revision removed]
44Commenter 2
Branch [archived branch] has been deleted by Commenter 1.

[revision removed]
45Commenter 2
Branch [archived branch] has been deleted by Commenter 1.

[revision removed]
46Commenter 2
Branch [archived branch] has been deleted by Commenter 1.

[revision removed]
47Commenter 2
Branch [archived branch] has been deleted by Commenter 1.

[revision removed]
48Commenter 2
Branch [archived branch] has been deleted by Commenter 1.

[revision removed]
49Commenter 2
Branch [archived branch] has been deleted by Commenter 1.

[revision removed]
50Commenter 2
Branch [archived branch] has been deleted by Commenter 1.

[revision removed]
51Commenter 2
Branch [archived branch] has been deleted by Commenter 1.

[revision removed]
52Commenter 2
Branch [archived branch] has been deleted by Commenter 1.

[revision removed]
53Commenter 2
Branch [archived branch] has been deleted by Commenter 1.

[revision removed]
54Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Wed Sep 4 00:49:54 2019 +0300

    0030791: Visualization - possibility to display materials by different hatching style for clipping
    
    # move capping style from presentation into drawer
    # crash in capping by setting projection as
    # several hatch templates

Author: Author
Date: Fri Aug 16 13:01:41 2019 +0300

    0030857: Visualization - using one implementation of Text in graphic group - warnings correction - merge correction
    
    #issue remarks correction
55Commenter 2
Branch [archived branch] has been deleted by Participant.

[revision removed]

Related records