Skip to content

Conversation

@ADmad
Copy link
Collaborator

@ADmad ADmad commented Jan 30, 2024

@konnng-dev I took your changes from Art4#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
Copy link
Contributor

Art4 commented Jan 30, 2024

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

ADmad commented Jan 30, 2024

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

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

@konnng-dev
Copy link
Contributor

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.

Interlacing for JPGs only seems to be working when using Imagick.
So need to look into that later or state that as a limitation in the docs.
@ADmad
Copy link
Collaborator Author

ADmad commented Jan 30, 2024

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

You can open a separate PR for that.

@ADmad
Copy link
Collaborator Author

ADmad commented Jan 30, 2024

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.

@ADmad ADmad merged commit 874a11f into 3.x Jan 30, 2024
@ADmad ADmad deleted the 3.x-interlacing branch January 30, 2024 14:27
@konnng-dev
Copy link
Contributor

konnng-dev commented Jan 30, 2024

@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

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

Successfully merging this pull request may close these issues.

4 participants