515917 - Minor optimization in PNG pixel copying

Status: RESOLVED DUPLICATE of bug 517713

(Core :: Graphics, defect)

Description

[Steve Snyder]

Attachment: Stop redundant checking/clearing of no-alphas flag

For the RGB_A1 and RGB_A8 formats each pixel is checked to see if it has an alpha byte, and a flag (initially set) is cleared for that line when it does.  Once a alpha pixel is found (marking that entire line as having alphas) further compare/jump tests are superfluous and just add unneeded overhead to the tight copy loops.

My modification is that once the flag is cleared, the remaining pixels for a given line are copied without examining them for an alpha byte.

Comment 1

[Peter Weilbacher]

Comment on attachment 400006 [details] [diff] [review]
Stop redundant checking/clearing of no-alphas flag

Steve, you need to ask for review if you want to see your patch in the codebase. (For your other open bugs, too. You can use http://www.mozilla.org/about/owners.html to select a suitable person.)

Joe is the owner of this code, so I select him.

Comment 2

[Joe Drew (not getting mail)]

Steve, has this shown up in any profiles? I'd rather not add extra code complexity to the packer unless we need to.

Comment 3

[Steve Snyder]

Well. the modified code blocks (copy line of pixels) do show up prominently in profiling, but it is really the GFX_PACKED_PIXEL() lines that take the bulk of the time, not the clearing of the no-alphas flag.

It seemed that the over-all code could do without the redundant compare/branch operations.  The application of the patch won't amaze anyone with the performance improvement, but is does take a little of the CPU burden off the copying operation.

Comment 4

[Joe Drew (not getting mail)]

Comment on attachment 400006 [details] [diff] [review]
Stop redundant checking/clearing of no-alphas flag

This code is, sadly, out of date; I made some pretty major changes to it in bug 753. This patch will need to be updated to trunk.

Comment 5

[Glenn Randers-Pehrson]

There is a patch for this same inefficiency in bug #517713.  That one avoids checking the alpha for all of the remaining rows instead of for the remaining pixels in the current row.

Comment 6

[Alfred Kayser]

Any plans to followup on this one?

Comment 7

[Steve Snyder]

(In reply to Alfred Kayser from comment #6)
> Any plans to followup on this one?

I let it drop because it appeared that bug #517713, owned by libpng's maintainer, would soon address the problem.

I'm still interested in not using CPU time, even if trivial, for unneeded work.  Since there's been no activity on this in nearly 2 years, I can rework my patch for contemporary code if desired.

Comment 8

[Alfred Kayser]

Bug 517713 also tackles this bit of optimization.