Bug 82428 - JPEGImageDecoder: Set frame alpha state before marking a frame complete
Summary: JPEGImageDecoder: Set frame alpha state before marking a frame complete
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: noel gordon
URL:
Keywords:
Depends on: 76498 78239
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-27 23:22 PDT by noel gordon
Modified: 2012-03-28 20:54 PDT (History)
2 users (show)

See Also:


Attachments
Patch (1.95 KB, patch)
2012-03-27 23:42 PDT, noel gordon
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description noel gordon 2012-03-27 23:22:49 PDT
No test was added in bug 76498 (for the reasons given there) but it allowed color correction for WebGL image textures in more cases (JPEG, opaque PNG images).

The fix for bug 78239 regressed that because the alpha state of the frame was set _after_ calling ImageFrame::setStatus() to mark the image frame complete.
Comment 1 noel gordon 2012-03-27 23:42:08 PDT
Created attachment 134223 [details]
Patch
Comment 2 Kenneth Russell 2012-03-28 13:46:04 PDT
Comment on attachment 134223 [details]
Patch

Code looks fine. Is this covered by existing tests, either layout tests or WebGL conformance tests? If not, is it possible to add one? r=me
Comment 3 noel gordon 2012-03-28 17:50:07 PDT
No existing tests that I know of.  My ChangeLog mentioned "No new tests. The application of the color profile is not guaranteed for images used as textures in WebGL.  Have things improved since your wrote that on bug 76498?
Comment 4 Kenneth Russell 2012-03-28 18:21:46 PDT
(In reply to comment #3)
> No existing tests that I know of.  My ChangeLog mentioned "No new tests. The application of the color profile is not guaranteed for images used as textures in WebGL.  Have things improved since your wrote that on bug 76498?

Note that in bug 75999 (as you reminded me over chat), a test case was added which verifies that application of a color profile doesn't completely destroy the contents of a texture. This test couldn't be added to the WebGL conformance suite, because some browsers might not apply the color profile -- but it was fine to add to the WebKit layout tests, and provided some value in the form of a regression test. The tolerance was increased to the point where the test passed even on platforms that didn't apply the color profile. I wonder whether something similar could be done here. But regardless, you have an r+, so you can determine whether you want to just commit the fix for the known bug now or try to add a test case for it.
Comment 5 noel gordon 2012-03-28 18:47:22 PDT
I'd rather not add a test that is flaky/sketchy/unreliable.
Comment 6 WebKit Review Bot 2012-03-28 19:18:34 PDT
Comment on attachment 134223 [details]
Patch

Clearing flags on attachment: 134223

Committed r112493: <http://trac.webkit.org/changeset/112493>
Comment 7 WebKit Review Bot 2012-03-28 19:18:38 PDT
All reviewed patches have been landed.  Closing bug.