search

thephpleague / glide

Wonderfully easy on-demand image manipulation library with an HTTP based API.
http://glide.thephpleague.com
MIT License
2.54k stars 198 forks source link

Fix interlacing #387

Closed ADmad closed 6 months ago

ADmad commented 6 months ago

@konnng-dev I took your changes from https://github.com/Art4/glide/pull/2 to fix interlacing.

Though when testing the generated image, interlacing doesn't seem to work for JPEGs when using GD. With Imagick identify -verbose kayak.jpeg | grep Interlace gives Interlace: JPEG but with GD it gives Interlace: None. It seems to work fine for PNGs for either driver.

@Art4 BTW Glide v2 doesn't interlace PNGs but you have added interlacing for all PNGs in v3. Is that intentional?

Art4 commented 6 months ago

@Art4 BTW Glide v2 doesn't interlace PNGs but you have added interlacing for all PNGs in v3. Is that intentional?

Interlacing for all png was added by @konnng-dev in Art4#1

ADmad commented 6 months ago

Interlacing for all png was added by @konnng-dev in https://github.com/Art4/glide/pull/1

Hmm.. I am leaning towards reverting it and maintaining status quo.

konnng-dev commented 6 months ago

Oh, it was indeed my fault

I can submit a new PR just interlacing JPEGS. But I would like to have option to interlace pngs on demand as well.

ADmad commented 6 months ago

But I would like to have option to interlace pngs on demand as well.

You can open a separate PR for that.

ADmad commented 6 months ago

Gonna merge this. Will look into JPG interlacing not working with Gd later or just state that as a limitation if me or someone else can't sort it out.

konnng-dev commented 6 months ago

@ADmad I can confirm that interlaced JPEG doesn't work w/ GD

Looking deep, it seems it is caused by intervention GD Driver JpegEncoder. It seems the encoder clones the image instance, losing the intervention flag set.

https://github.com/Intervention/image/blob/develop/src/Drivers/Gd/Encoders/JpegEncoder.php#L19