benkasminbullock / image-png-libpng

Perl interface to libpng
https://metacpan.org/release/Image-PNG-Libpng
0 stars 2 forks source link

Implement RGB/Alpha to Perl array #29

Closed benkasminbullock closed 4 years ago

benkasminbullock commented 4 years ago

See https://github.com/benkasminbullock/image-png-libpng/issues/27

PhilterPaper commented 4 years ago

If you can provide a fast (C code) method to separate A from RGB and G, that would be super! I await the new and improved Image::PNG::Libpng with bated breath (or, baited breath if I just had a sardine sandwich!).

As far as Alien::PNG goes, I contacted the owner about it and he is working with the Alien owner on swatting some bugs (just dropped a new release last night). If he gets it running well, please consider incorporating it into your module (as a prereq). That way, any Perl installation lacking a libpng will be able to use that library through your wrapper. Otherwise, users of poorly maintained Perl distros (like ActiveState) will be left out in the cold.

benkasminbullock commented 4 years ago

@PhilterPaper if you can give me a Perl test file and one or more example PNG images, and the outcome you want from the function, it would make it very much easier for me to write the function. You could use the existing molasses code to generate the expected outcome then I could write a C function which does the same thing as the molasses.

PhilterPaper commented 4 years ago

OK, let's see if you can get my Achannel.zip file. It contains Achannel.pl to generate .new files for the dumped data and alpha channels. There are the .bin files to compare against (should be identical, at least if you're on Windows -- might have line-ending differences if you're not Windows). The input data are 8 files from the Image::PNG::Libpng standard test set, and 1 large RGBA file. You will need to update the source paths in Achannel.pl. The tests used are RGB and Gray (with Alpha), 8 and 16 bps, interlaced and non-interlaced. There is an additional RGBA 8bps non-interlaced of a very large image to really hammer the code.

Achannel.zip

Let me know if you encounter any difficulties. The timings are given for each input file, with the Alpha channel splitting operation shown by itself. I'm hoping you can come up with C code to greatly increase the speed. If it's a standalone routine in your wrapper to replace the vec() calls, it should be easy to comment out the indicated foreach loops and make a single call to your routine. If you set a flag or call a routine to return separate chunks or something like that, it might get complicated.

Thanks much for looking at this!

benkasminbullock commented 4 years ago

Thank you that is exactly what I wanted from you. I've got your code into a shape I can use to write the tests for the functions. The remaining job is to write the functions themselves.

PhilterPaper commented 4 years ago

The remaining job is to write the functions themselves.

Just a SMOP, right? :-)

Any idea whether you're going to have a more-or-less standalone function (combined GA or RGBA in, separated G or RGB and A out), or a new method for retrieving separate data and alpha strings? Keep in mind that right now you retrieve (combined) in row chunks, which have to be assembled into the full GA or RGBA strings.

Right now there are 4 to 8 uvec() calls for each pixel. While each call is C code and reasonably fast, calling them millions of times for a large image really drags. I'm hoping that your plan is to come up with a single C call that does the whole thing in one operation, and is much faster than multiple uvec() calls.

benkasminbullock commented 4 years ago

On Wed, 22 Apr 2020 at 03:10, Phil Perry notifications@github.com wrote:

The remaining job is to write the functions themselves.

Just a SMOP, right? :-)

Programming against a concrete expected result is a much easier job for me than programming without a clear objective so I think we're halfway there.

Any idea whether you're going to have a more-or-less standalone function (combined GA or RGBA in, separated G or RGB and A out), or a new method for retrieving separate data and alpha strings?

I'll program something which gets the whole RGB/whole alpha into a Perl array, like your code. Then the end user can sort out what to do with that.

Keep in mind that right now you retrieve (combined) in row chunks, which have to be assembled into the full GA or RGBA strings.

The stuff in Image::PNG::Libpng is just a thin wrapper around the libpng functions themselves, so it's not really my design.

Right now there are 4 to 8 uvec() calls for each pixel. While each call is C code and reasonably fast, calling them millions of times for a large image really drags. I'm hoping that your plan is to come up with a single C call that does the whole thing in one operation, and is much faster than multiple uvec() calls.

Yes, that is the basic plan. I'm fitting this in in between working like mad on my user application so forgive me for not finishing it urgently, but I would think the whole thing will be sorted out by the end of April if there are no unforeseen problems.

PhilterPaper commented 4 years ago

Sounds good. A drop-in replacement for the for loop and nested uvec() calls is fine.

