Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why don't you use gdImagePaletteToTrueColor? #23

Open
hkmaly opened this issue May 24, 2021 · 6 comments
Open

Why don't you use gdImagePaletteToTrueColor? #23

hkmaly opened this issue May 24, 2021 · 6 comments

Comments

@hkmaly
Copy link

hkmaly commented May 24, 2021

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
Copy link
Author

hkmaly commented May 24, 2021

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

@dark-panda
Copy link
Owner

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
Copy link
Author

hkmaly commented May 25, 2021

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
Copy link
Owner

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

@hkmaly
Copy link
Author

hkmaly commented May 25, 2021

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
Copy link
Author

hkmaly commented May 25, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants