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

Black background on resize/rotate for transparent images #236

Open
raffaelj opened this Issue Nov 22, 2018 · 12 comments

Comments

Projects
None yet
2 participants
@raffaelj

raffaelj commented Nov 22, 2018

The issue is the same as #192 and #188, which are closed, but not fixed. I have the same problem with transparent png files. bestFit works, but thumbnail fails on crop.

Sample code:

$img = new \claviska\SimpleImage();
$img
  ->fromFile('png-32.png')
  ->thumbnail(100, 80)
  ->toFile('thumbnail.png');

setup:

SimpleImage: latest (3.3.3)

CentOS 7

PHP version: PHP 7.2.12 (cli) (built: Nov 6 2018 15:26:22) ( NTS )
Zend Engine: v3.2.0

GD library Version: 2.2.5
FreeType Version: 2.4.11
libJPEG Version: 6b
libPNG Version: 1.5.13

result:

thumbnail

original image:

png-32

local setup works:

It works like expected on my localhost with Xampp, PHP 7.2 and

GD Version: bundled (2.1.0 compatible)
FreeType Version: 2.8.1
libJPEG Version: 9 compatible
libPNG Version: 1.6.34

fix:

I found a code snippet in SimpleImage for transparent gif files, modified it a bit and added it to the crop method:

raffaelj@31959f0

What I really want to do:

Oh - and the actual reason is, to fix black thumbnails in Cockpit CMS on the shared host Uberspace, where I can't change the gd version. Their support said

PHP is compiled against the latest version and it's not possible to change it easily.

@claviska

This comment has been minimized.

Owner

claviska commented Nov 26, 2018

This looks good at a glance, but I'd really like to get some more feedback from other users before making this change to the crop method.

In the past, I've fixed edge cases like this that ended up created problems for existing users. The GD transparency issue has been a game of whack-a-mole. (I think this is an area where tests could really help.)

@raffaelj

This comment has been minimized.

raffaelj commented Nov 26, 2018

I understand... and I never used gd or SimpleImage before. So I don't know, what could be possible in certain scenarios. I read a few issues, digged a bit in the code and just found my simple fix.

It looks like the issue could be here with a fix for the next gd release (2.2.6):
libgd/libgd#432 (comment)

Interesting, PHP's bundled libgd doesn't seem to be affected by this issue, because the implementation was "slightly" different from the start: libgd vs. PHP.

So maybe it would be better to check against the gd version instead of gif/png... pseudo code:

if ($this->hasAlphaChannel && ($gdVersion !=  "bundled" || $gdVersion >= "2.2.6")) {
    // $this->image = new Image with transparent background and copy original image
} else {
    // use $this->image
}

Do you have an overview about working and not working gd versions?

@claviska

This comment has been minimized.

Owner

claviska commented Nov 26, 2018

I don't, and unfortunately this is exactly why PHP+GD is a pain to work with:

Interesting, PHP's bundled libgd doesn't seem to be affected by this issue, because the implementation was "slightly" different from the start: libgd vs. PHP.

So not all "2.2.6" versions are the same. Plus, as your host stated, it's not exactly the easiest thing to pick the GD version you want to use with PHP so that makes things even harder to test. 😖

@raffaelj

This comment has been minimized.

raffaelj commented Nov 26, 2018

I did some research...

There is hope, but not much, that libgd will be added to the php dependencies instead of bundling it. So maybe it's possible to have ONE gd version in the future.
See: https://externals.io/message/101801 and php/php-src#3080

I found some interesting constants to check the version and I wrote a simple test file. The test said, the crop function doesn't work with gif files, too. So my PR is not complete yet.

left: Xampp, right: remote, specs like above

simpleimage-compare-versions

content of testfile:

<!doctype html>
<html><head></head><body>
<?php

require_once('SimpleImage.php');

$img = new \claviska\SimpleImage();
$img->fromFile('stripes.png');
$img->crop(10, 10, 40, 40);
$img->toFile('thumb.png');

$img = new \claviska\SimpleImage();
$img->fromFile('stripes.gif');
$img->crop(10, 10, 40, 40);
$img->toFile('thumb.gif');

// echo '<pre>' . print_r(get_defined_constants(), true) . '</pre>';
// echo '<pre>' . print_r(gd_info(), true) . '</pre>';