By the way, just so you don't overlook it and think it's only 1 or 2 bytes per sample (trivial coding), the $bps can be 1, 2, 4, 8, or 16 bits per sample (R, G, B, g, or A value for the pixel). The bits are of course tightly packed (no padding or dead bits). I remind you of this as I don't know how familiar you are with the innards of PNG. Better safe than sorry!

If you've got something more urgent, it's understandable that you want to concentrate on that. A few weeks one way or the other is no problem. I'm putting out the next release of PDF::Builder within a week, so upgraded PNG support would have to wait until the following release to go out (July or August), allowing plenty of time for thorough testing.

Again, thanks!

benkasminbullock commented 4 years ago

On Wed, 22 Apr 2020 at 06:56, Phil Perry notifications@github.com wrote:

Sounds good. A drop-in replacement for the for loop and nested uvec() calls is fine.

Great. There might be a way to do it with the preprocessing in libpng but I haven't checked it though yet.

By the way, just so you don't overlook it and think it's only 1 or 2 bytes per sample (trivial coding), the $bps can be 1, 2, 4, 8, or 16 bits per sample (R, G, B, g, or A value for the pixel). The bits are of course tightly packed (no padding or dead bits). I remind you of this as I don't know how familiar you are with the innards of PNG. Better safe than sorry!

Thanks for the reminder. As it happens it's quite rare to find anyone using anything other than RGBA and 8 bits in practice. I made this web site as a kind of amusing thing which produces QR codes which use 1 bit PNGs:

https://www.qrpng.org/

Unfortunately it doesn't get a lot of traffic.

If you've got something more urgent, it's understandable that you want to concentrate on that.

Unfortunately I've got something very urgent, basically an unforeseen huge job which I have to get done as fast as I can.

A few weeks one way or the other is no problem. I'm putting out the next release of PDF::Builder within a week, so upgraded PNG support would have to wait until the following release to go out (July or August), allowing plenty of time for thorough testing.

OK, let's try to get something done before the next release of PDF::Builder.

Again, thanks!

I'm glad that someone is using Image::PNG::Libpng after all the work which went into it.

PhilterPaper commented 4 years ago

I'm glad that someone is using Image::PNG::Libpng after all the work which went into it.

Unfortunately, I can't make it a mandatory prerequisite, so I only recommend it (as an optional, manual install) in the installation process. This doesn't get back to you in the reverse dependency lists (who is using your package). I tried the optional packages requirement, but it appeared to try to do the installation anyway, and yet still didn't tell you in the reverse dependency list. Optional installations would be OK, except they produce nasty error messages if they fail, which could really scare some users.

PhilterPaper commented 4 years ago

