MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0029847Community[OCCT] OCCT:Visualizationpublic2018-06-06 21:282018-06-23 13:57
Reporterdipts 
Assigned Tobugmaster 
PrioritynormalSeverityminor 
StatusverifiedResolutionfixed 
PlatformWindowsOSVC++ 2015OS Version64 bit
Product Version[OCCT] 6.5.4 
Target Version[OCCT] 7.4.0*Fixed in Version 
Summary0029847: Visualization, Image_Diff - Tolerance is not effective for 24/32bit image formats
DescriptionFor any 24/32bit based image formats (RGB, BGR, RGB32, BGR32, RGBA, BGRA) the difference is calculated by subtraction of the individual components of the reference image from the components of the new image. Since the type of these components is (unsigned) byte, the substraction leads to an underflow if the value of the new image is smaller than the ref image (e.g. 125 - 126 equals to 255). This leads to higher differences in sum, which gets a problem when the color tolerance is used.
Steps To ReproduceSynthetic test case:
pload VISUALIZATION
vclear
vinit View1
vsetcolorbg 127 127 127
vdump img_127.png
vsetcolorbg 130 130 130
vdump img_130.png
set aNbDiff0  [diffimage img_127.png img_130.png img0.png -toleranceOfColor 0]
set aNbDiff1  [diffimage img_127.png img_130.png img1.png -toleranceOfColor 0.1]
set aNbDiff01 [diffimage img_127.png img_130.png img01.png -toleranceOfColor 0.01]
if { $aNbDiff0  != 167281 } { puts "Error" }
if { $aNbDiff1  != 0      } { puts "Error" }
if { $aNbDiff01 != 167281 } { puts "Error" }


Image test case:
pload VISUALIZATION
set aNbDiff [diffimage c:/work/ReferenceImage.png c:/work/NewImage.png diff.png -toleranceOfColor 0.5]

if { $aNbDiff != 0 } { puts "Error: difference with tolerance is incorrect" } 

Result: 156

C++ test case:
void ImageDiffWithTolerance()
{
    Handle(Image_PixMap) imageNew = new Image_PixMap();
    imageNew->InitTrash(Image_Format_RGB, 1, 1);
    imageNew->SetPixelColor(0, 0, Quantity_Color(0.5, 0.5, 0.5, Quantity_TOC_RGB) );
    Handle(Image_PixMap) imageRef = new Image_PixMap();
    imageRef->InitTrash(Image_Format_RGB, 1, 1);
    imageRef->SetPixelColor(0, 0, Quantity_Color(0.6, 0.6, 0.6, Quantity_TOC_RGB) );

    Handle(Image_Diff) imageDiff = new Image_Diff();
    imageDiff->Init(imageRef, imageNew);
    Standard_Integer result = imageDiff->Compare(); // should be 1
    imageDiff->SetColorTolerance(0.2);
    result = imageDiff->Compare(); // should be 0
}
TagsNo tags attached.
Test case numberbugs/vis/bug29847
Attached Filespng file icon ReferenceImage.png (7,584 bytes) 2018-06-06 22:03
png file icon NewImage.png (7,627 bytes) 2018-06-06 22:03

- Relationships
child of 0023272closedkgv Open CASCADE Image comparison algorithm 

-  Notes
(0076640)
kgv (developer)
2018-06-06 21:31
edited on: 2018-06-06 21:31

Are you going providing a patch or just registered an issue?

(0076641)
git (administrator)
2018-06-06 21:36

Branch CR29847 has been created by dipts.

SHA-1: 934f9535ede01955ae9f73d97ff8d75391a3f892


Detailed log of new commits:

Author: dipts
Date: Wed Jun 6 20:36:04 2018 +0200

    0029847: Visualization, Image_Diff - Tolerance is not effective for 24/32bit image formats
    
    Pixel values are converted from byte to signed integer before calculating the difference.
(0076642)
dipts (developer)
2018-06-06 21:38

Dear kgv, I am providing a patch. Please review, thanks a lot.
(0076643)
kgv (developer)
2018-06-06 21:57

Could you please attach 2 images for reproducing the problem for creating a test case?
From your description, I would expect that the problem happens within Image_ColorSub3, while your patch affects only Image_Diff.
By the way NCollection_Vec3<int> already defines a method SquareModulus().
(0076644)
dipts (developer)
2018-06-06 22:10

