MantisBT - Open CASCADE
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0023493Open CASCADE[OCCT] OCCT:Testspublic2012-10-26 14:182015-05-14 15:32
Reporterkgv 
Assigned Tobugmaster 
PrioritynormalSeverityminor 
StatusclosedResolutionfixed 
PlatformAOSLOS VersionL
Product Version[OCCT] 6.5.3 
Target Version[OCCT] 6.9.0Fixed in Version[OCCT] 6.9.0 
Test case numberNot needed
Summary0023493: Incorrect QAGetPixelColor usage
DescriptionQAGetPixelColor command was designed to obtain pixel color for specified position in 3D viewer and:
- Print components in format "RED : 0.93333333730697632 GREEN : 0 BLUE : 0".
- Compare color with specified by user (as 3 additional arguments). If colors aren't equal than command returns 1 (initiate exception) and prints extra message "Faulty : colors are not equal.".

There are several issues within this command and its usage in tests:

- Output format of this command was changed since 0019862 (originally "Faulty" keyword was printed at the beginning but now - after color components output) however some test cases perform checks like that (chl 902 R3):
> catch {QAGetPixelColor 89 363 0.933333 0 0} result
> set err [lindex $result 0]
> if {$err != "Faulty"} {

- Some tests (chl 919 C9) ignores the fact that QAGetPixelColor returns 1 when comparison is failed. Thus such code looks meaningless because it will not be executed in fault case (no catch):
> set r [QAGetPixelColor ${x1} ${y1} ${BLACK_R} ${BLACK_G} ${BLACK_B}]
> if {[lindex $r 0] == "Faulty"} {
> puts " OCC173 OK: vsettransparency of shape was made"
> } else {
> puts "OCC173 Faulty : vsettransparency of shape was NOT made"
> }

- Linux-specific comparison code:
> if {$station == "lin"} {
> catch {QAGetPixelColor 88 316} result
> set red_lin [lindex $result 2]
> set green_lin [lindex $result 5]
> set blue_lin [lindex $result 8]
> if {${red_lin} == 0
> || [expr abs(${green_lin} - 0.8) < 0.1]
> || [expr abs(${blue_lin} - 0.8) < 0.1]} {
> set err "OK"
> }
> }
This is unknown why these checks are Linux-specific but anyway them looks very strange because only single RGB component of color required to be equal (OR instead of AND).

- It seems that output of QAGetPixelColor wasn't designed to be parsed in such way. This is better to use vreadpixel command instead:
> catch {vreadpixel 200 200 rgb} aColor
> set aRed [lindex $aColor 0]
> set aGreen [lindex $aColor 1]
> set aBlue [lindex $aColor 2]

My suggestion is to analyze all cases where QAGetPixelColor currently used for common mistakes and probably replace comparison functionality with new command like vcheckpixel with more reliable usage syntax.
Steps To ReproduceNot required
Additional information
and documentation updates
TagsQA_vis
Relationships
related to 0023623closed apn Get wrong values of colors using QAGetPixelColor on Windows VC++ 2012 32bit 
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2012-10-26 14:18kgvNew Issue
2012-10-26 14:18kgvAssigned To => mkv
2012-12-06 16:43abvRelationship addedrelated to 0023623
2013-01-18 17:32kgvTag Attached: QA_vis
2013-01-18 17:39kgvAssigned Tomkv => aba
2013-02-26 18:28sanTarget Version6.6.0 => 6.7.0
2013-12-21 10:23abvTarget Version6.7.0 => 6.7.1
2014-04-04 18:19abvTarget Version6.7.1 => 6.8.0
2014-10-17 03:13abvAssigned Toaba => apn
2014-10-17 03:13abvStatusnew => assigned
2014-10-23 13:10apnAssigned Toapn => ski
2014-10-29 06:11abvTarget Version6.8.0 => 7.1.0
2014-11-28 16:43gitNote Added: 0034844
2014-12-02 14:27gitNote Added: 0034917
2014-12-02 14:33skiNote Added: 0034918
2014-12-02 14:33skiAssigned Toski => kgv
2014-12-02 14:33skiStatusassigned => resolved
2014-12-02 14:33skiSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=8705#r8705
2014-12-02 16:32kgvNote Added: 0034934
2014-12-02 16:32kgvAssigned Tokgv => ski
2014-12-02 16:32kgvStatusresolved => assigned
2014-12-02 18:21gitNote Added: 0034935
2014-12-02 18:24skiNote Added: 0034936
2014-12-02 18:24skiAssigned Toski => kgv
2014-12-02 18:24skiStatusassigned => resolved
2014-12-02 18:32kgvNote Added: 0034937
2014-12-02 19:51gitNote Added: 0034945
2014-12-02 19:52skiNote Added: 0034946
2014-12-03 09:54kgvNote Added: 0034950
2014-12-03 09:54kgvAssigned Tokgv => ski
2014-12-03 09:54kgvStatusresolved => assigned
2014-12-08 18:27gitNote Added: 0035154
2014-12-08 18:31gitNote Added: 0035157
2014-12-09 11:01skiNote Added: 0035190
2014-12-09 11:01skiAssigned Toski => kgv
2014-12-09 11:01skiStatusassigned => resolved
2014-12-09 16:10kgvNote Added: 0035215
2014-12-09 16:10kgvAssigned Tokgv => bugmaster
2014-12-09 16:10kgvStatusresolved => reviewed
2014-12-09 16:41gitNote Added: 0035218
2014-12-09 16:47skiNote Added: 0035219
2014-12-10 09:35mkvAssigned Tobugmaster => mkv
2014-12-10 19:14mkvNote Added: 0035284
2014-12-10 19:14mkvAssigned Tomkv => ski
2014-12-10 19:14mkvStatusreviewed => feedback
2014-12-10 19:16mkvTest case number => Not needed
2014-12-11 08:07kgvNote Added: 0035292
2014-12-11 11:07gitNote Added: 0035303
2014-12-11 11:16skiNote Added: 0035305
2014-12-11 11:18mkvAssigned Toski => bugmaster
2014-12-11 11:18mkvStatusfeedback => tested
2014-12-16 16:46bugmasterChangeset attached => occt master ccadc126
2014-12-16 16:46bugmasterStatustested => verified
2014-12-16 16:46bugmasterResolutionopen => fixed
2015-01-19 16:23bugmasterTarget Version7.1.0 => 6.9.0
2015-01-26 12:35gitNote Added: 0036575
2015-05-14 15:29aivStatusverified => closed
2015-05-14 15:32aivFixed in Version => 6.9.0

Notes
(0034844)
git   
2014-11-28 16:43   
Branch CR23493 has been created by ski.

SHA-1: 00e98b70f15f02d49265d3b01923f09ec849f2c9


Detailed log of new commits:

Author: ski
Date: Fri Nov 28 16:43:27 2014 +0300

    0023493: Incorrect QAGetPixelColor usage
    
    Usage of QAGetPixelColor were checked and corrected.
(0034917)
git   
2014-12-02 14:27   
Branch CR23493 has been updated forcibly by ski.

SHA-1: 179dacaae2733707246b933a0eb9f8afdd1b9270
(0034918)
ski   
2014-12-02 14:33   
Usage of command QAGetPixelColor in tests were checked.

Some tests were simplified by using command vreadpixel.

Changed test cases were tested on win32 vc9.

Please, review.
(0034934)
kgv   
2014-12-02 16:32    
+    set Color1 [vreadpixel ${x1} ${i}]
+    regexp {^([-0-9.+eE]+) ([-0-9.+eE]+) ([-0-9.+eE]+)} $Color1 full R1 G1 B1

is it really necessary / preferable to use regexp everywhere instead of simpler comparisons like suggested in the description?
> catch {vreadpixel 200 200 rgb} aColor
> set aRed   [lindex $aColor 0]
> set aGreen [lindex $aColor 1]
> set aBlue  [lindex $aColor 2]



+if { [regexp {^0 0 0} [vreadpixel $x1 $y1 rgb]] } {
+    puts "Faulty ${BugNumber}"

some cases might become simpler if replaced by color name comparison "[vreadpixel $x1 $y1 rgb name]" == "BLACK".
(0034935)
git   
2014-12-02 18:21   
Branch CR23493 has been updated by ski.

SHA-1: d58adea86281ca159caccb4483993398a2e74a87


Detailed log of new commits:

Author: ski
Date: Tue Dec 2 18:21:27 2014 +0300

    Using simple comparison instead of regexp.
(0034936)
ski   
2014-12-02 18:24   
Remarks were applied.

It seems that in test case bugs / vis / bug232 it is easier to use regexp instead of any other compare method.

All changes were tested on win32 vc9.

Please, review.
(0034937)
kgv   
2014-12-02 18:32    
+set BlackColor "0 0 0"
+
+if { "[vreadpixel $x1 $y1 rgb]" == "$BlackColor" } {

this is not what I have meant - there is option "name" for vreadpixel command which allows to print color name instead of RGB values like this:
set aColorName [vreadpixel $x1 $y1 rgb name]
if { "$aColorName" == "BLACK" }
(0034945)
git   
2014-12-02 19:51   
Branch CR23493 has been updated by ski.

SHA-1: 10323a68c0b6903cec3fd4c3fca869c7d91c5cce


Detailed log of new commits:

Author: ski
Date: Tue Dec 2 19:51:26 2014 +0300

    Improved usage of command vreadpixel for standard colors.
(0034946)
ski   
2014-12-02 19:52   
Usage of command vreadpixel for standard colors was improved.
(0034950)
kgv   
2014-12-03 09:54   
QAGetPixelColor is now used only within auxiliary procedures "checkpoint" and "checkcolor". It make sense to replace remaining occurrences by vreadpixel and drop QAGetPixelColor from TKQADraw at all.

The procedure "checkcolor" is defined 4 times in test base (2 times in occt and 2 times in products). I suggest to share implementation (move it to DrawResources/TestCommands.tcl?). Note that QAAISGetPixelColor2d do not exists anymore, thus the corresponding branches in "checkcolor" should be dropped.

By the way, color-picking issues due to different hardware might be mitigated by specifying extra width to presentation. E.g. for picking line in test case "vaspects prs -setwidth 4" (though there is no alternative for markers yet until 0025288 and 0025350). It would be good to analyze test cases using "checkcolor" for picking line color and try this trick.

-    if { "[vreadpixel ${x1} ${i}]" == "$BlackColor" } {
+    if { "[vreadpixel ${x1} ${i} name]" == "BLACK" } {

I suppose this particular test case has not been checked (without "rgb" command vreadpixel would return also alpha value "BLACK 0").
(0035154)
git   
2014-12-08 18:27   
Branch CR23493 has been updated by ski.

SHA-1: 64e37c29392713878d1c7d470c7d3d00920c9282


Detailed log of new commits:

Author: ski
Date: Mon Dec 8 18:26:53 2014 +0300

    Some test cases using "checkcolor" for picking line color were simplified.
    
    Procedures checkcolor and checkpoint were changed to handle situation when pixel is out of view.

Author: ski
Date: Wed Dec 3 17:55:28 2014 +0300

    Command QAGetPixelColor was dropped from TKQADraw.
    
    Procedures "checkcolor" and auxiliary "checkpoint" were moved to DrawResources/TestCommands.tcl
(0035157)
git   
2014-12-08 18:31   
Branch CR23493 has been updated forcibly by ski.

SHA-1: a1ac031fe59465a9eb8190f7b25aa638e8b855ea
(0035190)
ski   
2014-12-09 11:01   
Branch CR23493 was rebased on current master.
Product branch CR23493 was created.

All remarks were applied.

I had used "vaspects -unsetwidth" command at the end of tests to get more clearer test cases vdump.

Changed test cases were tested on win32 vc9 and Debian60-64.

Please, review.
(0035215)
kgv   
2014-12-09 16:10    
+vaspects -unsetwidth

Please consider taking final screenshots as is, with increased width for consistency.

+#checkcolor $x_coord $y_coord 0.93 0 0
+if {"[vreadpixel $x_coord $y_coord rgb name]" != "RED2"} {

Commented line looks redundant.
(0035218)
git   
2014-12-09 16:41   
Branch CR23493 has been updated by ski.

SHA-1: a890e898bacdabba2491fbbfe119edbd0b6fc335


Detailed log of new commits:

Author: ski
Date: Tue Dec 9 16:41:28 2014 +0300

    Removed unnecessary use of command "vaspects -setwidth" in tests.
(0035219)
ski   
2014-12-09 16:47   
Minor remarks were applied.

Following test cases will have differences in images (line width is increased):

bugs / vis / bug74
bugs / vis / bug7186
bugs / vis / bug60
bugs / vis / bug349_1
bugs / vis / bug349
bugs / vis / bug24867
bugs / vis / bug24717
bugs / vis / bug24714
bugs / vis / bug24564
bugs / vis / bug24391
bugs / vis / bug23709_4
bugs / vis / bug23709_3
bugs / vis / bug23709_2
bugs / vis / bug23709_1
bugs / vis / bug23525
bugs / vis / bug23407_2
bugs / vis / bug11615
bugs / vis / buc60688

Note that product branch CR23493 exists.
(0035284)
mkv   
2014-12-10 19:14   
Dear BugMaster,
Branch CR23493 from occt git-repository and CR23493from products git-repository were compiled on Linux, MacOS and Windows platforms and tested on Release mode.
SHA-1: a890e898bacdabba2491fbbfe119edbd0b6fc335
SHA-1: 9c1f3588f4754da14338be12b7ec768018f6d8ab

Number of compiler warnings:

occt component :
Linux: 18 (18 on master)
Windows: 0 (0 on master)

products component :
Linux: 11 (11 on master)
Windows: 1 (1 on master)

Regressions/Differences:
No regressions/differences

Testing cases:
Not needed

Testing on Linux:
occt component :
Total MEMORY difference: 363615872 / 363562240
Total CPU difference: 47870.45000000011 / 47537.4299999999
products component :
Total MEMORY difference: 111705008 / 111872212
Total CPU difference: 16025.420000000002 / 15946.739999999983

Testing on Windows:
occt component :
Total MEMORY difference: 276211196 / 277181316
Total CPU difference: 34273.921875 / 41681.890625
products component :
Total MEMORY difference: 72138620 / 71216368
Total CPU difference: 11328.9375 / 13037.484375

There are following differences in images found by testdiff.
http://occt-tests.nnov.opencascade.com/CR23493-CR23493-occt/Debian60-64/diff-Debian60-64.html [^]
http://occt-tests.nnov.opencascade.com/CR23493-CR23493-occt/Windows-32-VC10/diff-Windows-32-VC10.html [^]
IMAGE bugs vis bug24717: bug24717.png differs
IMAGE bugs vis bug60: bug60_Driver1_Viewer1_View1.png differs
IMAGE bugs vis bug24391: bug24391_Driver1_Viewer1_View1.png differs
IMAGE bugs vis bug24714: bug24714_Driver1_Viewer1_View1.png differs
IMAGE bugs vis bug71: bug7186_Driver1_Viewer1_View1.png differs
IMAGE bugs vis bug23525: bug23525_Driver1_Viewer1_View1.png differs
IMAGE bugs vis bug7186: bug7186_Driver1_Viewer1_View1.png differs
IMAGE bugs vis bug23709_3: bug23709_3_Driver1_Viewer1_View1.png differs
IMAGE bugs vis bug23709_2: bug23709_2_Driver1_Viewer1_View1.png differs
IMAGE bugs vis bug349: bug349_1_Driver1_Viewer1_View1_new.png differs
IMAGE bugs vis bug349: bug349_Driver1_Viewer1_View1_new.png differs
IMAGE bugs vis bug23709_4: bug23709_4_Driver1_Viewer1_View1.png differs
IMAGE bugs vis bug349_1: bug349_1_Driver1_Viewer1_View1_new.png differs
IMAGE bugs vis bug23709_1: bug23709_1_Driver1_Viewer1_View1.png differs
IMAGE bugs vis bug24867: bug24867_selected.png differs
IMAGE bugs vis bug24867: bug24867_highlighted.png differs
IMAGE bugs vis buc60688: buc60688_Driver1_Viewer1_View1.png differs
IMAGE bugs vis bug11615: bug11615_Driver1_Viewer1_View1.png differs
IMAGE bugs vis bug74: bug74_Driver1_Viewer1_View1.png differs
(0035292)
kgv   
2014-12-11 08:07   
Please revert -setwidth change in test bugs/vis/bug23525, other images look fine.
(0035303)
git   
2014-12-11 11:07   
Branch CR23493 has been updated by ski.

SHA-1: f1e9be1c309d140a6844fc743c83e581b040fd19


Detailed log of new commits:

Author: ski
Date: Thu Dec 11 11:06:13 2014 +0300

    Revert -setwidth change in test bugs/vis/bug23525
(0035305)
ski   
2014-12-11 11:16   
Changes in test bugs/vis/bug23525 were reverted.

Test and image are OK on win32 vc9 and Debian60-64.
(0036575)
git   
2015-01-26 12:35   
Branch CR23493 has been deleted by inv.

SHA-1: f1e9be1c309d140a6844fc743c83e581b040fd19