976371 - Figure out why converting a gfxImageSurface to a DataSourceSurface using GetSourceSurfaceForSurface changes the pixel data

Status: NEW

(Core :: Graphics, defect)

Description

[Jonathan Watt [:jwatt]]

In bug 975900 I discovered that when you convert a gfxImageSurface to a DataSourceSurface using GetSourceSurfaceForSurface it can change the pixel data so that it does not match the original gfxImageSurface. That's bad and we need to figure out why and fix the issue.

This may block the final conversion of imgIContainer::GetFrame to Moz2D (bug 950372) since image\test\unit\test_imgtools.js and toolkit\components\places\tests\favicons\test_favicons_conversions.js test for an exact pixel match.

Comment 1

[Jonathan Watt [:jwatt]]

This is only an issue on Windows.

Comment 2

[Jonathan Watt [:jwatt]]

In bug 975900 the issue occurred with test 3, "test encoding an unscaled JPEG":

http://mxr.mozilla.org/mozilla-central/source/image/test/unit/test_imgtools.js?rev=649a09e4524&mark=184-184,191-191#179

In other words for this test things go wrong under the GetFirstImageFrame call in imgTools::EncodeImage:

http://mxr.mozilla.org/mozilla-central/source/image/src/imgTools.cpp?rev=649a09e4524&mark=164-164#158

Comment 3

[Jonathan Watt [:jwatt]]

Attachment: debugging code added to imgTools.cpp

This debugging code adds some code to check the result of the current method of getting the pixel data with the (broken) GetSourceSurfaceForSurface method we'd prefer to use. With it you can run:

  ./mach xpcshell-test image/test/unit/test_imgtools.js

and get a debugger attached just before the bad GetFirstImageFrame call (start stepping the second time it's hit).

Comment 4

[Jonathan Watt [:jwatt]]

For the broken test there are 4096 pixels; 3573 pixels are the same and 523 pixels are different.

Comment 5

[Jonathan Watt [:jwatt]]

Oh, and enabling the |#if 0| code gets things working, so this is only an issue if passing nullptr to GetSourceSurfaceForSurface so that it uses a screen reference (platform native) draw target.