MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0031516Open CASCADE[OCCT] PRODUCTS:PMI Visualizationpublic2020-04-23 13:492020-07-04 12:55
Reportersshutina 
Assigned Tobugmaster 
PrioritynormalSeverityfeature 
StatusverifiedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version[OCCT] 7.5.0*Fixed in Version 
Summary0031516: PMI Visualization - Set/Get the number of the digits after the comma
DescriptionIt needs to implement the interface to setting/getting of the number of digits which should be displayed after the comma in PMI presentation.
TagsNo tags attached.
Test case numberNot required
Attached Filespng file icon test_results_1.PNG (31,805 bytes) 2020-06-29 11:08
png file icon test_results_2.PNG (32,550 bytes) 2020-06-29 11:08
png file icon test_results_3.PNG (34,493 bytes) 2020-06-29 11:08
png file icon test_results_4.PNG (30,252 bytes) 2020-07-02 12:45

- Relationships
related to 0031515newnds Incorrect conversion Standard_Real to TCollection_AsciiString and TCollection_ExtendedString 

-  Notes
(0092838)
sshutina (developer)
2020-06-29 11:52

Kirill,
could you please review CR31516.

Thank you in advance, Svetlana.
(0092851)
kgv (developer)
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 (developer)
2020-07-02 14:45

Kirill, could you please review it again.
Thank you in advance, Svetlana.
(0092897)
kgv (developer)
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 (developer)
2020-07-02 19:08

Kirill, could you please review it again.
Thank you in advance, Svetlana.
(0092917)
kgv (developer)
2020-07-02 19:40

Please raise the patch in OCC Products CR31516_4.
(0092952)
bugmaster (administrator)
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 (administrator)
2020-07-04 12:27

Fix has been integrated into master of occt-products repository

- Issue History
Date Modified Username Field Change
2020-04-23 13:49 sshutina New Issue
2020-04-23 13:49 sshutina Assigned To => nds
2020-04-23 15:27 sshutina Description Updated View Revisions
2020-04-23 16:05 sshutina Assigned To nds => sshutina
2020-04-23 19:17 nds Relationship added related to 0031515
2020-06-10 14:59 kgv Summary Set/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:41 sshutina File Added: test_results.docx
2020-06-24 14:45 sshutina File Added: test_results_1.PNG
2020-06-24 14:45 sshutina File Deleted: test_results.docx
2020-06-24 14:45 sshutina File Added: test_results_2.PNG
2020-06-24 14:45 sshutina File Added: test_results_3.PNG
2020-06-24 14:46 sshutina File Added: test_results_4.PNG
2020-06-29 11:08 sshutina File Deleted: test_results_1.PNG
2020-06-29 11:08 sshutina File Deleted: test_results_2.PNG
2020-06-29 11:08 sshutina File Deleted: test_results_3.PNG
2020-06-29 11:08 sshutina File Deleted: test_results_4.PNG
2020-06-29 11:08 sshutina File Added: test_results_1.PNG
2020-06-29 11:08 sshutina File Added: test_results_2.PNG
2020-06-29 11:08 sshutina File Added: test_results_3.PNG
2020-06-29 11:09 sshutina File Added: test_results_4.PNG
2020-06-29 11:52 sshutina Note Added: 0092838
2020-06-29 11:52 sshutina Assigned To sshutina => kgv
2020-06-30 08:26 sshutina Status new => resolved
2020-06-30 12:17 kgv Note Added: 0092851
2020-06-30 12:17 kgv Assigned To kgv => sshutina
2020-06-30 12:17 kgv Status resolved => assigned
2020-06-30 12:17 kgv Severity minor => feature
2020-06-30 18:28 sshutina File Deleted: test_results_4.PNG
2020-06-30 18:28 sshutina File Added: test_results_4.PNG
2020-07-02 12:45 sshutina File Deleted: test_results_4.PNG
2020-07-02 12:45 sshutina File Added: test_results_4.PNG
2020-07-02 14:45 sshutina Note Added: 0092895
2020-07-02 14:45 sshutina Assigned To sshutina => kgv
2020-07-02 14:45 sshutina Status assigned => resolved
2020-07-02 15:02 kgv Note Added: 0092897
2020-07-02 15:03 kgv Assigned To kgv => sshutina
2020-07-02 15:03 kgv Status resolved => assigned
2020-07-02 19:08 sshutina Note Added: 0092911
2020-07-02 19:08 sshutina Assigned To sshutina => kgv
2020-07-02 19:08 sshutina Status assigned => resolved
2020-07-02 19:40 kgv Note Added: 0092917
2020-07-02 19:40 kgv Assigned To kgv => bugmaster
2020-07-02 19:40 kgv Status resolved => reviewed
2020-07-04 12:14 bugmaster Note Added: 0092952
2020-07-04 12:14 bugmaster Status reviewed => tested
2020-07-04 12:18 bugmaster Test case number => Not required
2020-07-04 12:27 bugmaster Note Added: 0092957
2020-07-04 12:27 bugmaster Status tested => verified
2020-07-04 12:27 bugmaster Resolution open => fixed


Copyright © 2000 - 2020 MantisBT Team
Powered by Mantis Bugtracker