Bug 51498 – Avoid decoding images twice in texImage2D

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Zhenyao Mo]

Maybe postpone the decoding of an image until it's accessed?

This needs to be very careful as it affects the HTMLImageElement behavior.

[Kenneth Russell]

Could you expand upon the bug description? In the situation where we are supposed to skip the browser's normal color processing (i.e, premultiplication of alpha), then a WebGL-specific decode step is necessary. What could be avoided is the "normal" decode step for the HTMLImageElement, since these images likely won't be displayed on the page; that step could be made lazy.

[John Bauman]

Created attachment 87666 [details]
Patch

[John Bauman]

Created attachment 87667 [details]
This only helps when the image is opaque or the texture is premultiplied, but that's pretty often.

[Zhenyao Mo]

This looks good.  Are you planning to do the similar thing on the Mac/cg port also?

[John Bauman]

Ideally yes, although CGImage doesn't have that nice isOpaque function, so figuring that out might be a bit more difficult (or not, I'm not sure if it would be safe for BitmapImage::frameHasAlphaAtIndex to be public).

[Kenneth Russell]

Comment on attachment 87667 [details]
This only helps when the image is opaque or the texture is premultiplied, but that's pretty often.

This looks good, but could you please either:
 - File another bug about fixing the CG port, or
 - Look further into how to make this work for CG

Another option would be to try to pass down a flag from a higher level about whether alpha is present. In particular, for JPEGs there is definitely no alpha channel.

Also, do you have any measurements of how much faster this makes texture uploads to WebGL when the new code path is taken?

[John Bauman]

It looks like making BitmapImage::frameHasAlphaAtIndex public will work, so I'll just add CG support to this patch.

Times for doing texImage2D from 1000 images go from 5200 ms (jpeg to premultiplied), 4200 ms (jpeg to non-premultiplied), 9300 ms (png to premultiplied) and 8400 ms (png to nonpremultiplied) down to 1900ms, 2400ms, 4000ms, 8400 ms, respectively. On skia we even get a performance gain for pngs that have alpha channels but no transparent pixels, but unfortunately we can't get that gain on CG.

[Kenneth Russell]

(In reply to comment #7)
> It looks like making BitmapImage::frameHasAlphaAtIndex public will work, so I'll just add CG support to this patch.
> 
> Times for doing texImage2D from 1000 images go from 5200 ms (jpeg to premultiplied), 4200 ms (jpeg to non-premultiplied), 9300 ms (png to premultiplied) and 8400 ms (png to nonpremultiplied) down to 1900ms, 2400ms, 4000ms, 8400 ms, respectively. On skia we even get a performance gain for pngs that have alpha channels but no transparent pixels, but unfortunately we can't get that gain on CG.

That's fantastic. Thanks for tracking down this horrible inefficiency.

[Zhenyao Mo]

Great job.  Once the patch lands, could you please open another bug to track the "png to nonpremultiplied" optimization?  We still decode twice on these situations and we don't have to.

[Kenneth Russell]

(In reply to comment #9)
> Great job.  Once the patch lands, could you please open another bug to track the "png to nonpremultiplied" optimization?  We still decode twice on these situations and we don't have to.

The difficulty there will be getting the PNG decoded in the background with the options desired for WebGL. The background decompression premultiplies the alpha channel into the color channels. I don't think we can change that without imposing a severe performance penalty for all web content.

[Zhenyao Mo]

We might be able to make the decoding on demand.  So if an image is never displayed in the browser and it's only used by WebGL as texture source, then we only need to decode it once.

However, I am not sure how this on-demand decoding will effect the web performance at the moment.  Haven't really talked to many people yet, except for James Robinson a while ago, and he thought it's quite possible.

[John Bauman]

Created attachment 87886 [details]
add CG support

[WebKit Review Bot]

Attachment 87886 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/graphics/cg/GraphicsContext3DCG.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[John Bauman]

Created attachment 87889 [details]
add CG support

[Kenneth Russell]

Comment on attachment 87889 [details]
add CG support

Looks great. Thanks for optimizing the CG path too.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 87889 [details]:

java/lc3/JSObject/ToObject-001.html bug 53091 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 87889 [details]
add CG support

Clearing flags on attachment: 87889

Committed r82728: <http://trac.webkit.org/changeset/82728>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.