Bug 185956 – Enable subsampling for progressive JPEG images

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Chris Dumez]

Optimize ImageDecoderCG::frameAllowSubsamplingAtIndex() currently checks if the image is a Progressive JPEG. We do not need CGImageSourceCopyPropertiesAtIndex() for non-JPEG images.

[Chris Dumez]

Created attachment 341219 [details]
Patch

[Geoffrey Garen]

Comment on attachment 341219 [details]
Patch

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

r=me

> Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:388
> +    // Workaround for <rdar://problem/5184655> - Hang rendering very large progressive JPEG. Decoding progressive
> +    // images hangs for a very long time right now. Until this is fixed, don't sub-sample progressive images. This
> +    // will cause them to fail our large image check and they won't be decoded.
> +    // FIXME: Remove once underlying issue is fixed (<rdar://problem/5191418>)

This patch is fine -- but the FIXME here is about a bug that was fixed in 2014. Maybe we should just remove this code entirely?

[Chris Dumez]

(In reply to Geoffrey Garen from comment #2)
> Comment on attachment 341219 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=341219&action=review
> 
> r=me
> 
> > Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:388
> > +    // Workaround for <rdar://problem/5184655> - Hang rendering very large progressive JPEG. Decoding progressive
> > +    // images hangs for a very long time right now. Until this is fixed, don't sub-sample progressive images. This
> > +    // will cause them to fail our large image check and they won't be decoded.
> > +    // FIXME: Remove once underlying issue is fixed (<rdar://problem/5191418>)
> 
> This patch is fine -- but the FIXME here is about a bug that was fixed in
> 2014. Maybe we should just remove this code entirely?

Simon, do you think we can remove this code entirely?

[Chris Dumez]

(In reply to Chris Dumez from comment #3)
> (In reply to Geoffrey Garen from comment #2)
> > Comment on attachment 341219 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=341219&action=review
> > 
> > r=me
> > 
> > > Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:388
> > > +    // Workaround for <rdar://problem/5184655> - Hang rendering very large progressive JPEG. Decoding progressive
> > > +    // images hangs for a very long time right now. Until this is fixed, don't sub-sample progressive images. This
> > > +    // will cause them to fail our large image check and they won't be decoded.
> > > +    // FIXME: Remove once underlying issue is fixed (<rdar://problem/5191418>)
> > 
> > This patch is fine -- but the FIXME here is about a bug that was fixed in
> > 2014. Maybe we should just remove this code entirely?
> 
> Simon, do you think we can remove this code entirely?

I uploaded the test image to https://ibb.co/bG0pwT. I'll drop the code and try to load it.

[Chris Dumez]

Created attachment 341227 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 341227 [details]
Patch

Clearing flags on attachment: 341227

Committed r232177: <https://trac.webkit.org/changeset/232177>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/40541935>

[Ahmad Saleem]

*** Bug 137648 has been marked as a duplicate of this bug. ***