dark-panda / gd2-ffij

The Ruby/GD2 interface using FFI bindings.
Other
19 stars 12 forks source link

Why don't you use gdImagePaletteToTrueColor? #23

Open hkmaly opened 3 years ago

hkmaly commented 3 years ago

libGD has function gdImagePaletteToTrueColor. It's supposed to do exactly what your to_true_color is supposed to do, except that it actually WORKS for palette images with transparency.

Is there any reason why you aren't using it?

To provide example: 748482-ingo-s-tasty-food-logo-2880x2880

Load it with

require 'gd2-ffij'

file_in = ARGV.shift || '748482-ingo-s-tasty-food-logo-2880x2880.png'
file_pt = ARGV.shift || file_in.gsub(/\.png$/, '').gsub(/-\d+x\d+$/,'')

data = File.open(file_in, "rb").read
img  = GD2::Image.load(data)

Then compare following two outputs:

img = img.to_true_color
img.save_alpha = true
data = img.png(9.5)
File.open(file_pt + '-test01.png', 'wb') {|f|
    f.write(data)
}
::GD2::GD2FFI.attach_function(:gdImagePaletteToTrueColor, [:pointer], :pointer)
::GD2::GD2FFI.send(:gdImagePaletteToTrueColor, img.image_ptr)
img.save_alpha = true
data = img.png(9.5)
File.open(file_pt + '-test02.png', 'wb') {|f|
    f.write(data)
}
hkmaly commented 3 years ago

... oh. Because that function is only available in newer versions of library. Still, maybe with some detection?

dark-panda commented 3 years ago

These bindings were a minor reworking of the existing gd2 Ruby bindings that were written like 15 years ago, which used the old dl Ruby dynamic linker module, which I replaced with ffi. I didn't do much outside of directly replacing the dl calls with ffi at the time, and haven't done much to the library outside of fixes here and there, as it was intended to be a drop-in replacement for the original library.

The #to_true_color method is as it was in the original code, which you can see here: https://github.com/jmccaffrey/gd2/blob/master/lib/gd2/image.rb#L737.

Would something like this suffice?

https://github.com/dark-panda/gd2-ffij/compare/fix-to-true-color

hkmaly commented 3 years ago

I actually think that the copy_from is unnecessary, that you can just replace the image pointer ... I mean,

TrueColor.allocate.init_with_image(img.image_ptr)

like it's used in to_indexed_color. But seems that this will work as well.

Note that I also found another "solution" without using new function (so, for older gd2). If you replace copy_from(self, 0, 0, 0, 0, sz) with copy_from(self, 0, 0, 0, 0, sz, *sz) , forcing use of gdImageCopyResampled instead of gdImageCopy, it's slower but preserves transparency correctly.

dark-panda commented 3 years ago

If these all work I'll just take the most performant then I suppose.

hkmaly commented 3 years ago

Well, I'm doing both based on existence of gdImagePaletteToTrueColor. Don't want to risk I have old library hanging somewhere - note that the initialization code prefers oldest library it can find.

hkmaly commented 3 years ago

I mean, both "init_with_image" version and "copy_from(self, 0, 0, 0, 0, sz, sz)" version. I'll skip the version with unnecessary copy_from.