echo "PHP_VERSION:" . '<pre>' . print_r(PHP_VERSION, true) . '</pre>';
echo "PHP_OS:" . '<pre>' . print_r(PHP_OS, true) . '</pre>';
echo "gd_info()['GD Version']:" . '<pre>' . print_r(gd_info()['GD Version'], true) . '</pre>';
echo "GD_BUNDLED:" . '<pre>' . print_r(GD_BUNDLED, true) . '</pre>';
echo "GD_VERSION:" . '<pre>' . print_r(GD_VERSION, true) . '</pre>';

?>

<p>command:</p>
<pre>$img->crop(10, 10, 40, 40);</pre>

<div><p>png</p>
<img src="stripes.png">
<img src="thumb.png">
</div>

<div><p>gif</p>
<img src="stripes.gif">
<img src="thumb.gif">
</div>

</body></html>

original files:

gif:
stripes

png:
stripes

Now I'm not sure, what to do next. My goal is to fix (all) transparent gif and png issues - at least in the crop function. resize seems to work already. The next steps would be to publish a new SimpleImage release 3.3.4 so Cockpit CMS can be updated via composer dependencies and I can use my favourite web hoster. And I want to achieve this goal soon.

Everything I know about gd is what I read about it today and 4 days ago. This is probably not enough to write a consistent version check. Adding a check for gif and png in the crop function would be easy. I can do some more tests with other functions, too. But I'm very limited to a small set of test hosts.

Can you point me in a direction, please? Otherwise I would try to write a workaround for my specific needs and wait/hope for a fix in an upcoming PHP version with libgd 2.2.6.

@raffaelj

This comment has been minimized.

raffaelj commented Nov 28, 2018

The GD transparency issue has been a game of whack-a-mole. (I think this is an area where tests could really help.)

I wrote a simple test file here: https://github.com/raffaelj/SimpleImageTest/

The tests are with the original lib without my modification to crop. The code should be self-explanatory.

Just add a new line to the test array

$tests = [
//  ['method' => ['arg1', 'arg2']  ],
    ['crop'   => [10, 10, 40, 40]],
    ['rotate' => [45, 'green']],
    ['resize' => [40, 80]],
    ['thumbnail' => [60, 60, 'center']],
    ['sketch' => []],
];

or add different images to the input directory.

Have a look at the screenshots. The results are weird...

  • rotated gif (not transparent) with black border
  • completely broken rotated gif (transparent)
  • one host can rotate but not crop and the other host can't rotate transparent gif, but crop works
  • ...

I don't get it...

@claviska

This comment has been minimized.

Owner

claviska commented Nov 28, 2018

Can you point me in a direction, please?

I wish I could...I've been dealing with this one and off for years. Sometimes, I can reproduce it and fix it (and it usually ends up bungling somebody else's version). Other times, I can't reproduce it at all regardless of the version. I've come to the conclusion that it's something to do with the way PHP bundles GD, but I don't know enough about the internals to even guess.

It's gotten to the point where we have a reasonably high effectiveness, but there's always another edge case popping up.

If we can't find a proper fix, my suggestion is to use the library as-is and, in your own code, add a method that works for your particular instance. You won't be able to chain it, but at least 1) it will work and 2) you won't have to rely on a modified version of the library. Just make sure it returns a SimpleImage object and you should be fine.

@raffaelj

This comment has been minimized.

raffaelj commented Nov 28, 2018

I updated the test repo. Now the output is a bit better and I added two more screenshots from different hosts. I also started a call in the Cockpit Communitiy Forum to help with tests on different hosts. So hopefully you have a base to work with soon.

If we can't find a proper fix, my suggestion is to use the library as-is and, in your own code, add a method that works for your particular instance.

It shouldn't be too hard to write a fix for Cockpit (on my instance). It uses only thumbnail() (turns black) and bestFit() (resize works). But it would feel wrong.

raffaelj added a commit to raffaelj/SimpleImage that referenced this issue Nov 29, 2018

improved fixing transparent png/gif turning black
It seems, that all edge cases must be handled separately, like

* gif without transparency
* transparent gif 8bit
* transparent gif 1bit
* bundled or not bundled
* ...

`crop` and `resize` in combination seems to work now.

I started to fix `rotate` on bundled systems, but there are still some
issues, so I commented the code out.

I'll add the test screenshots to claviska#236
@raffaelj

This comment has been minimized.

raffaelj commented Nov 29, 2018