I have attached two images with minor differences which failed without the patch. Yes the underflow occurs in Image_ColorSub3, but as long as the returned types is also unsigned I see no way to return the correct value needed for the diff. It seemed safer to fix that issue in the way that Image_Diff determined the difference.
(0076645)
git (administrator)
2018-06-06 22:44

Branch CR29847_1 has been created by kgv.

SHA-1: 4458e54e3758977041aa1eccb2c649a931cc6838


Detailed log of new commits:

Author: dipts
Date: Wed Jun 6 22:42:11 2018 +0300

    0029847: Visualization, Image_Diff - Tolerance is not effective for 24/32bit image formats
    
    Image_Color - removed semibroken summ/difference operators.
    Image_Diff now uses signed integer for computing differnce between ubyte3 components.
(0076646)
git (administrator)
2018-06-06 23:26

Branch CR29847_1 has been updated forcibly by kgv.

SHA-1: fefa2a6be669b068fe21bb70417d70f9a7914230
(0076647)
kgv (developer)
2018-06-06 23:44

Corrected patch is ready for review.
(0076648)
git (administrator)
2018-06-07 07:03

Branch CR29847_1 has been updated forcibly by kgv.

SHA-1: 5b5df500b50096c7560d6f41f42905545cc779c4
(0076649)
abv (manager)
2018-06-07 08:24

Some remarks:

1. Image_Diff.cxx, lines 188 and 214:

   const Standard_Real aMaxDiffColor = ...

   There seem to be no reason of declaring that variable as real while it is always assigned an integer value.

2. Please consider either renaming struct Image_ColorXXX24 to somewhat more reasonable (e.g. Image_Diff_Color24) or using some existing struct instead.

3. I doubt if using Euclidean distance is appropriate for colors (which apparently do not form a metric space in general, plus are discrete in most cases). IMHO using Chebyshev distance would be more appropriate. This would allow more clear definition of the ColorTolerance() parameter, e.g. if I have image in RGB24 format and set tolerance equal to 1./255., this should mean that deviations of each of R, G, and B components of color by +/-1 (out of 255) are tolerable. Actually 1/255 this should be reasonable default for shaded images, e.g. in ray-tracing or path-tracing tests where color is computed by complex calculations and hence can suffer from rounding errors.
(0076653)
git (administrator)
2018-06-07 10:39

Branch CR29847_1 has been updated by kgv.

SHA-1: 68554a24ebde58c3484f63c29b06aba4c59a9195


Detailed log of new commits:

Author: kgv
Date: Thu Jun 7 10:39:30 2018 +0300

    Image_Diff - dropped declaration of Image_ColorXXX24.
    RGB color difference is now computed using Chebyshev distance instead of Euclidean distance
    Image_PixMap - added methods RawValue()/ChangeRawValue() returning a pointer
    to image where specified pixel data is defined.

(0076654)
kgv (developer)
2018-06-07 10:40

Updated patch is ready for review.
(0076666)
abv (manager)
2018-06-08 08:37

Test bugs modalg_6 bug28594 fails due to diffimage returning zero for quite different images, see http://occt-tests/CR29847_1-master-KGV-OCCT/Windows-64-VC10/bugs/modalg_6/bug28594.html [^]
(0076667)
kgv (developer)
2018-06-08 09:00

> Test bugs modalg_6 bug28594 fails due to diffimage
> returning zero for quite different images
Testing on Jenkins has not been restarted after fixing this regression.
(0076671)
kgv (developer)
2018-06-08 10:39

Patch has been tested.
http://jenkins-test-11.nnov.opencascade.com/view/CR29847_1-master-KGV/ [^]
(0076733)
abv (manager)
2018-06-13 11:17

Reviewed, please integrate
(0076738)
bugmaster (administrator)
2018-06-13 12:17

OCCT branch : CR29847_1 SHA - 68554a24ebde58c3484f63c29b06aba4c59a9195
Products branch : master SHA - 82570c1f4b0e27eb09789f573087eef089260f59
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:
Debian70-64:
OCCT
Total CPU difference: 17022.350000000086 / 17011.039999999866 [+0.07%]
Products
Total CPU difference: 7451.7700000000395 / 7441.7800000000125 [+0.13%]
Windows-64-VC10:
OCCT
Total CPU difference: 16794.41245579859 / 16821.806231398525 [-0.16%]
Products
Total CPU difference: 8268.521002999882 / 8237.071201399893 [+0.38%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0076974)
git (administrator)
2018-06-23 13:57

Branch CR29847 has been deleted by kgv.

