Bug 45322 - GIF image down sampling can miss some lines
Summary: GIF image down sampling can miss some lines
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Normal
Assignee: Yong Li
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-07 14:14 PDT by Yong Li
Modified: 2012-04-27 10:34 PDT (History)
6 users (show)

See Also:


Attachments
the patch (1.72 KB, patch)
2010-09-07 15:02 PDT, Yong Li
no flags Details | Formatted Diff | Diff
patch with layout test (112.86 KB, patch)
2010-09-08 07:48 PDT, 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 2010-09-07 14:14:46 PDT
The bug is obvious. Patch is coming soon
Comment 1 Yong Li 2010-09-07 15:02:49 PDT
Created attachment 66771 [details]
the patch

The bug is that xEnd and yEnd were calculated based on already scaled values (xBegin, yBegin). So they were scaled twice...
Comment 2 Peter Kasting 2010-09-07 15:08:06 PDT
Patch itself seems fine to me (but I'm not a reviewer).

It would be nice to layout-test this with a pixel test.
Comment 3 Yong Li 2010-09-07 15:20:08 PDT
(In reply to comment #2)
> Patch itself seems fine to me (but I'm not a reviewer).
> 
> It would be nice to layout-test this with a pixel test.

I didn't add a layout-test because it can be triggered by any GIF but only with certain configurations (down-sampling must be enabled, and the max pixels threshold must be some numbers)
Comment 4 Peter Kasting 2010-09-07 15:29:42 PDT
That's fine, it just means you'll have different layout test results for platforms with down-sampling versus without.

Even if no down-sampling platform runs layout tests (bad!), please at least add a manual test for this (WebCore/manual-tests/).
Comment 5 Yong Li 2010-09-08 07:48:24 PDT
Created attachment 66898 [details]
patch with layout test
Comment 6 WebKit Commit Bot 2010-09-16 04:16:26 PDT
Comment on attachment 66898 [details]
patch with layout test

Clearing flags on attachment: 66898

Committed r67604: <http://trac.webkit.org/changeset/67604>
Comment 7 WebKit Commit Bot 2010-09-16 04:16:31 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Satish Sampath 2010-09-16 04:56:12 PDT
This layout test fails in chromium, probably because there is no pixel test baseline to check against. I also notice that the HTML refers to the file with ".gif" extension whereas the checked in file has ".GIF" in caps. I'll try to add the baseline and fix the file name.
Comment 9 Yong Li 2010-09-16 07:56:46 PDT
(In reply to comment #8)
> I also notice that the HTML refers to the file with ".gif" extension whereas the checked in file has ".GIF" in caps. I'll try to add the baseline and fix the file name.

Oops. thanks
Comment 10 Dimitri Glazkov (Google) 2012-04-26 20:15:02 PDT
The test is crashing on Chromium bots sporadically. What gives?

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=fast%2Fimages%2Fgif-large-checkerboard.html
Comment 11 Yong Li 2012-04-27 07:04:43 PDT
(In reply to comment #10)
> The test is crashing on Chromium bots sporadically. What gives?
> 
> http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=fast%2Fimages%2Fgif-large-checkerboard.html

I cannot see anything with the URL. Is there a stack trace? Is chromium using image down sampling?
Comment 12 Dimitri Glazkov (Google) 2012-04-27 09:46:42 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > The test is crashing on Chromium bots sporadically. What gives?
> > 
> > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=fast%2Fimages%2Fgif-large-checkerboard.html
> 
> I cannot see anything with the URL. Is there a stack trace? Is chromium using image down sampling?

Click on "show results" link, you'll see some stack traces.
Comment 13 Yong Li 2012-04-27 10:07:16 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > The test is crashing on Chromium bots sporadically. What gives?
> > > 
> > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=fast%2Fimages%2Fgif-large-checkerboard.html
> > 
> > I cannot see anything with the URL. Is there a stack trace? Is chromium using image down sampling?
> 
> Click on "show results" link, you'll see some stack traces.

I cannot even see "show results" link. Can you paste them here? Also the gif image in the test is super big. The bot could run out-of-memory
Comment 14 Dimitri Glazkov (Google) 2012-04-27 10:34:45 PDT
(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > (In reply to comment #10)
> > > > The test is crashing on Chromium bots sporadically. What gives?
> > > > 
> > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=fast%2Fimages%2Fgif-large-checkerboard.html
> > > 
> > > I cannot see anything with the URL. Is there a stack trace? Is chromium using image down sampling?
> > 
> > Click on "show results" link, you'll see some stack traces.
> 
> I cannot even see "show results" link. Can you paste them here? Also the gif image in the test is super big. The bot could run out-of-memory

Please join discussion on bug 85073 to continue investigation of the crash.