A PR against PDF::API2 (ssimms/pdfapi2, https://github.com/ssimms/pdfapi2/pull/26) was submitted by Rob Scovell to use XS code to speed up RGBA/GA splitting into separate RGB/G and A arrays (replacing vec() usage). His XS code might be a good starting point for an A-channel separator as I requested earlier. It should be usable with my PDF::Builder, as the Perl code is very similar. I have not tried it yet and have no idea of its quality, but it might be a starting point.

benkasminbullock commented 4 years ago

Thanks.

I'll have a look at it and get back to you.

On Sat, 15 Aug 2020 at 03:28, Phil Perry notifications@github.com wrote:

A PR against PDF::API2 (ssimms/pdfapi2, ssimms/pdfapi2#26 https://github.com/ssimms/pdfapi2/pull/26) was submitted by Rob Scovell to use XS code to speed up RGBA/GA splitting into separate RGB/G and A arrays (replacing vec() usage). His XS code might be a good starting point for an A-channel separator as I requested earlier. It should be usable with my PDF::Builder, as the Perl code is very similar. I have not tried it yet and have no idea of its quality, but it might be a starting point.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/benkasminbullock/image-png-libpng/issues/29#issuecomment-674205843, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAY45IQ72KFDIB6FPEDEITSAV63TANCNFSM4MI7NSWQ .

PhilterPaper commented 4 years ago

I just got some feedback from the author of that routine, and it sounds like it's 8 bps and only for RGBA usage, if I understand his comments. Apparently it's not as general purpose as I had hoped. It also looks like it folds in some PNG processing specific to PDF::API2 (and PDF::Builder) and thus may not be suitable for a general purpose library. Anyway, it may still provide a useful starting point.

benkasminbullock commented 4 years ago

I didn't look at the suggested thing but I've implemented the basic functionality, and it's currently working correctly for the eight-bit cases. The 16 bit cases are not implemented yet. I'll try to get those done within a day or two. A bug is affecting the screenshot example.

On Wed, 19 Aug 2020 at 21:05, Phil Perry notifications@github.com wrote:

I just got some feedback from the author of that routine, and it sounds like it's 8 bps and only for RGBA usage, if I understand his comments. Apparently it's not as general purpose as I had hoped. It also looks like it folds in some PNG processing specific to PDF::API2 (and PDF::Builder) and thus may not be suitable for a general purpose library. Anyway, it may still provide a useful starting point.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/benkasminbullock/image-png-libpng/issues/29#issuecomment-676238293, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAY45JY4SVVXXQUX6EWSF3SBO5Z5ANCNFSM4MI7NSWQ .

benkasminbullock commented 4 years ago

OK it seems to be working now. Can you please do the following:

cpanm BKB/Image-PNG-Libpng-0.46_01.tar.gz

to install the function, and then provisionally your function is called "kungfu". You call it like this:

use Image::PNG::Libpng 'kungfu'; # or 'all' is OK
my $png = read_png ('/file/to/read.png');
my $fu = kungfu ($png);
my $data = $fu->{data};
my $alpha = $fu->{alpha};

The $data and $alpha exactly correspond to your "vec" things, as far as all my tests show.

Please let me know how it goes.

The function name WILL be changed from "kungfu" in the release version.

On Tue, 25 Aug 2020 at 15:17, Ben Bullock benkasminbullock@gmail.com wrote:

I didn't look at the suggested thing but I've implemented the basic functionality, and it's currently working correctly for the eight-bit cases. The 16 bit cases are not implemented yet. I'll try to get those done within a day or two. A bug is affecting the screenshot example.

On Wed, 19 Aug 2020 at 21:05, Phil Perry notifications@github.com wrote:

I just got some feedback from the author of that routine, and it sounds like it's 8 bps and only for RGBA usage, if I understand his comments. Apparently it's not as general purpose as I had hoped. It also looks like it folds in some PNG processing specific to PDF::API2 (and PDF::Builder) and thus may not be suitable for a general purpose library. Anyway, it may still provide a useful starting point.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/benkasminbullock/image-png-libpng/issues/29#issuecomment-676238293, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAY45JY4SVVXXQUX6EWSF3SBO5Z5ANCNFSM4MI7NSWQ .

PhilterPaper commented 4 years ago

Good to hear from you. I'll try to give it a shot tonight. Is this only 8bps you're handling (with 16bps to come), or will you also be handling 1, 2, and 4 bps? I need to know what to exclude from my PNG test suite.

benkasminbullock commented 4 years ago

It currently works on all the files referred to in Achannel-ref.pl.

On Wed, 26 Aug 2020 at 01:57, Phil Perry notifications@github.com wrote:

Good to hear from you. I'll try to give it a shot tonight. Is this only 8bps you're handling (with 16bps to come), or will you also be handling 1, 2, and 4 bps? I need to know what to exclude from my PNG test suite.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/benkasminbullock/image-png-libpng/issues/29#issuecomment-680148525, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAY45KCH6I7XAFZXYWFB6TSCPURJANCNFSM4MI7NSWQ .

PhilterPaper commented 4 years ago

OK, you're right about Alpha channel only being 8 or 16bps. I thought it included 1, 2, and 4, but my comments in the code are 8/16 only. If 1, 2, or 4 bps show up in the future, I may be back for an upgrade (hopefully not... dealing with masking and shifting individual bits will likely be very slow).

So I went and installed 0.46_1 and modified my code per your instructions. I first tried feeding $clearstream to kungfu(), which is the uncompressed and unfiltered string of RGBA (or GA) bytes ready to be split via vec(). That didn't work, so I tried the $png object (data loaded up with $png->read_data('/file/to/read.png'); and it worked. All I did was replace the big pixel loop and vec() calls with your kungfu() calls:

# new libpng code, temporarily 'kungfu'
my $fu = kungfu($png);
$self->{' stream'} = $fu->{'data'};
$dict->{' stream'} = $fu->{'alpha'};

By any chance are you duplicating any of the uncompress/unfilter code currently done in Perl, or are you strictly splitting the channels? If the former, I could get some more speedup by removing the uncompress/unfilter calls from Perl.

The results were great! All the resulting PDF images appear to be exactly the same as before, but the speedup ran anywhere from 2x to 24x. The biggest improvement was in the 8bps RGBA 2400x1800 pixel image, which went from 11.9 seconds end-to-end (from reading the PNG file to writing the PDF) to 0.5. The smaller test images were typically in the 3-5x range, as they had more overhead and relatively less time in the vec() loop. Anyway, replacing 35 million vec() calls with one kungfu() call really made a difference. I'll have to think about using some of Rob's other XS code to see if it will speed up PNG even more.

I have tested a wide range of 8bps and 16bps RGBA and GA, which is more than Rob is handling (8bps RGBA only, as far as I know), so this looks good to me. I am looking forward to the next Image::PNG::Libpng release. Thank you for the great job!

benkasminbullock commented 4 years ago

On Wed, 26 Aug 2020 at 11:18, Phil Perry notifications@github.com wrote:

OK, you're right about Alpha channel only being 8 or 16bps. I thought it included 1, 2, and 4, but my comments in the code are 8/16 only. If 1, 2, or 4 bps show up in the future, I may be back for an upgrade (hopefully not... dealing with masking and shifting individual bits will likely be very slow).

I don't know about that actually, I just got a simple function working with your examples.

So I went and installed 0.46_1 and modified my code per your instructions. I first tried feeding $clearstream to kungfu(), which is the uncompressed and unfiltered string of RGBA (or GA) bytes ready to be split via vec(). That didn't work, so I tried the $png object (data loaded up with $png->read_data('/file/to/read.png'); and it worked. All I did was replace the big pixel loop and vec() calls with your kungfu() calls:

new libpng code, temporarily 'kungfu'

my $fu = kungfu($png); $self->{' stream'} = $fu->{'data'}; $dict->{' stream'} = $fu->{'alpha'};

By any chance are you duplicating any of the uncompress/unfilter code currently done in Perl, or are you strictly splitting the channels? If the former, I could get some more speedup by removing the uncompress/unfilter calls from Perl.

Well, my module is for reading PNG data directly from files so the code for uncompressing and unfiltering is already there in libpng.

The results were great! All the resulting PDF images appear to be exactly the same as before, but the speedup ran anywhere from 2x to 24x. The biggest improvement was in the 8bps RGBA 2400x1800 pixel image, which went from 11.9 seconds end-to-end (from reading the PNG file to writing the PDF) to 0.5. The smaller test images were typically in the 3-5x range, as they had more overhead and relatively less time in the vec() loop. Anyway, replacing 35 million vec() calls with one kungfu() call really made a difference. I'll have to think about using some of Rob's other XS code to see if it will speed up PNG even more.

I'm glad this is working for you. It took about three or four hours of work for me to get this far.

I have tested a wide range of 8bps and 16bps RGBA and GA, which is more than Rob is handling (8bps RGBA only, as far as I know), so this looks good to me. I am looking forward to the next Image::PNG::Libpng release. Thank you for the great job!

OK, I'll probably name the function "SplitAlpha" or something like that and make it a method.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/benkasminbullock/image-png-libpng/issues/29#issuecomment-680419583, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAY45KWAESVSF7N4FKS7HLSCRWG3ANCNFSM4MI7NSWQ .

PhilterPaper commented 4 years ago

Well, my module is for reading PNG data directly from files so the code for uncompressing and unfiltering is already there in libpng.

If that's the case, Rob's unfilter() and paeth_predictor() implemented in C are now unnecessary? My code may also still have some unnecessary code left over from the days when PNG processing was in pure Perl. I've have to look at it when I get a chance.

benkasminbullock commented 4 years ago

On Thu, 27 Aug 2020 at 01:21, Phil Perry notifications@github.com wrote:

Well, my module is for reading PNG data directly from files so the code for uncompressing and unfiltering is already there in libpng.

If that's the case, Rob's unfilter() and paeth_predictor() implemented in C are now unnecessary?

They're not necessary for Image::PNG::Libpng purposes, I'm not sure about your module though.

PhilterPaper commented 4 years ago

It doesn't look like a big deal to skip the following code:

    my $rows = $png->get_rows();
    for (my $row = 0; $row < @{$rows}; $row++) {
    $self->{' stream'} .= $rows->[$row];
    }

for RGBA and GA models (CS), as use of $self->{' stream'} is bypassed by your new kungfu method. Other than that, I had apparently already removed a bunch of unnecessary uncompress and unfilter code several years ago when I began using Image::PNG::Libpng. It occurs to me that I might be able to save even more code if you now provide any direct method of $self->{' stream'} = $???->{'data'}; for the other color models -- does that already exist? If it doesn't, it's no big deal to continue building stream by appending rows, as above (for non-Alpha color models). I just wanted to make sure I hadn't overlooked anything simple.

I'd like to get the next version of PDF::Builder (using the new and improved libpng code) out in 2 to 3 months -- do you have any release plans yet for the 0.47 release within, say, 1 to 2 months? I can wait a bit beyond that, but if the next release is a long way off, I'll have to consider my options. Appreciate it!

benkasminbullock commented 4 years ago

On Fri, 4 Sep 2020 at 12:08, Phil Perry notifications@github.com wrote:

It doesn't look like a big deal to skip the following code:

my $rows = $png->get_rows();
for (my $row = 0; $row < @{$rows}; $row++) {

$self->{' stream'} .= $rows->[$row]; }

for RGBA and GA models (CS), as use of $self->{' stream'} is bypassed by your new kungfu method.

I am thinking of calling it "split_alpha".

Other than that, I had apparently already removed a bunch of unnecessary uncompress and unfilter code several years ago when I began using Image::PNG::Libpng. It occurs to me that I might be able to save even more code if you now provide any direct method of $self->{' stream'} = $???->{'data'}; for the other color models -- does that already exist? If it doesn't, it's no big deal to continue building stream by appending rows, as above (for non-Alpha color models). I just wanted to make sure I hadn't overlooked anything simple.

What data do you want here? Usually the PNG data is in "chunks" which can be individually removed. Do you mean you want to have the red, green, blue, grey pixels separately like the alpha values?

I'd like to get the next version of PDF::Builder (using the new and improved libpng code) out in 2 to 3 months -- do you have any release plans yet for the 0.47 release within, say, 1 to 2 months? I can wait a bit beyond that, but if the next release is a long way off, I'll have to consider my options. Appreciate it!

OK that seems reasonable. The remaining job is to write tests and documentation for the split_alpha.

Ben

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/benkasminbullock/image-png-libpng/issues/29#issuecomment-686873841, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAY45LZMYAK3BKXHQHLQODSEBK2XANCNFSM4MI7NSWQ .

PhilterPaper commented 4 years ago

What data do you want here? Usually the PNG data is in "chunks" which can be individually removed. Do you mean you want to have the red, green, blue, grey pixels separately like the alpha values?

Oh no, nothing like that. What I'm saying is right now, for non-Alpha modes I (still) have to assemble the " stream" data by gluing together rows of data, whereas with split_alpha(), $sa->{'data'} provides it. I was just wondering if I had overlooked anything to get the data for the non-Alpha modes in such an easy manner. It's not a burden to continue doing it this way; it just would be a bit nicer if there was a built-in method.

benkasminbullock commented 4 years ago

On Sat, 5 Sep 2020 at 01:43, Phil Perry notifications@github.com wrote:

What data do you want here? Usually the PNG data is in "chunks" which can be individually removed. Do you mean you want to have the red, green, blue, grey pixels separately like the alpha values?

Oh no, nothing like that. What I'm saying is right now, for non-Alpha modes I (still) have to assemble the " stream" data by gluing together rows of data, whereas with split_alpha(), $sa->{'data'} provides it. I was just wondering if I had overlooked anything to get the data for the non-Alpha modes in such an easy manner. It's not a burden to continue doing it this way; it just would be a bit nicer if there was a built-in method.

I don't think there is, I think the "rows" stuff is what libpng gives you. libpng is a little "unusual" in its design. There is a libpng mailing list you can ask on where they might know more, when I have asked things there they were very helpful.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/benkasminbullock/image-png-libpng/issues/29#issuecomment-687261972, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAY45JTMH72QNYKBHPKDV3SEEKMXANCNFSM4MI7NSWQ .

PhilterPaper commented 4 years ago

OK, if there's not a slightly "cleaner" way to get the data for non-Alpha modes, no big deal. The current code works fine and probably would be sped up only slightly. I just thought that since you had apparently gone ahead and done it for the Alpha modes (built up the full data from rows internally, if that's what you did), that you might have also done it for non-Alpha modes (or at least, most of the work was already done).

Do you have a feel for when you might complete the split_alpha and release on CPAN? Maybe within a couple months, or "no idea" at this point? If it's a long ways off, I was hoping to avoid having to release my own PDF::Builder XS module just for this one split_alpha function (and remove it later when your update came out). Per your previous response, it sounds like you are amenable to getting it out sooner than later (just some tests and documentation), which would fit fine with my plans.

benkasminbullock commented 4 years ago

On Sat, 5 Sep 2020 at 23:59, Phil Perry notifications@github.com wrote:

OK, if there's not a slightly "cleaner" way to get the data for non-Alpha modes, no big deal. The current code works fine and probably would be sped up only slightly. I just thought that since you had apparently gone ahead and done it for the Alpha modes (built up the full data from rows internally, if that's what you did), that you might have also done it for non-Alpha modes (or at least, most of the work was already done).

