Fully supports progressive JPGS and Interlaced images for 3.x by konnng-dev · Pull Request #394 · thephpleague/glide

konnng-dev · 2024-04-22T21:36:10Z

The intent of this PR is to introduce support to generate progressive JPEGs or interlaced images.

Couple days ago, the author of Intervention provided support to accomplish that https://github.com/Intervention/image/releases/tag/3.6.0

With that, I did some adjustments on Glide API to get the desired support.

Highlights:

Encode manipulator moved from the $manipulators list, because it needs to be the last manipulator to run when generating the images.
Encode manipulator now returns a EncodedImage. This change avoids to double encode image when generating it.
Encode also makes more use of Intervention enums to encode the image properly.
interlace parameter also was added to create progressive JPEGs or interlaced PNGs or GIFs.

konnng-dev · 2024-04-22T21:36:49Z

TODO:

work on tests to check for progressive / interlaced images.
~~fix code formatting issues~~

konnng-dev · 2024-04-22T21:39:23Z

@ADmad would you mind to take a look and tell me what you think?

I'm not sure if introducing this new encoding strategy would be the best for this package.

feedbacks are welcome :)

ADmad · 2024-04-27T04:20:19Z

Sorry I haven't had the time to look into this but could you please address the failing tests in the meantime.

konnng-dev · 2024-05-09T14:05:33Z

@ADmad I addressed all the issues that Github Actions was issuing.

Before setting this ready for review, could you point me the best place where I could add the tests for the progressive/interlaced images I am introducing?

src/Manipulators/Encode.php

src/ServerFactory.php

src/Manipulators/Encode.php

src/Api/Api.php

konnng-dev · 2024-05-20T13:05:29Z

@ADmad I added some comments to help the review process.

Ideally, it would be good to add tests for the new option added to the image API. But I don't have idea what would be the best place to put it.

src/Manipulators/Encode.php

src/Api/Api.php

…ages

konnng-dev · 2024-06-03T18:08:03Z

@ADmad I applied the suggested changes and also add a new doc entry for the new option. I'm just not sure how to preview documentation locally, but I guess it is following the same convention used before.

Also, do you have idea on how we could test this new feature? I just updated EncodeTest to comply with the recent changes, but I didn't have idea on how test that.

konnng-dev · 2024-06-25T13:17:47Z

@ADmad friendly ping to check if you had time to review changes.

ADmad

An issue with having the encoder as a separate step instead of a manipulation is that one cannot add manipulators to be run after the image is encoded. For e.g. there's this optimizer manipulator which needs to run after encoding.

ADmad · 2024-06-25T16:58:13Z

src/Api/Encode.php

+    /**
+     * The manipulation params.
+     */
+    private $params;


Suggested change

private $params;

private array $params;

ADmad · 2024-06-25T17:04:50Z

tests/Api/EncodeTest.php

+     *
+     * @psalm-suppress PropertyNotSetInConstructor
+     */
+    private $encoder;


You can specify the type for all these properties and avoid the @var tags.

ADmad · 2024-06-25T17:07:04Z

tests/Api/EncodeTest.php

