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

Fix interlacing #387

Merged
merged 1 commit into from
Jan 30, 2024
Merged

Fix interlacing #387

merged 1 commit into from
Jan 30, 2024

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

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

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
8 of 9 checks passed
@ADmad ADmad deleted the 3.x-interlacing branch January 30, 2024 14:27
@konnng-dev
Copy link

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.

3 participants