I wrote some more for multiple edge cases... libgd is annoying. Everytime I thought, it would work, another image had artefacts or was completely empty.

So obviously crop and resize have to work in harmony with 1bit and 8bit transparent gifs and sometimes only for bundled/non-bundled versions.

Here are the screenshots from my latest commit. It looks promising.

Works for me. What do you think?

bildschirmfoto am 2018-11-29 um 05 36 08-fullpage
bildschirmfoto am 2018-11-29 um 05 35 59-fullpage
bildschirmfoto am 2018-11-29 um 05 35 49-fullpage
bildschirmfoto am 2018-11-29 um 05 35 44-fullpage

@claviska

This comment has been minimized.

Owner

claviska commented Nov 29, 2018

This is looking good. I'll add some comments to the PR in a moment.

@claviska claviska changed the title from v3.3.3 thumbnail method - black background for transparent png to Black background on resize/rotate for transparent images Nov 29, 2018

@raffaelj

This comment has been minimized.

raffaelj commented Dec 3, 2018

I gave it another try and now I think, I'm on the right way.

I added some checks to skip the copy-to-new-image process when it's not necessary.
I also changed the transparent color to the correct one in fromFile().

Now crop and resize work on both hosts (bundled and libgd), but applying color filters leads to losing transparency again on non-bundled systems.
See: https://github.com/claviska/SimpleImage/pull/237/files#diff-88dd2efe0e7f544f22fd56eab1ebad03R151

I also updated the test repo with gif files with all bitrates. The complicated ones are

  • black - rgb(0,0,0) --> imagefill with autodetected black as background can lead to losing the transparency
  • 1bit black or coloured - they are "black and white" instead of rgb, the first color field is visible and the second one is transparent

screenshots:

first one on localhost with bundled gd, second on remote with libgd
The files are sorted by bitrate

bildschirmfoto am 2018-12-03 um 02 02 40-fullpage

bildschirmfoto am 2018-12-03 um 02 03 06-fullpage

@raffaelj

This comment has been minimized.

raffaelj commented Dec 4, 2018

I digged again into the code and I want to share my conclusions and theories.

Gif files don't have alpha channels, they have one color in the palette, which is used as alpha. So they can't be handled like png files


https://github.com/claviska/SimpleImage/blob/master/src/claviska/SimpleImage.php#L136-L145
https://github.com/claviska/SimpleImage/blob/master/src/claviska/SimpleImage.php#L163

  • detecting the transparent color with imagecolortransparent doesn't work (correct) anymore
  • without imagepalettetotruecolor detecting imagecolortransparent works, but all imagefilters don't work anymore

https://github.com/claviska/SimpleImage/blob/master/src/claviska/SimpleImage.php#L234

imagesavealpha and imagealphablending are for png files. They have no effect on gif files


A theory, why rotate leads to losing transparency: If you rotate a black rectangle, the edges turn gray and the whole color palette moves/gets bigger. So the position of the original transparent pixel moves, too. Not sure about this, but I realized a difference when rotating a black square with Illustrator and exporting it. The color palette was bigger with a few gray values.


I found a very simple gd image class to crop and resize and I wanted to know, how the author fixed the gif transparency issue (docs).
https://github.com/deniskomlev/php-imageresize/blob/master/ImageResize.php#L470-L491

It looks nice, crop and resize work on bundled and libgd. But it turns black when calling crop and resize in one run. It also loses transparency with 1bit-transparent-white gif files.

I tried a rewrite with this approach, fixed the 1bit-white-bug - and lost the transparency in thumbnails again (crop + resize = 2x truecolorimage = weird_imagecolortransparent).

https://github.com/raffaelj/SimpleImage/blob/rewrite-preserve-transparency/src/claviska/SimpleImage.php#L1945-L1962


Conclusion:

It was very interesting to learn a lot about the gd extension and about transparent gif files.

@claviska Nice work. Your library has a lot of useful functions.

It is really frustrating. Everytime I think, it works - the images on the other host have artefacts or turn black. So - it was interesting, but I have to go now and search for another workaround instead to display my transparent thumbnails on a non-bundled system.

@claviska

This comment has been minimized.

Owner

claviska commented Dec 4, 2018

GD can definitely be very frustrating, which is why I wrote SimpleImage in the first place. This issue, however, has proven impossible to perfect — thanks for trying and sharing your experience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment