veluca93 / fpnge

Demo of a fast PNG encoder.
Apache License 2.0
88 stars 8 forks source link

Remove mask/adler buffers #8

Closed animetosho closed 2 years ago

animetosho commented 2 years ago

Not sure if these buffers originally had different intended purposes, but removing them reduces cache usage, and simplifies a bunch of code.

This change also removes the POPCNT requirement, and may slightly change output as RLE now works on the last vector of the line.

This also seems to yield a minor performance benefit - on a 12700K:

Old code - image 1
   295.585 MP/s
    10.787 bits/pixel
Old code - image 2
   384.460 MP/s
    16.240 bits/pixel

New code - image 1
   308.533 MP/s
    10.787 bits/pixel
New code - image 2
   385.016 MP/s
    16.240 bits/pixel

CLA response: I release these changes to the public domain subject to the CC0 license (https://creativecommons.org/publicdomain/zero/1.0/).

google-cla[bot] commented 2 years ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

veluca93 commented 2 years ago

LGTM. For a bit of context, originally we could have "holes" in the data (to handle i.e. RGB as if it was RGBA), and that was what the mask and adler buffers were for :)

When I changed to only having necessary data in the buffers, I didn't change that. Thanks for the cleanup!

animetosho commented 2 years ago

I see - I wondered if it was something like that, though yeah, such a conversion would make more sense during the copy phase. Thanks for the explanation!