MantisBT - Open CASCADE
View Issue Details
0031516Open CASCADE[OCCT] PRODUCTS:PMI Visualizationpublic2020-04-23 13:492020-09-14 19:20
sshutina 
bugmaster 
normalfeature 
verifiedfixed 
 
[OCCT] 7.5.0 
Not required
0031516: PMI Visualization - Set/Get the number of the digits after the comma
It needs to implement the interface to setting/getting of the number of digits which should be displayed after the comma in PMI presentation.
No tags attached.
related to 0031515closed nds Open CASCADE Incorrect conversion Standard_Real to TCollection_AsciiString and TCollection_ExtendedString 
png test_results_1.PNG (31,805) 2020-06-29 11:08
https://tracker.dev.opencascade.org/
png test_results_2.PNG (32,550) 2020-06-29 11:08
https://tracker.dev.opencascade.org/
png test_results_3.PNG (34,493) 2020-06-29 11:08
https://tracker.dev.opencascade.org/
png test_results_4.PNG (30,252) 2020-07-02 12:45
https://tracker.dev.opencascade.org/
Issue History
2020-04-23 13:49sshutinaNew Issue
2020-04-23 13:49sshutinaAssigned To => nds
2020-04-23 15:27sshutinaDescription Updatedbug_revision_view_page.php?rev_id=22856#r22856
2020-04-23 16:05sshutinaAssigned Tonds => sshutina
2020-04-23 19:17ndsRelationship addedrelated to 0031515
2020-06-10 14:59kgvSummarySet/Get the number of the digits after the comma => PMI Visualization - Set/Get the number of the digits after the comma
2020-06-24 14:41sshutinaFile Added: test_results.docx
2020-06-24 14:45sshutinaFile Added: test_results_1.PNG
2020-06-24 14:45sshutinaFile Deleted: test_results.docx
2020-06-24 14:45sshutinaFile Added: test_results_2.PNG
2020-06-24 14:45sshutinaFile Added: test_results_3.PNG
2020-06-24 14:46sshutinaFile Added: test_results_4.PNG
2020-06-29 11:08sshutinaFile Deleted: test_results_1.PNG
2020-06-29 11:08sshutinaFile Deleted: test_results_2.PNG
2020-06-29 11:08sshutinaFile Deleted: test_results_3.PNG
2020-06-29 11:08sshutinaFile Deleted: test_results_4.PNG
2020-06-29 11:08sshutinaFile Added: test_results_1.PNG
2020-06-29 11:08sshutinaFile Added: test_results_2.PNG
2020-06-29 11:08sshutinaFile Added: test_results_3.PNG
2020-06-29 11:09sshutinaFile Added: test_results_4.PNG
2020-06-29 11:52sshutinaNote Added: 0092838
2020-06-29 11:52sshutinaAssigned Tosshutina => kgv
2020-06-30 08:26sshutinaStatusnew => resolved
2020-06-30 12:17kgvNote Added: 0092851
2020-06-30 12:17kgvAssigned Tokgv => sshutina
2020-06-30 12:17kgvStatusresolved => assigned
2020-06-30 12:17kgvSeverityminor => feature
2020-06-30 18:28sshutinaFile Deleted: test_results_4.PNG
2020-06-30 18:28sshutinaFile Added: test_results_4.PNG
2020-07-02 12:45sshutinaFile Deleted: test_results_4.PNG
2020-07-02 12:45sshutinaFile Added: test_results_4.PNG
2020-07-02 14:45sshutinaNote Added: 0092895
2020-07-02 14:45sshutinaAssigned Tosshutina => kgv
2020-07-02 14:45sshutinaStatusassigned => resolved
2020-07-02 15:02kgvNote Added: 0092897
2020-07-02 15:03kgvAssigned Tokgv => sshutina
2020-07-02 15:03kgvStatusresolved => assigned
2020-07-02 19:08sshutinaNote Added: 0092911
2020-07-02 19:08sshutinaAssigned Tosshutina => kgv
2020-07-02 19:08sshutinaStatusassigned => resolved
2020-07-02 19:40kgvNote Added: 0092917
2020-07-02 19:40kgvAssigned Tokgv => bugmaster
2020-07-02 19:40kgvStatusresolved => reviewed
2020-07-04 12:14bugmasterNote Added: 0092952
2020-07-04 12:14bugmasterStatusreviewed => tested
2020-07-04 12:18bugmasterTest case number => Not required
2020-07-04 12:27bugmasterNote Added: 0092957
2020-07-04 12:27bugmasterStatustested => verified
2020-07-04 12:27bugmasterResolutionopen => fixed
2020-09-11 17:37abvRelationship addedrelated to 0031651
2020-09-14 19:20kgvRelationship replacedparent of 0031651

Notes
(0092838)
sshutina   
2020-06-29 11:52   
Kirill,
could you please review CR31516.

Thank you in advance, Svetlana.
(0092851)
kgv   
2020-06-30 12:17   
+  Standard_EXPORT TCollection_ExtendedString digitsAfterDecimal (const Standard_Real theValue) const;


Function name is confusing - it is a number format first of all, rather then method returning a number of digits.
It might be "formatReal" or "formatNumber".

+  Standard_Integer NbDigitsAfterDecimal()

Why non-const method?

+  theDI.Add ("PMInbdigitssfterdecimal",
+             "PMInbdigitssfterdecimal {-name PMI_OBJECT|-all} -nbdigits value ",
+             __FILE__, SetNbDigitsAfterDecimal, aGroup);

