View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0023493Open CASCADEOCCT:Testspublic2012-10-26 14:182021-11-26 10:08
Reporterkgv Assigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
PlatformAOSL 
Product Version6.5.3 
Target Version6.9.0Fixed in Version6.9.0 
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
TagsNo tags attached.
Test case numberNot needed

Relationships

related to 0023623 closedapn Get wrong values of colors using QAGetPixelColor on Windows VC++ 2012 32bit 

Activities

git

2014-11-28 16:43

administrator   ~0034844

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.

git

2014-12-02 14:27

administrator   ~0034917

Branch CR23493 has been updated forcibly by ski.

SHA-1: 179dacaae2733707246b933a0eb9f8afdd1b9270

ski

2014-12-02 14:33

developer   ~0034918

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.

kgv

2014-12-02 16:32

developer   ~0034934 

+    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".

git

2014-12-02 18:21

administrator   ~0034935

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.

ski

2014-12-02 18:24

developer   ~0034936

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.

kgv

2014-12-02 18:32

developer   ~0034937 

+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" }

git

2014-12-02 19:51

administrator   ~0034945

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.

ski

2014-12-02 19:52

developer   ~0034946

Usage of command vreadpixel for standard colors was improved.

kgv

2014-12-03 09:54

developer   ~0034950

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").

git

2014-12-08 18:27

administrator   ~0035154

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

git

2014-12-08 18:31

administrator   ~0035157

Branch CR23493 has been updated forcibly by ski.

SHA-1: a1ac031fe59465a9eb8190f7b25aa638e8b855ea

ski

2014-12-09 11:01

developer   ~0035190

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.

kgv

2014-12-09 16:10

developer   ~0035215 

+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.

git

2014-12-09 16:41

administrator   ~0035218

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.

ski

2014-12-09 16:47

developer   ~0035219

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.

mkv

2014-12-10 19:14

tester   ~0035284

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

kgv

2014-12-11 08:07

developer   ~0035292

Please revert -setwidth change in test bugs/vis/bug23525, other images look fine.

git

2014-12-11 11:07

administrator   ~0035303

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

ski

2014-12-11 11:16

developer   ~0035305

Changes in test bugs/vis/bug23525 were reverted.

Test and image are OK on win32 vc9 and Debian60-64.

git

2015-01-26 12:35

administrator   ~0036575

Branch CR23493 has been deleted by inv.

SHA-1: f1e9be1c309d140a6844fc743c83e581b040fd19

Related Changesets

occt: master ccadc126

2014-12-11 13:47:12

ski


Committer: bugmaster Details Diff		 0023493: Incorrect QAGetPixelColor usage

Usage of QAGetPixelColor were checked and corrected.

Using simple comparison instead of regexp.

Improved usage of command vreadpixel for standard colors.

Command QAGetPixelColor was dropped from TKQADraw.

Procedures "checkcolor" and auxiliary "checkpoint" were moved to DrawResources/TestCommands.tcl

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.

Removed unnecessary use of command "vaspects -setwidth" in tests.

Revert -setwidth change in test bugs/vis/bug23525		 Affected Issues
0023493
mod - src/DrawResources/TestCommands.tcl Diff File
mod - src/QADraw/QADraw.cxx Diff File
mod - tests/bugs/begin Diff File
mod - tests/bugs/iges/bug22888 Diff File
mod - tests/bugs/modalg_2/bug21909 Diff File
mod - tests/bugs/modalg_2/bug22884 Diff File
mod - tests/bugs/moddata_1/bug22039 Diff File
mod - tests/bugs/vis/buc60574 Diff File
mod - tests/bugs/vis/buc60688 Diff File
mod - tests/bugs/vis/bug11615 Diff File
mod - tests/bugs/vis/bug1174 Diff File
mod - tests/bugs/vis/bug12121 Diff File
mod - tests/bugs/vis/bug173_2 Diff File
mod - tests/bugs/vis/bug173_3 Diff File
mod - tests/bugs/vis/bug223 Diff File
mod - tests/bugs/vis/bug22900 Diff File
mod - tests/bugs/vis/bug22906 Diff File
mod - tests/bugs/vis/bug23012 Diff File
mod - tests/bugs/vis/bug23102 Diff File
mod - tests/bugs/vis/bug23153 Diff File
mod - tests/bugs/vis/bug232 Diff File
mod - tests/bugs/vis/bug23226 Diff File
mod - tests/bugs/vis/bug23363 Diff File
mod - tests/bugs/vis/bug23385 Diff File
mod - tests/bugs/vis/bug23407_2 Diff File
mod - tests/bugs/vis/bug23709_1 Diff File
mod - tests/bugs/vis/bug23709_2 Diff File
mod - tests/bugs/vis/bug23709_3 Diff File
mod - tests/bugs/vis/bug23709_4 Diff File
mod - tests/bugs/vis/bug24391 Diff File
mod - tests/bugs/vis/bug24555 Diff File
mod - tests/bugs/vis/bug24564 Diff File
mod - tests/bugs/vis/bug24714 Diff File
mod - tests/bugs/vis/bug24717 Diff File
mod - tests/bugs/vis/bug24867 Diff File
mod - tests/bugs/vis/bug24901 Diff File
mod - tests/bugs/vis/bug24966 Diff File
mod - tests/bugs/vis/bug349 Diff File
mod - tests/bugs/vis/bug349_1 Diff File
mod - tests/bugs/vis/bug597_1 Diff File
mod - tests/bugs/vis/bug597_2 Diff File
mod - tests/bugs/vis/bug597_3 Diff File
mod - tests/bugs/vis/bug597_4 Diff File
mod - tests/bugs/vis/bug597_5 Diff File
mod - tests/bugs/vis/bug597_6 Diff File
mod - tests/bugs/vis/bug597_7 Diff File
mod - tests/bugs/vis/bug5988 Diff File
mod - tests/bugs/vis/bug60 Diff File
mod - tests/bugs/vis/bug7186 Diff File
mod - tests/bugs/vis/bug74 Diff File
mod - tests/bugs/vis/ger61351_1 Diff File
mod - tests/bugs/vis/ger61351_2 Diff File
mod - tests/bugs/xde/bug22982 Diff File
mod - tests/bugs/xde/bug23193 Diff File
mod - tests/bugs/xde/bug25381 Diff File
mod - tests/demo/begin Diff File

