View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0030623 | Open CASCADE | OCCT:DRAW | public | 2019-04-01 14:15 | 2022-04-30 22:24 |
Reporter | Assigned To | bugmaster | |||
Priority | normal | Severity | feature | ||
Status | closed | Resolution | fixed | ||
Target Version | 7.4.0 | Fixed in Version | 7.4.0 | ||
Summary | 0030623: Draw Harness - support hex color codes within ViewerTest::ParseColor() | ||||
Description | 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). | ||||
Steps To Reproduce | Not required | ||||
Tags | No tags attached. | ||||
Test case number | Not needed | ||||
parent of | 0030991 | closed | bugmaster | Draw Harness - ViewerTest::ParseColors() defines out-of-range alpha component |
parent of | 0032955 | closed | Draw Harness, ViewerTest - extend vcolorconvert command to print color in hex format | |
related to | 0031226 | closed | bugmaster | Foundation Classes - TCollection_AsciiString::IsRealValue() returns true if a string contains a non-real value |
child of | 0030592 | closed | bugmaster | Draw Harness, ViewerTest - provide vbackground command unifying vsetbg, vsetbgmode, vsetgradientbg, vsetgrbgmode, vsetcolorbg |
|
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. |
|
Branch CR30623 has been updated forcibly by tizmaylo. SHA-1: a313428688586b2148197d87263766918d12d698 |
|
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. |
|
+ 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. |
|
+ 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. |
|
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: |
|
Branch CR30623 has been updated forcibly by tizmaylo. SHA-1: 39367727a57b4d013d94f4b9939689313a834ee8 |
|
Patch is ready for review: http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR30623-master-TIV/view/ALL/ |
|
+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 ...) |
|
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: |
|
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). |
|
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 #. |
|
Please prepare squashed branch before sending patch back for review. |
|
+ 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. |
|
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. |
|
Branch CR30623 has been updated forcibly by tizmaylo. SHA-1: d69dfdfdebe1f8c52a0cf3b09faf8926aa8b97db |
|
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 |
|
+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. |
|
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 |
|
Branch CR30623_1 has been updated forcibly by tizmaylo. SHA-1: cdaacb1765688055917a500ad2971733ceba275b |
|
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 |
|
Please correct commit title and raise the patch. |
|
Branch CR30623_1 has been updated forcibly by tizmaylo. SHA-1: 7ba19547304c8b2c68f4731baab4e8ad800545e3 |
|
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 |
|
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 '}' |
|
Branch CR30623_1 has been updated forcibly by tizmaylo. SHA-1: 830ac09b2835dd0a68186d83ad47e6713df72567 |
|
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. |
|
Branch CR30623_1 has been updated forcibly by tizmaylo. SHA-1: 8b7ed5b36cfefb9648430806eaf98e8469322540 |
|
Branch CR30623_2 has been updated forcibly by tizmaylo. SHA-1: 9f4acaacc933b7e2dcc2dcab3cc8bc1da04ceea6 |
|
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 |
|
Error of compilation on ViewerTest: http://jenkins-test-08.nnov.opencascade.com/view/IR-WEEK15_IR-WEEK15/view/OCCT%20compile/ |
|
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 |
|
Branch CR30623_2 has been updated forcibly by tizmaylo. SHA-1: 65b851fcd6503462f62c1d9b6b71c19768238512 |
|
Compilation error is fixed. Main branch: CR30623_1 Branch with squashed commits: CR30623_2 |
|
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 |
|
Branch CR30623_2 has been deleted by inv. SHA-1: 65b851fcd6503462f62c1d9b6b71c19768238512 |
|
Branch CR30623_1 has been deleted by inv. SHA-1: c2025432d2ffa115ea833c0b550e9251bd8e1be0 |
|
Branch CR30623 has been deleted by inv. SHA-1: a31efc92f85f13b4ffa2a05cbdfef9f7b281a3a7 |
occt: master f9b30c0d 2019-04-12 07:45:25 tiv Committer: bugmaster Details Diff |
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. |
Affected Issues 0030623 |
|
mod - src/Draw/Draw.hxx | Diff File | ||
mod - src/Draw/Draw_VariableCommands.cxx | Diff File | ||
mod - src/NCollection/NCollection_Mat4.hxx | Diff File | ||
mod - src/NCollection/NCollection_Vec2.hxx | Diff File | ||
mod - src/NCollection/NCollection_Vec3.hxx | Diff File | ||
mod - src/NCollection/NCollection_Vec4.hxx | Diff File | ||
mod - src/Quantity/FILES | Diff File | ||
mod - src/Quantity/Quantity_Color.cxx | Diff File | ||
mod - src/Quantity/Quantity_Color.hxx | Diff File | ||
add - src/Quantity/Quantity_ColorRGBA.cxx | Diff File | ||
mod - src/Quantity/Quantity_ColorRGBA.hxx | Diff File | ||
mod - src/ViewerTest/ViewerTest.cxx | Diff File | ||
mod - src/ViewerTest/ViewerTest.hxx | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2019-04-01 14:15 |
|
New Issue | |
2019-04-01 14:15 |
|
Assigned To | => tizmaylo |
2019-04-01 14:15 |
|
Relationship added | child of 0030592 |
2019-04-01 17:26 | git | Note Added: 0083334 | |
2019-04-01 19:45 | git | Note Added: 0083339 | |
2019-04-02 10:06 |
|
Note Added: 0083350 | |
2019-04-02 10:18 |
|
Assigned To | tizmaylo => osa |
2019-04-02 10:18 |
|
Status | new => resolved |
2019-04-02 10:18 |
|
Steps to Reproduce Updated | |
2019-04-02 10:47 |
|
Note Edited: 0083350 | |
2019-04-02 10:49 |
|
Note Edited: 0083350 | |
2019-04-02 10:51 |
|
Note Edited: 0083350 | |
2019-04-02 10:52 |
|
Note Edited: 0083350 | |
2019-04-02 11:39 | kgv | Note Added: 0083353 | |
2019-04-02 12:54 | kgv | Note Added: 0083359 | |
2019-04-02 12:55 | kgv | Assigned To | osa => tizmaylo |
2019-04-02 12:55 | kgv | Status | resolved => assigned |
2019-04-02 15:42 | git | Note Added: 0083366 | |
2019-04-02 16:56 | git | Note Added: 0083368 | |
2019-04-02 19:38 |
|
Note Added: 0083372 | |
2019-04-02 19:38 |
|
Assigned To | tizmaylo => osa |
2019-04-02 19:38 |
|
Status | assigned => resolved |
2019-04-03 10:56 |
|
Note Added: 0083378 | |
2019-04-03 12:05 | git | Note Added: 0083379 | |
2019-04-03 14:20 |
|
Note Added: 0083392 | |
2019-04-03 14:21 |
|
Note Edited: 0083392 | |
2019-04-04 13:20 | kgv | Note Added: 0083411 | |
2019-04-04 13:21 | kgv | Note Added: 0083412 | |
2019-04-04 13:21 | kgv | Assigned To | osa => tizmaylo |
2019-04-04 13:21 | kgv | Status | resolved => assigned |
2019-04-04 13:26 | kgv | Note Added: 0083413 | |
2019-04-04 14:20 | git | Note Added: 0083414 | |
2019-04-04 14:59 | git | Note Added: 0083415 | |
2019-04-04 15:03 | git | Note Added: 0083416 | |
2019-04-04 15:20 | kgv | Note Added: 0083417 | |
2019-04-04 18:41 | git | Note Added: 0083419 | |
2019-04-04 18:41 | git | Note Added: 0083420 | |
2019-04-05 10:07 |
|
Note Added: 0083425 | |
2019-04-05 10:08 |
|
Assigned To | tizmaylo => osa |
2019-04-05 10:08 |
|
Status | assigned => resolved |
2019-04-05 10:33 | kgv | Note Added: 0083426 | |
2019-04-05 10:33 | kgv | Assigned To | osa => bugmaster |
2019-04-05 10:33 | kgv | Severity | minor => feature |
2019-04-05 10:33 | kgv | Status | resolved => reviewed |
2019-04-05 10:33 | kgv | Summary | Draw Harness, ViewerTest: ViewerTest::ParseColor() function improvement => Draw Harness - ViewerTest::ParseColor() function improvement |
2019-04-05 10:33 | kgv | Summary | Draw Harness - ViewerTest::ParseColor() function improvement => Draw Harness - suppotr hex color codes within ViewerTest::ParseColor() |
2019-04-05 10:34 | kgv | Summary | Draw Harness - suppotr hex color codes within ViewerTest::ParseColor() => Draw Harness - support hex color codes within ViewerTest::ParseColor() |
2019-04-05 10:45 | git | Note Added: 0083428 | |
2019-04-05 12:04 | apn | Test case number | => Not needed |
2019-04-05 12:04 | apn | Note Added: 0083430 | |
2019-04-05 12:04 | apn | Status | reviewed => tested |
2019-04-08 01:05 | apn | Note Added: 0083468 | |
2019-04-08 01:06 | apn | Assigned To | bugmaster => tizmaylo |
2019-04-08 01:06 | apn | Status | tested => assigned |
2019-04-08 11:33 | git | Note Added: 0083474 | |
2019-04-08 11:33 | git | Note Added: 0083475 | |
2019-04-08 13:51 | git | Note Added: 0083518 | |
2019-04-08 13:51 | git | Note Added: 0083519 | |
2019-04-10 10:42 |
|
Note Added: 0083594 | |
2019-04-10 10:42 |
|
Assigned To | tizmaylo => osa |
2019-04-10 10:42 |
|
Status | assigned => resolved |
2019-04-10 10:51 | kgv | Assigned To | osa => bugmaster |
2019-04-10 10:51 | kgv | Status | resolved => reviewed |
2019-04-12 10:09 | bugmaster | Note Added: 0083649 | |
2019-04-12 10:09 | bugmaster | Assigned To | bugmaster => tizmaylo |
2019-04-12 10:09 | bugmaster | Status | reviewed => assigned |
2019-04-12 10:52 | git | Note Added: 0083650 | |
2019-04-12 10:52 | git | Note Added: 0083651 | |
2019-04-12 10:53 |
|
Note Added: 0083652 | |
2019-04-12 10:53 |
|
Assigned To | tizmaylo => bugmaster |
2019-04-12 10:53 |
|
Status | assigned => resolved |
2019-04-12 10:54 |
|
Status | resolved => reviewed |
2019-04-14 14:09 | bugmaster | Status | reviewed => tested |
2019-04-14 14:14 | bugmaster | Note Added: 0083704 | |
2019-04-14 15:36 | bugmaster | Changeset attached | => occt master f9b30c0d |
2019-04-14 15:36 | bugmaster | Status | tested => verified |
2019-04-14 15:36 | bugmaster | Resolution | open => fixed |
2019-04-14 15:41 | git | Note Added: 0083709 | |
2019-04-14 15:41 | git | Note Added: 0083710 | |
2019-04-14 15:41 | git | Note Added: 0083717 | |
2019-09-21 11:44 | kgv | Relationship added | parent of 0030991 |
2020-12-17 16:04 | kgv | Relationship added | related to 0031226 |
2022-04-30 22:24 | kgv | Relationship added | parent of 0032955 |