Please use so-named function/command pair.
Command description should be added.

+puts "============"
+puts "nb_digits_after_decimal_-1"

Please add reference to this bug for new test cases.

+  Standard_Integer myNbDigitsAfterDecimal; //!< The number of digits which should be displayed after 
the comma in PMI presentation.
+                                           //!< It is by default 6.

`//!<` should be set on the first line of a comment; other lines in multi-line comment should start with `//!` for proper processing by Doxygen.
"It is 6 by default" would be more correct.

+  BaseParameters()->SetNbDigitsAfterDecimal (theNbDigits);
+  SetToUpdate();

I would expect `SetToUpdate()` being called only on parameter value change.

   ceil (log10 (abs (theValue) + 0.5));

Log10(), Abs(), Ceiling().

+  Sprintf (aDoubleToStr, "%.*g", aNbDigitsBeforeDecimal + aNbDigitsAfterDecimal, theValue);


There are no tests checking behavior of this formatter in case when %g prefers exponential form %e over %f. Please add such cases.

+    std::cout << theArgs[0] << " error: not enough parameters." << std::endl;

+    return 1;

Message::SendFail()

+  for (Standard_Integer anArgIter = 1; anArgIter < theArgNum; ++anArgIter)
+  {
+    anUpdateTool.parseRedrawMode (theArgs[anArgIter])
...
+  Standard_Integer aNbDigits = 0;
+  if (!aMap.Find ("nbdigits", aValues) || !aValues->Value (1).IsIntegerValue())
+  {

Please handle arguments in the same loop with striker invalid syntax checks.

+      aPrs->SetNbDigitsAfterDecimal (aNbDigits);
+      aPrs->SetToUpdate();

`SetToUpdate()` looks duplicating if `SetNbDigitsAfterDecimal()` calls it as well.

+    {
+       std::cout << theArgs[0] <<  ": Object is not a PMI presentation" << 
std::endl;
+       return 1;

Broken indentation.

    if (!anAISMap.IsBound2 (aValues->Value (1)))
...
+    Handle(PMIVis_Presentation) aPrs = Handle(PMIVis_Presentation)::DownCast (anAISMap.Find2 (aValues->Value 
(1)));

Handle(AIS_InteractiveObject) aPrs;
if (!anAISMap.Find2 (aValues->Value (1), aPrs))

> 0031516: Set/Get the number of the digits after the comma

Commit description should clarify changes in more detail.
This usually include most critical new public API method changed/added and default behavior changes.
(0092895)
sshutina   
2020-07-02 14:45   
Kirill, could you please review it again.
Thank you in advance, Svetlana.
(0092897)
kgv   
2020-07-02 15:02   
+  //! @param theValue the value that wiil be convert to string

the value to be formatted

+  //! Gets the string with the required number of digits which should be displayed
+  //! after the comma in PMI presentation.

Formats the number with specified number of digits after comma.

+  //! Gets the number of digits which should be displayed after the comma in PMI presentation.
+  Standard_Integer NbDigitsAfterDecimal() const

Please document default value within getters.

+    TCollection_AsciiString anArg (theArgs[anArgIter]);
+    if (anArg.IsEqual ("-nbdigits"))

Please do case-insensitive comparison.

+    else if (anArg.IsEqual ("-name"))
+    {
+      aShapeName = TCollection_AsciiString (theArgs[++anArgIter]);
+    }

Number of arguments should be checked.

+      Message::SendFail() << "Syntax error at '" << anArg << ".\n";


Unclosed quote symbol.

+    anUpdateTool.parseRedrawMode (theArgs[anArgIter]);

This should be within `if` block, not outside - the method returns if argument has been processed or not.

+  if (theArgNum < 2)
+  {
+    Message::SendFail() << theArgs[0] << " error: not enough parameters.\n";

+    return 1;
+  }

Redundant - should be already covered by further checks.

+             "\n\t\t    -name      the name of the PMI object, for which the number of digits after 
decimal is set"
+             "\n\t\t  or"
+             "\n\t\t    -all       the number of digits after decimal is set for all PMI objects"


This description is confusing - please provide general command description and document its arguments independently. E.g.:
> Sets the number of digits after comma for formatting values in PMI presentation.
> -name specific PMI presentation to update
> -all update all PMI presentations in the viewer
> -...
(0092911)
sshutina   
2020-07-02 19:08   
Kirill, could you please review it again.
Thank you in advance, Svetlana.
(0092917)
kgv   
2020-07-02 19:40   
Please raise the patch in OCC Products CR31516_4.
(0092952)
bugmaster   
2020-07-04 12:14   
Combination -
OCCT branch : IR-2020-07-03
master SHA - 8f5760bc1679b45691fe8ac5fff1a4e978524009
a206de37fbfa0bf71bd534ae47192bbec23b8522
Products branch : IR-2020-07-03 SHA - 715997b86e7db13b7a7ddd1f26bab717621d84ec
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: 17174.900000000107 / 17168.64000000016 [+0.04%]
Products
Total CPU difference: 11226.760000000077 / 11202.250000000082 [+0.22%]
Windows-64-VC14:
OCCT
Total CPU difference: 18704.34375 / 18712.96875 [-0.05%]
Products
Total CPU difference: 13043.15625 / 13084.109375 [-0.31%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0092957)
bugmaster   
2020-07-04 12:27   
Fix has been integrated into master of occt-products repository