Do you have a feel for when you might complete the split_alpha and release on CPAN? Maybe within a couple months, or "no idea" at this point? If it's a long ways off, I was hoping to avoid having to release my own PDF::Builder XS module just for this one split_alpha function (and remove it later when your update came out). Per your previous response, it sounds like you are amenable to getting it out sooner than later (just some tests and documentation), which would fit fine with my plans.

I need to do some small jobs before releasing it which consists of writing a simple test and sending another testing version to CPAN.

I'll try to get it done soon and let you knwo.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/benkasminbullock/image-png-libpng/issues/29#issuecomment-687622396, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAY45IJGGE7EB6YGE3W5XDSEJG3ZANCNFSM4MI7NSWQ .

benkasminbullock commented 4 years ago

A new version including tests is on CPAN:

https://metacpan.org/release/BKB/Image-PNG-Libpng-0.46_02

If the tests go reasonably well the next stage is to bump the version and then this should be ready for your system to use.

PhilterPaper commented 4 years ago

Hi Ben,

I have tried 0.46_02 and it appears to be working well. No errors, high speed, PDFs made with .png files appear to be identical to the old way. All I did was change in my code kungfu to split_alpha -- was there anything else to change?

the next stage is to bump the version and then this should be ready

Don't forget to update the documentation... it currently doesn't appear to mention split_alpha!