Issue History

Date Modified Username Field Change
2012-10-26 14:18 kgv New Issue
2012-10-26 14:18 kgv Assigned To => mkv
2012-12-06 16:43 abv Relationship added related to 0023623
2013-01-18 17:32 kgv Tag Attached: QA_vis
2013-01-18 17:39 kgv Assigned To mkv => aba
2013-02-26 18:28 san Target Version 6.6.0 => 6.7.0
2013-12-21 10:23 abv Target Version 6.7.0 => 6.7.1
2014-04-04 18:19 abv Target Version 6.7.1 => 6.8.0
2014-10-17 03:13 abv Assigned To aba => apn
2014-10-17 03:13 abv Status new => assigned
2014-10-23 13:10 apn Assigned To apn => ski
2014-10-29 06:11 abv Target Version 6.8.0 => 7.1.0
2014-11-28 16:43 git Note Added: 0034844
2014-12-02 14:27 git Note Added: 0034917
2014-12-02 14:33 ski Note Added: 0034918
2014-12-02 14:33 ski Assigned To ski => kgv
2014-12-02 14:33 ski Status assigned => resolved
2014-12-02 14:33 ski Steps to Reproduce Updated
2014-12-02 16:32 kgv Note Added: 0034934
2014-12-02 16:32 kgv Assigned To kgv => ski
2014-12-02 16:32 kgv Status resolved => assigned
2014-12-02 18:21 git Note Added: 0034935
2014-12-02 18:24 ski Note Added: 0034936
2014-12-02 18:24 ski Assigned To ski => kgv
2014-12-02 18:24 ski Status assigned => resolved
2014-12-02 18:32 kgv Note Added: 0034937
2014-12-02 19:51 git Note Added: 0034945
2014-12-02 19:52 ski Note Added: 0034946
2014-12-03 09:54 kgv Note Added: 0034950
2014-12-03 09:54 kgv Assigned To kgv => ski
2014-12-03 09:54 kgv Status resolved => assigned
2014-12-08 18:27 git Note Added: 0035154
2014-12-08 18:31 git Note Added: 0035157
2014-12-09 11:01 ski Note Added: 0035190
2014-12-09 11:01 ski Assigned To ski => kgv
2014-12-09 11:01 ski Status assigned => resolved
2014-12-09 16:10 kgv Note Added: 0035215
2014-12-09 16:10 kgv Assigned To kgv => bugmaster
2014-12-09 16:10 kgv Status resolved => reviewed
2014-12-09 16:41 git Note Added: 0035218
2014-12-09 16:47 ski Note Added: 0035219
2014-12-10 09:35 mkv Assigned To bugmaster => mkv
2014-12-10 19:14 mkv Note Added: 0035284
2014-12-10 19:14 mkv Assigned To mkv => ski
2014-12-10 19:14 mkv Status reviewed => feedback
2014-12-10 19:16 mkv Test case number => Not needed
2014-12-11 08:07 kgv Note Added: 0035292
2014-12-11 11:07 git Note Added: 0035303
2014-12-11 11:16 ski Note Added: 0035305
2014-12-11 11:18 mkv Assigned To ski => bugmaster
2014-12-11 11:18 mkv Status feedback => tested
2014-12-16 16:46 bugmaster Changeset attached => occt master ccadc126
2014-12-16 16:46 bugmaster Status tested => verified
2014-12-16 16:46 bugmaster Resolution open => fixed
2015-01-19 16:23 bugmaster Target Version 7.1.0 => 6.9.0
2015-01-26 12:35 git Note Added: 0036575
2015-05-14 15:29 aiv Status verified => closed
2015-05-14 15:32 aiv Fixed in Version => 6.9.0
2021-11-26 10:08 kgv Tag Detached: QA_vis