Attachment 158914 Details for Bug 94241 – Patch

[patch] Patch

bug-94241-20120816150347.patch (text/plain), 11.30 KB, created by Hin-Chung Lam on 2012-08-16 15:04:22 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Hin-Chung Lam

Created: 2012-08-16 15:04:22 PDT

Size: 11.30 KB

patch

obsolete

>Subversion Revision: 125357
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 092930390648477958ded2b4ff971f1bdddec7d4..9a39e739c45f5a800211fcafcb54c8a6b592dab2 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,44 @@
>+2012-08-16  Alpha Lam  <hclam@chromium.org>
>+
>+        Report frame bytes by platform ImageDecoder
>+        https://bugs.webkit.org/show_bug.cgi?id=94241
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Decoded frame bytes should be reported by the platform ImageSource and
>+        ImageDecoder. BitmapImage used to guess system memory used by a frame
>+        but this is no longer true if a frame is backed by an accelerated
>+        surface or defer-decoded.
>+
>+        Adds ImageSource::frameBytesAtIndex and ImageDecoder::frameBytesAtIndex
>+        such that platform can report memory usage correctly.
>+
>+        No new tests. Refactoring without change of behavior.
>+        Tested on Chromium port with pixel tests without any crash or failure.
>+
>+        * platform/graphics/BitmapImage.cpp:
>+        (WebCore::BitmapImage::destroyDecodedData):
>+        (WebCore::BitmapImage::destroyDecodedDataIfNecessary):
>+        (WebCore::BitmapImage::destroyMetadataAndNotify):
>+        (WebCore::BitmapImage::cacheFrame):
>+        (WebCore::BitmapImage::dataChanged):
>+        * platform/graphics/BitmapImage.h:
>+        (WebCore::FrameData::FrameData):
>+        (FrameData):
>+        (BitmapImage):
>+        * platform/graphics/ImageSource.cpp:
>+        (WebCore::ImageSource::frameBytesAtIndex):
>+        (WebCore):
>+        * platform/graphics/ImageSource.h:
>+        * platform/graphics/cg/ImageSourceCG.cpp:
>+        (WebCore::ImageSource::frameBytesAtIndex):
>+        (WebCore):
>+        * platform/image-decoders/ImageDecoder.cpp:
>+        (WebCore::ImageDecoder::frameBytesAtIndex):
>+        (WebCore):
>+        * platform/image-decoders/ImageDecoder.h:
>+        (ImageDecoder):
>+
> 2012-08-11  Benjamin Poulain  <bpoulain@apple.com>
> 
>         Do the DecimalNumber to String conversion on 8 bits
>diff --git a/Source/WebCore/platform/graphics/BitmapImage.cpp b/Source/WebCore/platform/graphics/BitmapImage.cpp
>index e286a7c8bf86143bd7ec641ba085e17ec0d66b96..98f4e4b42f41b7041e35ef3f597f5fa72e33acc6 100644
>--- a/Source/WebCore/platform/graphics/BitmapImage.cpp
>+++ b/Source/WebCore/platform/graphics/BitmapImage.cpp
>@@ -38,11 +38,6 @@
> 
> namespace WebCore {
> 
>-static int frameBytes(const IntSize& frameSize)
>-{
>-    return frameSize.width() * frameSize.height() * 4;
>-}
>-
> BitmapImage::BitmapImage(ImageObserver* observer)
>     : Image(observer)
>     , m_currentFrame(0)
>@@ -85,17 +80,18 @@ bool BitmapImage::hasSingleSecurityOrigin() const
> 
> void BitmapImage::destroyDecodedData(bool destroyAll)
> {
>-    int framesCleared = 0;
>-    const size_t clearBeforeFrame = destroyAll ? m_frames.size() : m_currentFrame;
>+    unsigned frameBytesCleared = 0;
>+    const unsigned clearBeforeFrame = destroyAll ? m_frames.size() : m_currentFrame;
>     for (size_t i = 0; i < clearBeforeFrame; ++i) {
>         // The underlying frame isn't actually changing (we're just trying to
>         // save the memory for the framebuffer data), so we don't need to clear
>         // the metadata.
>+        unsigned frameBytes = m_frames[i].m_frameBytes;
>         if (m_frames[i].clear(false))
>-          ++framesCleared;
>+            frameBytesCleared += frameBytes;
>     }
> 
>-    destroyMetadataAndNotify(framesCleared);
>+    destroyMetadataAndNotify(frameBytesCleared);
> 
>     m_source.clear(destroyAll, clearBeforeFrame, data(), m_allDataReceived);
>     return;
>@@ -106,19 +102,23 @@ void BitmapImage::destroyDecodedDataIfNecessary(bool destroyAll)
>     // Animated images >5MB are considered large enough that we'll only hang on
>     // to one frame at a time.
>     static const unsigned cLargeAnimationCutoff = 5242880;
>-    if (m_frames.size() * frameBytes(m_size) > cLargeAnimationCutoff)
>+    unsigned allFrameBytes = 0;
>+    for (size_t i = 0; i < m_frames.size(); ++i)
>+        allFrameBytes += m_frames[i].m_frameBytes;
>+
>+    if (allFrameBytes > cLargeAnimationCutoff)
>         destroyDecodedData(destroyAll);
> }
> 
>-void BitmapImage::destroyMetadataAndNotify(int framesCleared)
>+void BitmapImage::destroyMetadataAndNotify(unsigned frameBytesCleared)
> {
>     m_isSolidColor = false;
>     m_checkedForSolidColor = false;
>     invalidatePlatformData();
> 
>-    int deltaBytes = framesCleared * -frameBytes(m_size);
>+    int deltaBytes = -frameBytesCleared;
>     m_decodedSize += deltaBytes;
>-    if (framesCleared > 0) {
>+    if (frameBytesCleared > 0) {
>         deltaBytes -= m_decodedPropertiesSize;
>         m_decodedPropertiesSize = 0;
>     }
>@@ -144,12 +144,13 @@ void BitmapImage::cacheFrame(size_t index)
>     if (repetitionCount(false) != cAnimationNone)
>         m_frames[index].m_duration = m_source.frameDurationAtIndex(index);
>     m_frames[index].m_hasAlpha = m_source.frameHasAlphaAtIndex(index);
>+    m_frames[index].m_frameBytes = m_source.frameBytesAtIndex(index);
> 
>     const IntSize frameSize(index ? m_source.frameSizeAtIndex(index) : m_size);
>     if (frameSize != m_size)
>         m_hasUniformFrameSize = false;
>     if (m_frames[index].m_frame) {
>-        int deltaBytes = frameBytes(frameSize);
>+        int deltaBytes = m_frames[index].m_frameBytes;
>         m_decodedSize += deltaBytes;
>         // The fully-decoded frame will subsume the partially decoded data used
>         // to determine image properties.
>@@ -236,15 +237,16 @@ bool BitmapImage::dataChanged(bool allDataReceived)
>     // start of the frame data), and any or none of them might be the particular
>     // frame affected by appending new data here. Thus we have to clear all the
>     // incomplete frames to be safe.
>-    int framesCleared = 0;
>+    unsigned frameBytesCleared = 0;
>     for (size_t i = 0; i < m_frames.size(); ++i) {
>         // NOTE: Don't call frameIsCompleteAtIndex() here, that will try to
>         // decode any uncached (i.e. never-decoded or
>         // cleared-on-a-previous-pass) frames!
>+        unsigned frameBytes = m_frames[i].m_frameBytes;
>         if (m_frames[i].m_haveMetadata && !m_frames[i].m_isComplete)
>-            framesCleared += (m_frames[i].clear(true) ? 1 : 0);
>+            frameBytesCleared += (m_frames[i].clear(true) ? frameBytes : 0);
>     }
>-    destroyMetadataAndNotify(framesCleared);
>+    destroyMetadataAndNotify(frameBytesCleared);
>     
>     // Feed all the data we've seen so far to the image decoder.
>     m_allDataReceived = allDataReceived;
>diff --git a/Source/WebCore/platform/graphics/BitmapImage.h b/Source/WebCore/platform/graphics/BitmapImage.h
>index 05212804e2e9aaaea1696cb2a3887c8a2bdabd33..6132c3b87f608e0cc978847e35efe95b935544f8 100644
>--- a/Source/WebCore/platform/graphics/BitmapImage.h
>+++ b/Source/WebCore/platform/graphics/BitmapImage.h
>@@ -75,6 +75,7 @@ public:
>         , m_haveMetadata(false)
>         , m_isComplete(false)
>         , m_hasAlpha(true) 
>+        , m_frameBytes(0)
>     {
>     }
> 
>@@ -93,6 +94,7 @@ public:
>     bool m_haveMetadata : 1;
>     bool m_isComplete : 1;
>     bool m_hasAlpha : 1;
>+    unsigned m_frameBytes;
> };
> 
> // =================================================
>@@ -234,8 +236,8 @@ protected:
> 
>     // Generally called by destroyDecodedData(), destroys whole-image metadata
>     // and notifies observers that the memory footprint has (hopefully)
>-    // decreased by |framesCleared| times the size (in bytes) of a frame.
>-    void destroyMetadataAndNotify(int framesCleared);
>+    // decreased by |frameBytesCleared|.
>+    void destroyMetadataAndNotify(unsigned frameBytesCleared);
> 
>     // Whether or not size is available yet.    
>     bool isSizeAvailable();
>diff --git a/Source/WebCore/platform/graphics/ImageSource.cpp b/Source/WebCore/platform/graphics/ImageSource.cpp
>index c433de4012e988dc6f9091697875530567151b6b..ca0f95d772208b7741f8d2441ed00529268c627b 100644
>--- a/Source/WebCore/platform/graphics/ImageSource.cpp
>+++ b/Source/WebCore/platform/graphics/ImageSource.cpp
>@@ -198,4 +198,11 @@ bool ImageSource::frameIsCompleteAtIndex(size_t index)
>     return buffer && buffer->status() == ImageFrame::FrameComplete;
> }
> 
>+unsigned ImageSource::frameBytesAtIndex(size_t index) const
>+{
>+    if (!m_decoder)
>+        return 0;
>+    return m_decoder->frameBytesAtIndex(index);
>+}
>+
> }
>diff --git a/Source/WebCore/platform/graphics/ImageSource.h b/Source/WebCore/platform/graphics/ImageSource.h
>index 40d757fa09a3cf6ebe6bd05218367c43eeb70296..7c8b0a703e3078a8edb87beb683ab94edec9f9b2 100644
>--- a/Source/WebCore/platform/graphics/ImageSource.h
>+++ b/Source/WebCore/platform/graphics/ImageSource.h
>@@ -146,6 +146,10 @@ public:
>     bool frameIsCompleteAtIndex(size_t); // Whether or not the frame is completely decoded.
>     ImageOrientation orientationAtIndex(size_t) const; // EXIF image orientation
> 
>+    // Return the number of bytes in the decoded frame. If the frame is not yet
>+    // decoded then return 0.
>+    unsigned frameBytesAtIndex(size_t) const;
>+
> #if ENABLE(IMAGE_DECODER_DOWN_SAMPLING)
>     static unsigned maxPixelsPerDecodedImage() { return s_maxPixelsPerDecodedImage; }
>     static void setMaxPixelsPerDecodedImage(unsigned maxPixels) { s_maxPixelsPerDecodedImage = maxPixels; }
>diff --git a/Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp b/Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp
>index 7cf02968edc6d6a80f5f119f7e2c5abd303d4c7e..7b1e39de16e7daf054a1738e7322fb539d8f1d0e 100644
>--- a/Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp
>+++ b/Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp
>@@ -386,6 +386,12 @@ bool ImageSource::frameHasAlphaAtIndex(size_t)
>     return true;
> }
> 
>+unsigned ImageSource::frameBytesAtIndex(size_t index) const
>+{
>+    IntSize frameSize = frameSizeAtIndex(index, RespectImageOrientation);
>+    return frameSize.width() * frameSize.height() * 4;
>+}
>+
> }
> 
> #endif // USE(CG)
>diff --git a/Source/WebCore/platform/image-decoders/ImageDecoder.cpp b/Source/WebCore/platform/image-decoders/ImageDecoder.cpp
>index 995110827e55d69a910967f4ac1367b18a2217c2..74947f2777cc8a41da9b8f8d78ff3964692bb0aa 100644
>--- a/Source/WebCore/platform/image-decoders/ImageDecoder.cpp
>+++ b/Source/WebCore/platform/image-decoders/ImageDecoder.cpp
>@@ -286,6 +286,14 @@ bool ImageDecoder::frameHasAlphaAtIndex(size_t index) const
>     return true;
> }
> 
>+unsigned ImageDecoder::frameBytesAtIndex(size_t index) const
>+{
>+    if (m_frameBufferCache.size() <= index)
>+        return 0;
>+    // FIXME: Use the dimension of the requested frame.
>+    return m_size.width() * m_size.height() * 4;
>+}
>+
> void ImageDecoder::prepareScaleDataIfNecessary()
> {
>     m_scaled = false;
>diff --git a/Source/WebCore/platform/image-decoders/ImageDecoder.h b/Source/WebCore/platform/image-decoders/ImageDecoder.h
>index f2aaecc4707600fcd0f473bf88d267d881617f26..bce05aea97df8fd471db137bec2704f662443b33 100644
>--- a/Source/WebCore/platform/image-decoders/ImageDecoder.h
>+++ b/Source/WebCore/platform/image-decoders/ImageDecoder.h
>@@ -279,6 +279,9 @@ namespace WebCore {
>         // Make the best effort guess to check if the requested frame has alpha channel.
>         virtual bool frameHasAlphaAtIndex(size_t) const;
> 
>+        // Number of bytes in the decoded frame request, return 0 if not yet decoded.
>+        virtual unsigned frameBytesAtIndex(size_t) const;
>+
>         void setIgnoreGammaAndColorProfile(bool flag) { m_ignoreGammaAndColorProfile = flag; }
>         bool ignoresGammaAndColorProfile() const { return m_ignoreGammaAndColorProfile; }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 94241: 158914 | 158917 | 158966 | 159497