benkasminbullock commented 4 years ago

It seems to have passed all the tests so I'll set the version to a non-testing version and release to CPAN:

https://metacpan.org/release/BKB/Image-PNG-Libpng-0.47

On Mon, 14 Sep 2020 at 00:03, Phil Perry notifications@github.com wrote:

Hi Ben,

I have tried 0.46_02 and it appears to be working well. No errors, high speed, PDFs made with .png files appear to be identical to the old way. All I did was change in my code kungfu to split_alpha -- was there anything else to change?

the next stage is to bump the version and then this should be ready

Don't forget to update the documentation... it currently doesn't appear to mention split_alpha!

Thank you for the reminder!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/benkasminbullock/image-png-libpng/issues/29#issuecomment-691682603, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAY45MW7365IZ23DMDDRHLSFTNLVANCNFSM4MI7NSWQ .

PhilterPaper commented 4 years ago

Thank you for the effort to get this update out; I've tested it (0.47) with my final code and it works well. The next release of PDF::Builder, in a month or two, will use the new code.

If I may, I'd like to suggest an improvement to the documentation: Under split_alpha(), mention that if your purpose of splitting out the Alpha channel is simply to discard it, that you may get better performance by using PNG_TRANSFORM_STRIP_ALPHA instead. This wouldn't warrant a new release by itself, but could be in 0.48, whenever that comes out.

