MantisBT - Open CASCADE
View Issue Details
0030623Open CASCADE[OCCT] OCCT:DRAWpublic2019-04-01 14:152019-04-14 15:41
tizmaylo 
bugmaster 
normalfeature 
verifiedfixed 
 
[OCCT] 7.4.0* 
Not needed
0030623: Draw Harness - support hex color codes within ViewerTest::ParseColor()
ViewerTest::ParseColor() should be extended to:
- Allow [0, 255] color components definition by checking that all arguments are integers, at least one argument is >=2 and all arguments <=255.
  So that 0 0 0 and 1 0 0 will be parsed as floating [0.0, 1.0] values.
- Support hex color codes like [#]0000FF and [#]0000FFFF (and also short hex codes like #00F and [#]00FF).
Not required
No tags attached.
child of 0030592tested bugmaster Draw Harness, ViewerTest - provide vbackground command unifying vsetbg, vsetbgmode, vsetgradientbg, vsetgrbgmode, vsetcolorbg 
Issue History
2019-04-01 14:15tizmayloNew Issue
2019-04-01 14:15tizmayloAssigned To => tizmaylo
2019-04-01 14:15tizmayloRelationship addedchild of 0030592
2019-04-01 17:26gitNote Added: 0083334
2019-04-01 19:45gitNote Added: 0083339
2019-04-02 10:06tizmayloNote Added: 0083350
2019-04-02 10:18tizmayloAssigned Totizmaylo => osa
2019-04-02 10:18tizmayloStatusnew => resolved
2019-04-02 10:18tizmayloSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=20953#r20953
2019-04-02 10:47tizmayloNote Edited: 0083350bug_revision_view_page.php?bugnote_id=83350#r20955
2019-04-02 10:49tizmayloNote Edited: 0083350bug_revision_view_page.php?bugnote_id=83350#r20956
2019-04-02 10:51tizmayloNote Edited: 0083350bug_revision_view_page.php?bugnote_id=83350#r20957
2019-04-02 10:52tizmayloNote Edited: 0083350bug_revision_view_page.php?bugnote_id=83350#r20958
2019-04-02 11:39kgvNote Added: 0083353
2019-04-02 12:54kgvNote Added: 0083359
2019-04-02 12:55kgvAssigned Toosa => tizmaylo
2019-04-02 12:55kgvStatusresolved => assigned
2019-04-02 15:42gitNote Added: 0083366
2019-04-02 16:56gitNote Added: 0083368
2019-04-02 19:38tizmayloNote Added: 0083372
2019-04-02 19:38tizmayloAssigned Totizmaylo => osa
2019-04-02 19:38tizmayloStatusassigned => resolved
2019-04-03 10:56osaNote Added: 0083378
2019-04-03 12:05gitNote Added: 0083379
2019-04-03 14:20tizmayloNote Added: 0083392
2019-04-03 14:21tizmayloNote Edited: 0083392bug_revision_view_page.php?bugnote_id=83392#r20969
2019-04-04 13:20kgvNote Added: 0083411
2019-04-04 13:21kgvNote Added: 0083412
2019-04-04 13:21kgvAssigned Toosa => tizmaylo
2019-04-04 13:21kgvStatusresolved => assigned
2019-04-04 13:26kgvNote Added: 0083413
2019-04-04 14:20gitNote Added: 0083414
2019-04-04 14:59gitNote Added: 0083415
2019-04-04 15:03gitNote Added: 0083416
2019-04-04 15:20kgvNote Added: 0083417
2019-04-04 18:41gitNote Added: 0083419
2019-04-04 18:41gitNote Added: 0083420
2019-04-05 10:07tizmayloNote Added: 0083425
2019-04-05 10:08tizmayloAssigned Totizmaylo => osa
2019-04-05 10:08tizmayloStatusassigned => resolved
2019-04-05 10:33kgvNote Added: 0083426
2019-04-05 10:33kgvAssigned Toosa => bugmaster
2019-04-05 10:33kgvSeverityminor => feature
2019-04-05 10:33kgvStatusresolved => reviewed
2019-04-05 10:33kgvSummaryDraw Harness, ViewerTest: ViewerTest::ParseColor() function improvement => Draw Harness - ViewerTest::ParseColor() function improvement
2019-04-05 10:33kgvSummaryDraw Harness - ViewerTest::ParseColor() function improvement => Draw Harness - suppotr hex color codes within ViewerTest::ParseColor()
2019-04-05 10:34kgvSummaryDraw Harness - suppotr hex color codes within ViewerTest::ParseColor() => Draw Harness - support hex color codes within ViewerTest::ParseColor()
2019-04-05 10:45gitNote Added: 0083428
2019-04-05 12:04apnTest case number => Not needed
2019-04-05 12:04apnNote Added: 0083430
2019-04-05 12:04apnStatusreviewed => tested
2019-04-08 01:05apnNote Added: 0083468
2019-04-08 01:06apnAssigned Tobugmaster => tizmaylo
2019-04-08 01:06apnStatustested => assigned
2019-04-08 11:33gitNote Added: 0083474
2019-04-08 11:33gitNote Added: 0083475
2019-04-08 13:51gitNote Added: 0083518
2019-04-08 13:51gitNote Added: 0083519
2019-04-10 10:42tizmayloNote Added: 0083594
2019-04-10 10:42tizmayloAssigned Totizmaylo => osa
2019-04-10 10:42tizmayloStatusassigned => resolved
2019-04-10 10:51kgvAssigned Toosa => bugmaster
2019-04-10 10:51kgvStatusresolved => reviewed
2019-04-12 10:09bugmasterNote Added: 0083649
2019-04-12 10:09bugmasterAssigned Tobugmaster => tizmaylo
2019-04-12 10:09bugmasterStatusreviewed => assigned
2019-04-12 10:52gitNote Added: 0083650
2019-04-12 10:52gitNote Added: 0083651
2019-04-12 10:53tizmayloNote Added: 0083652
2019-04-12 10:53tizmayloAssigned Totizmaylo => bugmaster
2019-04-12 10:53tizmayloStatusassigned => resolved
2019-04-12 10:54tizmayloStatusresolved => reviewed
2019-04-14 14:09bugmasterNote Added: 0083703
2019-04-14 14:09bugmasterStatusreviewed => tested
2019-04-14 14:11bugmasterNote Deleted: 0083703
2019-04-14 14:14bugmasterNote Added: 0083704
2019-04-14 15:36bugmasterChangeset attached => occt master f9b30c0d
2019-04-14 15:36bugmasterStatustested => verified
2019-04-14 15:36bugmasterResolutionopen => fixed
2019-04-14 15:41gitNote Added: 0083709
2019-04-14 15:41gitNote Added: 0083710
2019-04-14 15:41gitNote Added: 0083717

Notes
(0083334)
git   
2019-04-01 17:26   
Branch CR30623 has been created by tizmaylo.

SHA-1: b62050043e757b7a07296ec625cd8e5fd47ee9ea


Detailed log of new commits:

Author: tiv
Date: Mon Apr 1 17:17:18 2019 +0300

    0030623: Draw Harness, ViewerTest: ViewerTest::ParseColor() function improvement
    
    ViewerTest::ParseColor() function is improved to be able to parse the following set of input arguments:
      - "Red Green Blue [Alpha]", where Red, Green, Blue, Alpha must be integers within the range [0, 255] or reals within the range [0.0, 1.0]. Note that "0 0 1" triple is parsed as "0.0 0.0 1.0" and will be interpreted as a blue color.
      - "ColorName [Alpha]", where ColorName is one of WHITE, BLACK, RED, GREEN, BLUE, etc. (look at Quantity_NameOfColor enumeration for all possible variants). Alpha may be integer or real, as described at the previous list item.
      - #HHH, [#]HHH[H], [#]HHHHHH[HH], where H is a hexadecimal digit (0 .. 9, a .. f, or A .. F). There are a short hexadecimal RGB, RGBA formats, and a usual RGB[A], respectively.
(0083339)
git   
2019-04-01 19:45   
Branch CR30623 has been updated forcibly by tizmaylo.

SHA-1: a313428688586b2148197d87263766918d12d698
(0083350)
tizmaylo   
2019-04-02 10:06   
(edited on: 2019-04-02 10:52)
Patch is ready for review:
http://jenkins-test-12.nnov.opencascade.com:8080/view/CR30623-master-TIV/view/ALL/ [^]

Some reviewer's remarks (0030592:0083319) from 0030592 are taken into account.

(0083353)
kgv   
2019-04-02 11:39   
+  template <typename TheLessComparableType>
+  bool IsInRange (const TheLessComparableType& theValue,
+                  const TheLessComparableType& theLowBound,
+                  const TheLessComparableType& theHighBound)
+  {
+    return !(theValue < theLowBound) && !(theHighBound < theValue);

Please remove this redundant and confusing function IsInRange() and use direct comparison in code instead.
By convention in conditions, tested value (e.g. variable) is expected at left side, while constrain (constant) is expected at right side.
Please avoid also confusing negation !(<) statements in comparison.

+        theColorComponent = 1.0;

theColorComponent is float.

+  bool ParseNumericalColorComponent (const Standard_CString theColorComponentString,
+                                     Standard_Integer&      theIntegerColorComponent)

By convention, local functions are marked as "static" even within anonymous namespace.
Local functions are also treated as "protected/private" and suggested to start with lower-case.
(0083359)
kgv   
2019-04-02 12:54   
+    for (std::size_t aColorComponentIndex = 3; aColorComponentIndex-- > 0;)

Using ">0" within unsigned values is confusing, as well as changing index within second block of for instead of third.
Consider revising this code.
(0083366)
git   
2019-04-02 15:42   
Branch CR30623 has been updated by tizmaylo.

SHA-1: b598425e5bd37fee93991119c160a20567870f02


Detailed log of new commits:

Author: tiv
Date: Tue Apr 2 15:36:19 2019 +0300

    0030623: Draw Harness, ViewerTest: ViewerTest::ParseColor() function improvement
    
    Reviewer's remarks on CR30592 are taken into account:
      - functions in anonymous namespaces:
(0083368)
git   
2019-04-02 16:56   
Branch CR30623 has been updated forcibly by tizmaylo.

SHA-1: 39367727a57b4d013d94f4b9939689313a834ee8
(0083372)
tizmaylo   
2019-04-02 19:38   
Patch is ready for review:
http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR30623-master-TIV/view/ALL/ [^]
(0083378)
osa   
2019-04-03 10:56   
+bool Draw::ParseInteger (const Standard_CString theExpressionString, Standard_Integer& theParsedIntegerValue)
+{
+ Standard_Real aParsedRealValue = 0.0f;

aParsedRealValue is not float.

+//=======================================================================
+// function : ParseReal
+// purpose :
+//=======================================================================
+bool Draw::ParseInteger (const Standard_CString theExpressionString, Standard_Integer& theParsedIntegerValue)
+{

Correct comment.

+ theColor = aColorRGBA.GetRGB ();
+ if (!Quantity_Color::ColorFromName (theColorNameString, aColor.ChangeRGB ()))

Remove extra space before empty brackets.

Kirill wrote "By convention, local functions are marked as "static" even within anonymous namespace.
Local functions are also treated as "protected/private" and suggested to start with lower-case." Please use this recommendation in Quantity_ColorRGBA.cxx (methods TakeColorComponentFromInteger ...)
(0083379)
git   
2019-04-03 12:05   
Branch CR30623 has been updated by tizmaylo.

SHA-1: 5e9d6eb07479cf17fcec1c44c10d7f0ec8050d70


Detailed log of new commits:

Author: tiv
Date: Wed Apr 3 11:55:34 2019 +0300

    0030623: Draw Harness, ViewerTest: ViewerTest::ParseColor() function improvement
    
    Reviewer's remarks are taken into account:
(0083392)
tizmaylo   
2019-04-03 14:20   
(edited on: 2019-04-03 14:21)
Done (commit SHA 5e9d6eb07479cf17fcec1c44c10d7f0ec8050d70).

Fixes are ready for review: http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR30623-master-TIV/view/ALL/ [^]


Kirill wrote "By convention, local functions are marked as "static" even within anonymous namespace.
Local functions are also treated as "protected/private" and suggested to start with lower-case." Please use this recommendation in Quantity_ColorRGBA.cxx (methods TakeColorComponentFromInteger ...)


These places have been already fixed (look at the previous commit SHA 39367727a57b4d013d94f4b9939689313a834ee8).

(0083411)
kgv   
2019-04-04 13:20   
0030623: Draw Harness, ViewerTest: ViewerTest::ParseColor() function improvement

Reviewer's remarks on CR30592 are taken into account:
  - functions in anonymous namespaces:
    * → static
    * first letter in their names → lowercase
  - function IsInRange is removed
  - floating point literal's type is fixed: 1.0 → 1.0f

By convention, extra commits in the branch:
- Should NOT start with the bug title.
- Intermediate comments should be started with # on each line,
  while text to be kept within final squashed commit - without #.
(0083412)
kgv   
2019-04-04 13:21   
Please prepare squashed branch before sending patch back for review.
(0083413)
kgv   
2019-04-04 13:26   
+  Standard_EXPORT static Standard_Boolean ColorFromName (const Standard_CString theColorNameString, 
Quantity_Color& theColor)
+  {
+    Quantity_NameOfColor aColorName = Quantity_NOC_BLACK;

Standard_EXPORT should not be used with inline methods.
(0083414)
git   
2019-04-04 14:20   
Branch CR30623 has been updated by tizmaylo.

SHA-1: aac0b0af2f939484c0fd43b2ba4e78e4b3408228


Detailed log of new commits:

Author: tiv
Date: Thu Apr 4 14:14:41 2019 +0300

    # Standard_EXPORT specifier is removed for inline static methods.

(0083415)
git   
2019-04-04 14:59   
Branch CR30623 has been updated forcibly by tizmaylo.

SHA-1: d69dfdfdebe1f8c52a0cf3b09faf8926aa8b97db
(0083416)
git   
2019-04-04 15:03   
Branch CR30623_1 has been created by tizmaylo.

SHA-1: 98dcbe9230a45194afe488317c9c22ff447559af


Detailed log of new commits:

Author: tiv
Date: Thu Apr 4 14:57:27 2019 +0300

    0030623: Draw Harness, ViewerTest: ViewerTest::ParseColor() function improvement
    
    ViewerTest::ParseColor() function is improved to be able to parse the following set of input arguments:
      - "Red Green Blue [Alpha]", where Red, Green, Blue, Alpha must be integers within the range [0, 255] or reals within the range [0.0, 1.0]. Note that "0 0 1" triple is parsed as "0.0 0.0 1.0" and will be interpreted as a blue color.
      - "ColorName [Alpha]", where ColorName is one of WHITE, BLACK, RED, GREEN, BLUE, etc. (look at Quantity_NameOfColor enumeration for all possible variants). Alpha may be integer or real, as described at the previous list item.
      - #HHH, [#]HHH[H], [#]HHHHHH[HH], where H is a hexadecimal digit (0 .. 9, a .. f, or A .. F). There are a short hexadecimal RGB, RGBA formats, and a usual RGB[A], respectively.
    
    # Squashed
(0083417)
kgv   
2019-04-04 15:20   
+bool Quantity_ColorRGBA::ColorFromHex (const char* const   theHexColorString,
+                                       Quantity_ColorRGBA& theColor,
+                                       const bool          theAlphaComponentIsOff)
+{
+  std::size_t aHexColorStringLength = std::strlen (theHexColorString);
+  if (aHexColorStringLength == 0)
+  {
+    return false;
+  }
+
+  const bool        hasPrefix       = (theHexColorString[0] == '#');
+  const std::size_t aPrefixLength   = hasPrefix ? 1 : 0;
+  const char* const aHexColorString = theHexColorString + aPrefixLength;
+  aHexColorStringLength -= aPrefixLength;
+  if (!isHexString (aHexColorString, aHexColorStringLength))

...

+  static bool isHexString (const char* const theString, const std::size_t theLength)
+  {
+    Standard_ASSERT_RETURN (theLength != 0, __FUNCTION__ ": 'theLength' must be greater than zero.", 
false);

It looks redundant handling '#' input through assert.
(0083419)
git   
2019-04-04 18:41   
Branch CR30623 has been updated by tizmaylo.

SHA-1: a31efc92f85f13b4ffa2a05cbdfef9f7b281a3a7


Detailed log of new commits:

Author: tiv
Date: Thu Apr 4 18:33:21 2019 +0300

    # color input strings consisting of a single symbol # are parsed correctly now

(0083420)
git   
2019-04-04 18:41   
Branch CR30623_1 has been updated forcibly by tizmaylo.

SHA-1: cdaacb1765688055917a500ad2971733ceba275b
(0083425)
tizmaylo   
2019-04-05 10:07   
Patch is ready for review: http://jenkins-test-12.nnov.opencascade.com:8080/view/CR30623-master-TIV/view/ALL/ [^]

The main branch: CR30623

The branch with squashed commits: CR30623_1
(0083426)
kgv   
2019-04-05 10:33   
Please correct commit title and raise the patch.
(0083428)
git   
2019-04-05 10:45   
Branch CR30623_1 has been updated forcibly by tizmaylo.

SHA-1: 7ba19547304c8b2c68f4731baab4e8ad800545e3
(0083430)
apn   
2019-04-05 12:04   
Combination -
OCCT branch : CR30623
master SHA - a31efc92f85f13b4ffa2a05cbdfef9f7b281a3a7
d67d4b811012eef8913d3c535c29654d0acf3c4c
Products branch : master SHA - 7c6a97d908d022e1797a28a8a71125f04caa8e40
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: 16574.37000000007 / 16552.78999999999 [+0.13%]
Products
Total CPU difference: 10487.240000000054 / 10486.700000000052 [+0.01%]
Windows-64-VC14:
OCCT
Total CPU difference: 17943.84375 / 17950.40625 [-0.04%]
Products
Total CPU difference: 11988.90625 / 11925.8125 [+0.53%]

Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0083468)
apn   
2019-04-08 01:05   
There are following compilation errors on Windows-32-vc9, Windows-64-vc9, Windows-32-vc10, Windows-64-vc10, Windows-32-vc11, Windows-64-vc11:

http://jenkins-test-08.nnov.opencascade.com/view/IR-2019-04-05_IR-2019-04-05/view/OCCT%20compile/job/IR-2019-04-05_IR-2019-04-05-OCCT-Windows-32-VC10-opt-compile/1/parsed_console/ [^]

c:\builds\IR-2019-04-05_IR-2019-04-05\Windows-32-VC10-opt\OCCT\src\Quantity\Quantity_ColorRGBA.cxx(92) : error C2275: 'Quantity_ColorRGBA' : illegal use of this type as an expression
c:\builds\IR-2019-04-05_IR-2019-04-05\Windows-32-VC10-opt\OCCT\src\Quantity\Quantity_ColorRGBA.cxx(92) : error C2143: syntax error : missing ';' before '{'
c:\builds\IR-2019-04-05_IR-2019-04-05\Windows-32-VC10-opt\OCCT\src\Quantity\Quantity_ColorRGBA.cxx(92) : error C2143: syntax error : missing ';' before '}'
(0083474)
git   
2019-04-08 11:33   
Branch CR30623_1 has been updated forcibly by tizmaylo.

SHA-1: 830ac09b2835dd0a68186d83ad47e6713df72567
(0083475)
git   
2019-04-08 11:33   
Branch CR30623_2 has been created by tizmaylo.

SHA-1: 091162f039fa0f00bcef515efce04d33365de0f1


Detailed log of new commits:

Author: tiv
Date: Mon Apr 8 11:26:26 2019 +0300

    0030623: Draw Harness - support hex color codes within ViewerTest::ParseColor()
    
    ViewerTest::ParseColor() function is improved to be able to parse the following set of input arguments:
      - "Red Green Blue [Alpha]", where Red, Green, Blue, Alpha must be integers within the range [0, 255] or reals within the range [0.0, 1.0]. Note that "0 0 1" triple is parsed as "0.0 0.0 1.0" and will be interpreted as a blue color.
      - "ColorName [Alpha]", where ColorName is one of WHITE, BLACK, RED, GREEN, BLUE, etc. (look at Quantity_NameOfColor enumeration for all possible variants). Alpha may be integer or real, as described at the previous list item.
      - #HHH, [#]HHH[H], [#]HHHHHH[HH], where H is a hexadecimal digit (0 .. 9, a .. f, or A .. F). There are a short hexadecimal RGB, RGBA formats, and a usual RGB[A], respectively.
(0083518)
git   
2019-04-08 13:51   
Branch CR30623_1 has been updated forcibly by tizmaylo.

SHA-1: 8b7ed5b36cfefb9648430806eaf98e8469322540
(0083519)
git   
2019-04-08 13:51   
Branch CR30623_2 has been updated forcibly by tizmaylo.

SHA-1: 9f4acaacc933b7e2dcc2dcab3cc8bc1da04ceea6
(0083594)
tizmaylo   
2019-04-10 10:42   
Patch is ready for review: http://jenkins-test-12.nnov.opencascade.com:8080/view/CR30623_2-master-TIV/view/ALL/ [^]

Main branch: CR30623_1

Branch with squashed commits: CR30623_2
(0083649)
bugmaster   
2019-04-12 10:09   
Error of compilation on ViewerTest:

http://jenkins-test-08.nnov.opencascade.com/view/IR-WEEK15_IR-WEEK15/view/OCCT%20compile/ [^]
(0083650)
git   
2019-04-12 10:52   
Branch CR30623_1 has been updated by tizmaylo.

SHA-1: c2025432d2ffa115ea833c0b550e9251bd8e1be0


Detailed log of new commits:

Author: tiv
Date: Fri Apr 12 10:35:40 2019 +0300

    # VC9 compilation error is fixed

(0083651)
git   
2019-04-12 10:52   
Branch CR30623_2 has been updated forcibly by tizmaylo.

SHA-1: 65b851fcd6503462f62c1d9b6b71c19768238512
(0083652)
tizmaylo   
2019-04-12 10:53   
Compilation error is fixed.

Main branch: CR30623_1

Branch with squashed commits: CR30623_2
(0083704)
bugmaster   
2019-04-14 14:14   
Combination -
OCCT branch : IR-2019-04-13
master SHA - f9b30c0db34c00de8b7a3d0724f3b46e23dfd590
d67d4b811012eef8913d3c535c29654d0acf3c4c
Products branch : IR-2019-04-13 SHA - bcb5483cf17c66425886b3ab24cc11049e73b75a
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: 16535.94999999989 / 16561.32 [-0.15%]
Products
Total CPU difference: 10520.190000000048 / 10513.120000000044 [+0.07%]
Windows-64-VC14:
OCCT
Total CPU difference: 17951.609375 / 17981.5625 [-0.17%]
Products
Total CPU difference: 12011.875 / 11993.9375 [+0.15%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0083709)
git   
2019-04-14 15:41   
Branch CR30623_2 has been deleted by inv.

SHA-1: 65b851fcd6503462f62c1d9b6b71c19768238512
(0083710)
git   
2019-04-14 15:41   
Branch CR30623_1 has been deleted by inv.

SHA-1: c2025432d2ffa115ea833c0b550e9251bd8e1be0
(0083717)
git   
2019-04-14 15:41   
Branch CR30623 has been deleted by inv.

SHA-1: a31efc92f85f13b4ffa2a05cbdfef9f7b281a3a7