Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(152)

Issue 1647703006: Track positions of dimensions in image files (Closed)

Created:
4 years, 10 months ago by bengr
Modified:
4 years, 8 months ago
CC:
chromium-reviews, blink-reviews, kinuko+watch, asvitkine+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Track positions of dimensions in image files Records how much of an image file must be retrieved in bytes to decode the image dimensions with the current implementation and with an optimized implementation. BUG=582486

Patch Set 1 #

Patch Set 2 : Updated GIF dimensions UMA logic #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+103 lines, -5 lines) Patch
M third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.h View 2 chunks +5 lines, -1 line 0 comments Download
M third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp View 4 chunks +19 lines, -2 lines 1 comment Download
M third_party/WebKit/Source/platform/image-decoders/gif/GIFImageReader.cpp View 1 2 chunks +5 lines, -1 line 0 comments Download
M third_party/WebKit/Source/platform/image-decoders/jpeg/JPEGImageDecoder.h View 2 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/image-decoders/jpeg/JPEGImageDecoder.cpp View 5 chunks +44 lines, -1 line 0 comments Download
M tools/metrics/histograms/histograms.xml View 2 chunks +26 lines, -0 lines 1 comment Download

Messages

Total messages: 9 (3 generated)
bengr
PTAL
4 years, 10 months ago (2016-01-30 00:39:36 UTC) #2
bengr
PTAL.
4 years, 10 months ago (2016-01-30 00:42:19 UTC) #4
Alexei Svitkine (slow)
https://codereview.chromium.org/1647703006/diff/20001/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp File third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp (right): https://codereview.chromium.org/1647703006/diff/20001/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp#newcode340 third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp:340: "Blink.Images.SizeOffsetOptimized_GIF", m_sizeOffsetOptimized, 1, 1000000, 50); If you prefer, you ...
4 years, 10 months ago (2016-02-01 20:51:11 UTC) #6
Noel Gordon
Note sure what's happening with this CL, or why this is image decoder change is ...
4 years, 9 months ago (2016-03-03 04:11:33 UTC) #7
bengr
On 2016/03/03 04:11:33, noel gordon wrote: > Note sure what's happening with this CL, or ...
4 years, 9 months ago (2016-03-07 23:59:14 UTC) #8
Noel Gordon
4 years, 9 months ago (2016-03-08 12:52:09 UTC) #9
On 2016/03/07 23:59:14, bengr wrote:
> On 2016/03/03 04:11:33, noel gordon wrote:
> > Note sure what's happening with this CL, or why this is image decoder change
> is
> > needed.
> 
> Thanks, Noel. I might close this out, or reduce the scope. Please hold off on
> reviewing.

OK, thanks for the FYI.

> We are considering issuing range requests when layout needs dimensions and the
> image itself isn't needed. The CL would determine how many bytes into the
files
> we'd have to read in practice to retrieve the dimensions.

Blink can't layout an image until we have read the image header.  The
ImageDecoder{.h,.cpp} have the concept of isSizeAvailable(), which transitions
from false -> true once sufficient bytes have been received from the network to
read the image header without error.  Once the image size is available, that
means that the header reading is complete and that we know the image width and
height.  Only then do we attempt to layout / paint the image. 

Image headers are variable length, of course.  They are usually small-ish (under
1K) but can be much larger due to things like having an embedded color profile
for example, which are contained in the image header bytes and can be quite
large when decompressed (up to 4Meg), so YMMV for what you are trying to do.

Powered by Google App Engine
This is Rietveld 408576698