Note that there are a number of calls and PNG_* constants to create a new Alpha channel from, say, a tRNS block. I would assume that split_alpha() should work with these, but I haven't tested them. If you haven't specifically tested them either, you might want to think about doing so!

benkasminbullock commented 4 years ago

On Sat, 19 Sep 2020 at 08:57, Phil Perry notifications@github.com wrote:

Thank you for the effort to get this update out; I've tested it (0.47) with my final code and it works well. The next release of PDF::Builder, in a month or two, will use the new code.

If I may, I'd like to suggest an improvement to the documentation: Under split_alpha(), mention that if your purpose of splitting out the Alpha channel is simply to discard it, that you may get better performance by using PNG_TRANSFORM_STRIP_ALPHA instead. This wouldn't warrant a new release by itself, but could be in 0.48, whenever that comes out.

Thanks, I've added that to the documentation in 8b8b4e0e748653a61809a4d8d1d64f7f2c042a87.

Note that there are a number of calls and PNG_ constants to create* a new Alpha channel from, say, a tRNS block. I would assume that split_alpha() should work with these, but I haven't tested them. If you haven't specifically tested them either, you might want to think about doing so!

OK. split_alpha might have some unforeseen problems so do let me know if you find anything suspicious.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/benkasminbullock/image-png-libpng/issues/29#issuecomment-695132138, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAY45OMXSKV2J277SK4VH3SGPXXNANCNFSM4MI7NSWQ .