SHA-1: 934f9535ede01955ae9f73d97ff8d75391a3f892
(0076975)
git (administrator)
2018-06-23 13:57

Branch CR29847_1 has been deleted by kgv.

SHA-1: 68554a24ebde58c3484f63c29b06aba4c59a9195

- Related Changesets
occt: master 56cc44e0
Timestamp: 2018-06-06 19:42:11
Author: dipts
Committer: bugmaster
Details ] Diff ]
0029847: Visualization, Image_Diff - Tolerance is not effective for 24/32bit image formats

Image_Color - removed semibroken summ/difference operators.
Image_Diff now uses signed integer for computing differnce between ubyte3 components;
properly compare squared tolerance.

Image_Diff - dropped declaration of Image_ColorXXX24.
RGB color difference is now computed using Chebyshev distance instead of Euclidean distance
Image_PixMap - added methods RawValue()/ChangeRawValue() returning a pointer
to image where specified pixel data is defined.
mod - src/Image/Image_Color.hxx Diff ] File ]
mod - src/Image/Image_Diff.cxx Diff ] File ]
mod - src/Image/Image_PixMap.hxx Diff ] File ]
add - tests/bugs/vis/bug29847 Diff ] File ]

- Issue History
Date Modified Username Field Change
2018-06-06 21:28 dipts New Issue
2018-06-06 21:28 dipts Assigned To => dipts
2018-06-06 21:31 kgv Note Added: 0076640
2018-06-06 21:31 kgv Note Edited: 0076640 View Revisions
2018-06-06 21:36 git Note Added: 0076641
2018-06-06 21:37 dipts Assigned To dipts => kgv
2018-06-06 21:38 dipts Note Added: 0076642
2018-06-06 21:38 dipts Status new => resolved
2018-06-06 21:57 kgv Note Added: 0076643
2018-06-06 21:57 kgv Assigned To kgv => dipts
2018-06-06 21:57 kgv Status resolved => feedback
2018-06-06 22:03 dipts File Added: ReferenceImage.png
2018-06-06 22:03 dipts File Added: NewImage.png
2018-06-06 22:10 dipts Note Added: 0076644
2018-06-06 22:10 dipts Assigned To dipts => kgv
2018-06-06 22:16 kgv Steps to Reproduce Updated View Revisions
2018-06-06 22:21 kgv Steps to Reproduce Updated View Revisions
2018-06-06 22:44 git Note Added: 0076645
2018-06-06 22:48 kgv Steps to Reproduce Updated View Revisions
2018-06-06 22:49 kgv Steps to Reproduce Updated View Revisions
2018-06-06 22:50 kgv Steps to Reproduce Updated View Revisions
2018-06-06 22:58 kgv Relationship added child of 0023272
2018-06-06 22:58 kgv Product Version 7.3.0 => 6.5.4
2018-06-06 23:26 git Note Added: 0076646
2018-06-06 23:44 kgv Note Added: 0076647
2018-06-06 23:44 kgv Assigned To kgv => abv
2018-06-06 23:44 kgv Status feedback => resolved
2018-06-07 07:03 git Note Added: 0076648
2018-06-07 08:24 abv Note Added: 0076649
2018-06-07 08:25 abv Assigned To abv => kgv
2018-06-07 08:25 abv Status resolved => assigned
2018-06-07 10:39 git Note Added: 0076653
2018-06-07 10:40 kgv Note Added: 0076654
2018-06-07 10:40 kgv Assigned To kgv => abv
2018-06-07 10:40 kgv Status assigned => resolved
2018-06-08 08:37 abv Note Added: 0076666
2018-06-08 09:00 kgv Note Added: 0076667
2018-06-08 10:39 kgv Note Added: 0076671
2018-06-13 11:17 abv Note Added: 0076733
2018-06-13 11:17 abv Assigned To abv => bugmaster
2018-06-13 11:17 abv Status resolved => reviewed
2018-06-13 12:17 bugmaster Note Added: 0076738
2018-06-13 12:17 bugmaster Status reviewed => tested
2018-06-13 12:17 bugmaster Test case number => bugs/vis/bug29847
2018-06-14 18:20 bugmaster Changeset attached => occt master 56cc44e0
2018-06-14 18:20 bugmaster Status tested => verified
2018-06-14 18:20 bugmaster Resolution open => fixed
2018-06-23 13:57 git Note Added: 0076974
2018-06-23 13:57 git Note Added: 0076975


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker