Bug 103749 - ENABLE(IMAGE_DECODER_DOWN_SAMPLING): Should use TURBO_JPEG_RGB_SWIZZLE fast path for non-scaled images
Summary: ENABLE(IMAGE_DECODER_DOWN_SAMPLING): Should use TURBO_JPEG_RGB_SWIZZLE fast p...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 98878
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-30 07:54 PST by Yong Li
Modified: 2012-12-03 07:48 PST (History)
6 users (show)

See Also:


Attachments
the patch (1.73 KB, patch)
2012-11-30 10:45 PST, Yong Li
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yong Li 2012-11-30 07:54:12 PST
There is currently following line #if !ENABLE(IMAGE_DECODER_DOWN_SAMPLING) && defined(TURBO_JPEG_RGB_SWIZZLE) in JPEGImaggeDecoder.cpp. The reason of this !ENABLE(IMAGE_DECODER_DOWN_SAMPLING) check is we cannot do swizzling when the buffer wants a down-sampled image. However this doesn't work. As there is no such check when it sets the colorspace. If you enable both IMAGE_DECODER_DOWN_SAMPLING and TURBO_JPEG_RGB_SWIZZLE, it will crash for sure.

ENABLE(IMAGE_DECODER_DOWN_SAMPLING) is probably not widely used, and could be improved. However, I found it super easy to make TURBO_JPEG_RGB_SWIZZLE work with it, so why not?
Comment 1 Yong Li 2012-11-30 10:30:25 PST
seems the crash has been fixed.
Comment 2 Yong Li 2012-11-30 10:45:47 PST
Created attachment 176995 [details]
the patch
Comment 3 Yong Li 2012-11-30 12:18:57 PST
+Recent JPEGImageDecoder reviewers and Antonio
Comment 4 Yong Li 2012-11-30 12:20:19 PST
This is safe since http://trac.webkit.org/changeset/131075.

(See bug 98878)
Comment 5 Kenneth Russell 2012-11-30 13:37:21 PST
Noel: could you unofficially review this?
Comment 6 noel gordon 2012-11-30 23:15:48 PST
Thank you Yong Li. Looks good to me.
Comment 7 noel gordon 2012-11-30 23:18:36 PST
(In reply to comment #1)
> seems the crash has been fixed.

Yes. Fixed on bug 98878.
Comment 8 Rob Buis 2012-12-03 07:43:34 PST
Comment on attachment 176995 [details]
the patch

LGTM.
Comment 9 WebKit Review Bot 2012-12-03 07:48:54 PST
Comment on attachment 176995 [details]
the patch

Clearing flags on attachment: 176995

Committed r136401: <http://trac.webkit.org/changeset/136401>
Comment 10 WebKit Review Bot 2012-12-03 07:48:57 PST
All reviewed patches have been landed.  Closing bug.