+            $manager->create(100, 100)->encode(new MediaTypeEncoder('image/gif'))->toFilePointer()
+        );
+
+        if (function_exists('imagecreatefromwebp')) {


Shouldn't this function be always available for PHP 8.1+?

ADmad · 2024-06-25T17:07:11Z

tests/Api/EncodeTest.php

+            );
+        }
+
+        if (function_exists('imagecreatefromavif')) {


Shouldn't this function be always available for PHP 8.1+?

ADmad · 2024-06-25T17:09:04Z

tests/Api/EncodeTest.php

+        /**
+         * @psalm-suppress ArgumentTypeCoercion
+         */
+        $this->assertInstanceOf('League\Glide\Api\Encode', $this->encoder);


Suggested change

$this->assertInstanceOf('League\Glide\Api\Encode', $this->encoder);

$this->assertInstanceOf(Encode::class, $this->encoder);

with use statement for it.

ADmad · 2024-06-25T17:11:32Z

tests/Api/EncodeTest.php

+        $this->assertSame('image/jpeg', $this->getMime($this->encoder->setParams(['fm' => 'jpg'])->run($this->jpg)));
+        $this->assertSame('image/jpeg', $this->getMime($this->encoder->setParams(['fm' => 'jpg'])->run($this->png)));
+        $this->assertSame('image/jpeg', $this->getMime($this->encoder->setParams(['fm' => 'jpg'])->run($this->gif)));
+        $this->assertSame('image/jpeg', $this->getMime($this->encoder->setParams(['fm' => 'pjpg'])->run($this->jpg)));


Do we have a way to check if the image is actually interlaced?

ADmad · 2024-06-25T17:12:21Z

docs/3.0/api/progressive-interlaced.md

+
+The `interlaced` parameter controls whether an image is rendered in a progressive or interlaced format. This feature enhances the loading experience of images, making them appear gradually as they are downloaded, which can improve the user experience on slower connections.
+
+> Caution: for GIF/PNG, it can generate a sligher larger file size.


Suggested change

> Caution: for GIF/PNG, it can generate a sligher larger file size.

> Caution: for GIF/PNG, it can generate a slightly larger file size.

ADmad · 2024-06-25T17:14:40Z

docs/3.0/api/progressive-interlaced.md

+- **JPG**: The `Interlaced` parameter applies a progressive scan to JPG images.
+- **PNG** and **GIF**: The `Interlaced` parameter enables interlacing for GIF/PNG images.
+
+> Note: Case `ext` is set to `.pjpg`, it will automatically generate a progressive JPG image, regardless of the `interlaced` parameter.


Suggested change

> Note: Case `ext` is set to `.pjpg`, it will automatically generate a progressive JPG image, regardless of the `interlaced` parameter.

> Note: In case the `fm` parameter is set to `.pjpg`, it will automatically generate a progressive JPG image, regardless of the `interlaced` parameter.

C2H6-383 · 2024-09-22T13:50:42Z

Just wanted to check up on the progress of this pr: Are there any updates about the progression to merge this into 3.x?

I would really like to see version 3 to be released but afaik the release depends on the merge of this pr...

konnng-dev · 2024-09-23T17:35:12Z

@C2H6-383 I ended up forgetting to push the asked changes to this PR. I appreciate the reminder.

I will try to address it this week so @ADmad can reviiew it.

C2H6-383 · 2024-10-23T06:34:49Z

Hey @konnng-dev, just wanted to check up on this issue again. Are there more updates on this PR to allow it to be merged? 😅

ADmad · 2024-10-27T05:43:31Z

Since @konnng-dev seems to be unavailable I'll try and finish this up myself.

konnng-dev · 2024-10-27T17:37:07Z

Thanks for giving proper attention @ADmad and my apologies for overpromising. I ended up delivering the project using this package for the client and moved on to a different projects. Which caused me to sidetrack and lost focus on delivering this one.

I hope my updates helps this library to be better compatible with Intervention v3.

ADmad · 2024-10-27T17:42:27Z

@konnng-dev No apology needed, thank you for your contributions.

ADmad mentioned this pull request Apr 27, 2024

3.x release date #395

Closed

konnng-dev commented May 20, 2024

View reviewed changes

src/Manipulators/Encode.php Show resolved Hide resolved

src/Manipulators/Encode.php Show resolved Hide resolved

src/ServerFactory.php Show resolved Hide resolved

src/Manipulators/Encode.php Show resolved Hide resolved

src/Api/Api.php Outdated Show resolved Hide resolved

konnng-dev marked this pull request as ready for review May 20, 2024 13:03

ADmad reviewed May 26, 2024

View reviewed changes

src/Manipulators/Encode.php Show resolved Hide resolved

src/Manipulators/Encode.php Show resolved Hide resolved

src/Manipulators/Encode.php Show resolved Hide resolved

src/Api/Api.php Outdated Show resolved Hide resolved

konnng-dev added 12 commits June 3, 2024 11:02

chore: dependency bump

6103f18

fix: update image generation to support progressive and interlaced im…

8113701

…ages

fix: linting issues

9a17fb5

fix: remove repeated condition

923669c

fix(test): fix Encoding test

6d50e0f

fix: revert changes

0a78cc5

fix(test): fix ApiTest assertions

6a6cce2

fix: update logic to avoid psalm error

bb744d1

chore: update returning types documentation

11fb220

fix: psalm validation issues addressed

b0086f8

fix: move Encoder manipulator do Api

62623fe

fix: revert typings return

c5eb1e2

juliovedovatto force-pushed the 3.x branch from eb3745d to c5eb1e2 Compare June 3, 2024 14:02

konnng-dev added 4 commits June 3, 2024 12:03

test: move Encoder test

a602e8f

test: fix test typings

d2dd5f2

test: fix lint issue

db23123

chore: docs entry added

c4211b1

konnng-dev requested a review from ADmad June 3, 2024 18:08

ADmad reviewed Jun 25, 2024

View reviewed changes

ADmad changed the base branch from 3.x to feat/progressive-interlaced October 27, 2024 05:41

ADmad merged commit d3e4bcf into thephpleague:feat/progressive-interlaced Oct 27, 2024

	$this->assertInstanceOf('League\Glide\Api\Encode', $this->encoder);
	$this->assertInstanceOf(Encode::class, $this->encoder);


		The `interlaced` parameter controls whether an image is rendered in a progressive or interlaced format. This feature enhances the loading experience of images, making them appear gradually as they are downloaded, which can improve the user experience on slower connections.

		> Caution: for GIF/PNG, it can generate a sligher larger file size.

	> Note: Case `ext` is set to `.pjpg`, it will automatically generate a progressive JPG image, regardless of the `interlaced` parameter.
	> Note: In case the `fm` parameter is set to `.pjpg`, it will automatically generate a progressive JPG image, regardless of the `interlaced` parameter.

Conversation

konnng-dev commented Apr 22, 2024

Uh oh!

konnng-dev commented Apr 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

konnng-dev commented Apr 22, 2024

Uh oh!

ADmad commented Apr 27, 2024

Uh oh!

konnng-dev commented May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

konnng-dev commented May 20, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

konnng-dev commented Jun 3, 2024

Uh oh!

konnng-dev commented Jun 25, 2024

Uh oh!

ADmad left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ADmad Jun 25, 2024

Choose a reason for hiding this comment

Uh oh!

ADmad Jun 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ADmad Jun 25, 2024

Choose a reason for hiding this comment

Uh oh!

ADmad Jun 25, 2024

Choose a reason for hiding this comment

Uh oh!

ADmad Jun 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ADmad Jun 25, 2024

Choose a reason for hiding this comment

Uh oh!

ADmad Jun 25, 2024

Choose a reason for hiding this comment

Uh oh!

ADmad Jun 25, 2024

Choose a reason for hiding this comment

Uh oh!

C2H6-383 commented Sep 22, 2024

Uh oh!

konnng-dev commented Sep 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

C2H6-383 commented Oct 23, 2024

Uh oh!

ADmad commented Oct 27, 2024

Uh oh!

konnng-dev commented Oct 27, 2024

Uh oh!

ADmad commented Oct 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

konnng-dev commented Apr 22, 2024 •

edited

Loading

konnng-dev commented May 9, 2024 •

edited

Loading

ADmad left a comment •

edited

Loading

ADmad Jun 25, 2024 •

edited

Loading

ADmad Jun 25, 2024 •

edited

Loading

konnng-dev commented Sep 23, 2024 •

edited

Loading