Bug 134916 - Clean up image subsampling code, make it less iOS-specific
Summary: Clean up image subsampling code, make it less iOS-specific
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on: 135538
Blocks:
  Show dependency treegraph
 
Reported: 2014-07-14 21:21 PDT by Simon Fraser (smfr)
Modified: 2014-08-08 11:56 PDT (History)
13 users (show)

See Also:


Attachments
Patch (71.89 KB, patch)
2014-07-14 21:51 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (71.89 KB, patch)
2014-07-14 23:09 PDT, Simon Fraser (smfr)
dino: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (562.93 KB, application/zip)
2014-07-15 01:17 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2014-07-14 21:21:42 PDT
Clean up image subsampling code, make it less iOS-specific
Comment 1 Simon Fraser (smfr) 2014-07-14 21:51:08 PDT
Created attachment 234904 [details]
Patch
Comment 2 WebKit Commit Bot 2014-07-14 21:53:14 PDT
Attachment 234904 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/BitmapImage.h:323:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
Total errors found: 1 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Simon Fraser (smfr) 2014-07-14 23:07:03 PDT
I tried to make some ref tests, and failed (bug 134918, and slight ref mismatches).
Comment 4 Simon Fraser (smfr) 2014-07-14 23:09:40 PDT
Created attachment 234906 [details]
Patch
Comment 5 WebKit Commit Bot 2014-07-14 23:11:36 PDT
Attachment 234906 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/BitmapImage.h:323:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
Total errors found: 1 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Build Bot 2014-07-15 01:17:38 PDT
Comment on attachment 234906 [details]
Patch

Attachment 234906 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6534446131970048

New failing tests:
media/video-ended-event-negative-playback.html
Comment 7 Build Bot 2014-07-15 01:17:42 PDT
Created attachment 234909 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 8 Dean Jackson 2014-07-15 13:50:56 PDT
Comment on attachment 234906 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=234906&action=review

> Source/WebCore/platform/graphics/BitmapImage.cpp:390
> +    // We may have cached a frame with a higher subsampling level, in which case we need to
> +    // re-decode with a lower level.
> +    if (index < m_frames.size() && m_frames[index].m_frame && subsamplingLevel < m_frames[index].m_subsamplingLevel) {
>          // If the image is already cached, but at too small a size, re-decode a larger version.

The wording around higher and lower, smaller and larger is often confusing. Maybe the comment should be "higher subsampling level (smaller image)"?

And, the second comment here mostly duplicates the comment you added.

> Source/WebCore/platform/graphics/BitmapImage.cpp:395
> +        WTFLogAlways("BitmapImage %p frameAtIndex recaching, m_decodedSize now %u (size change %d)", this, m_decodedSize, sizeChange);

Always? Why not add a new logging channel?
Comment 9 Simon Fraser (smfr) 2014-08-01 16:25:43 PDT
http://trac.webkit.org/changeset/171957
Comment 10 WebKit Commit Bot 2014-08-01 22:08:03 PDT
Re-opened since this is blocked by bug 135538
Comment 11 Carlos Alberto Lopez Perez 2014-08-01 22:10:28 PDT
(In reply to comment #9)
> http://trac.webkit.org/changeset/171957

This has caused lot of regressions on EFL and GTK.

Reported on bug 135537 and bug 135532
Comment 12 Simon Fraser (smfr) 2014-08-02 09:13:07 PDT
That's why I rolled it out.
Comment 13 Simon Fraser (smfr) 2014-08-08 11:56:53 PDT
Relanded with fixes:
http://trac.webkit.org/changeset/172348