Lossless conversion for webp - #7348

Closed
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless
Closed

Lossless conversion for webp#7348
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless

Conversation

@morsssss

Copy link
Copy Markdown
Contributor

I'm propagating lossless conversion from libgd to our bundled gd.

I've also changed "quantization" to "quality", as it is in libgd, making our webp code just a little closer to what's in libgd.

Added test as well.

Note that this change adds a new possible value of 101 for quality, indicating losslessness. This will need to be documented.

/cc @adamsilverstein

Propagating lossless conversion from libgd to our bundled gd.
Changing "quantization" to "quality" as in libgd.
Adding test.
Comment threadext/gd/tests/webp_basic.phpt
@morsssss

Copy link
Copy Markdown
ContributorAuthor

P.S. This is the parallel PR in libgd: libgd/libgd#698

Comment threadext/gd/tests/webp_basic.phpt Outdated

@nikicnikic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @cmb69?

Comment threadext/gd/gd.c
Comment on lines +381 to +383
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if ext/gd is compiled against a libgd which does not support lossless WebP encoding? Furthermore, there is currently no way to detect whether it is supported. So maybe:

Suggested change
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);
#ifdefgdWebpLossless
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", gdWebpLossless, CONST_CS | CONST_PERSISTENT);
#endif

(and ditch the definition of PHP_IMG_WEBP_LOSSLESS)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I get it. Going forward, libgd will require a version of libwebp that's recent enough to support lossless WebP encoding. But your concern is that someone might use an older version of libgd. That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

If I'm getting that right, I think this solution is pretty clever.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

Right.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I see precedent for this in the code:

#ifdefgdEffectMultiplyREGISTER_LONG_CONSTANT("IMG_EFFECT_MULTIPLY", gdEffectMultiply, CONST_CS | CONST_PERSISTENT);
#endif

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! gdEffectMultiply is defined as of libgd 2.1.1, but we're still supporting 2.1.0. Still, something that can be removed sometime in the future.

@cmb69cmb69 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@morsssss

Copy link
Copy Markdown
ContributorAuthor

Thank you!

Thank you for your consistent patience and good cheer!

@cmb69cmb69 closed this in eb6c9ebAug 12, 2021
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Includes unit tests.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
jrfnl added a commit to jrfnl/doc-en that referenced this pull request Mar 11, 2022
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@morsssss@andypost@nikic@cmb69@krakjoe
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Lossless conversion for webp - #7348

Closed
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless
Closed

Lossless conversion for webp#7348
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless

Conversation

@morsssss

Copy link
Copy Markdown
Contributor

I'm propagating lossless conversion from libgd to our bundled gd.

I've also changed "quantization" to "quality", as it is in libgd, making our webp code just a little closer to what's in libgd.

Added test as well.

Note that this change adds a new possible value of 101 for quality, indicating losslessness. This will need to be documented.

/cc @adamsilverstein

Propagating lossless conversion from libgd to our bundled gd.
Changing "quantization" to "quality" as in libgd.
Adding test.
Comment threadext/gd/tests/webp_basic.phpt
@morsssss

Copy link
Copy Markdown
ContributorAuthor

P.S. This is the parallel PR in libgd: libgd/libgd#698

Comment threadext/gd/tests/webp_basic.phpt Outdated

@nikicnikic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @cmb69?

Comment threadext/gd/gd.c
Comment on lines +381 to +383
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if ext/gd is compiled against a libgd which does not support lossless WebP encoding? Furthermore, there is currently no way to detect whether it is supported. So maybe:

Suggested change
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);
#ifdefgdWebpLossless
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", gdWebpLossless, CONST_CS | CONST_PERSISTENT);
#endif

(and ditch the definition of PHP_IMG_WEBP_LOSSLESS)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I get it. Going forward, libgd will require a version of libwebp that's recent enough to support lossless WebP encoding. But your concern is that someone might use an older version of libgd. That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

If I'm getting that right, I think this solution is pretty clever.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

Right.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I see precedent for this in the code:

#ifdefgdEffectMultiplyREGISTER_LONG_CONSTANT("IMG_EFFECT_MULTIPLY", gdEffectMultiply, CONST_CS | CONST_PERSISTENT);
#endif

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! gdEffectMultiply is defined as of libgd 2.1.1, but we're still supporting 2.1.0. Still, something that can be removed sometime in the future.

@cmb69cmb69 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@morsssss

Copy link
Copy Markdown
ContributorAuthor

Thank you!

Thank you for your consistent patience and good cheer!

@cmb69cmb69 closed this in eb6c9ebAug 12, 2021
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Includes unit tests.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
jrfnl added a commit to jrfnl/doc-en that referenced this pull request Mar 11, 2022
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@morsssss@andypost@nikic@cmb69@krakjoe
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Lossless conversion for webp - #7348

Closed
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless
Closed

Lossless conversion for webp#7348
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless

Conversation

@morsssss

Copy link
Copy Markdown
Contributor

I'm propagating lossless conversion from libgd to our bundled gd.

I've also changed "quantization" to "quality", as it is in libgd, making our webp code just a little closer to what's in libgd.

Added test as well.

Note that this change adds a new possible value of 101 for quality, indicating losslessness. This will need to be documented.

/cc @adamsilverstein

Propagating lossless conversion from libgd to our bundled gd.
Changing "quantization" to "quality" as in libgd.
Adding test.
Comment threadext/gd/tests/webp_basic.phpt
@morsssss

Copy link
Copy Markdown
ContributorAuthor

P.S. This is the parallel PR in libgd: libgd/libgd#698

Comment threadext/gd/tests/webp_basic.phpt Outdated

@nikicnikic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @cmb69?

Comment threadext/gd/gd.c
Comment on lines +381 to +383
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if ext/gd is compiled against a libgd which does not support lossless WebP encoding? Furthermore, there is currently no way to detect whether it is supported. So maybe:

Suggested change
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);
#ifdefgdWebpLossless
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", gdWebpLossless, CONST_CS | CONST_PERSISTENT);
#endif

(and ditch the definition of PHP_IMG_WEBP_LOSSLESS)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I get it. Going forward, libgd will require a version of libwebp that's recent enough to support lossless WebP encoding. But your concern is that someone might use an older version of libgd. That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

If I'm getting that right, I think this solution is pretty clever.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

Right.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I see precedent for this in the code:

#ifdefgdEffectMultiplyREGISTER_LONG_CONSTANT("IMG_EFFECT_MULTIPLY", gdEffectMultiply, CONST_CS | CONST_PERSISTENT);
#endif

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! gdEffectMultiply is defined as of libgd 2.1.1, but we're still supporting 2.1.0. Still, something that can be removed sometime in the future.

@cmb69cmb69 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@morsssss

Copy link
Copy Markdown
ContributorAuthor

Thank you!

Thank you for your consistent patience and good cheer!

@cmb69cmb69 closed this in eb6c9ebAug 12, 2021
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Includes unit tests.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
jrfnl added a commit to jrfnl/doc-en that referenced this pull request Mar 11, 2022
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@morsssss@andypost@nikic@cmb69@krakjoe
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Lossless conversion for webp - #7348

Closed
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless
Closed

Lossless conversion for webp#7348
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless

Conversation

@morsssss

Copy link
Copy Markdown
Contributor

I'm propagating lossless conversion from libgd to our bundled gd.

I've also changed "quantization" to "quality", as it is in libgd, making our webp code just a little closer to what's in libgd.

Added test as well.

Note that this change adds a new possible value of 101 for quality, indicating losslessness. This will need to be documented.

/cc @adamsilverstein

Propagating lossless conversion from libgd to our bundled gd.
Changing "quantization" to "quality" as in libgd.
Adding test.
Comment threadext/gd/tests/webp_basic.phpt
@morsssss

Copy link
Copy Markdown
ContributorAuthor

P.S. This is the parallel PR in libgd: libgd/libgd#698

Comment threadext/gd/tests/webp_basic.phpt Outdated

@nikicnikic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @cmb69?

Comment threadext/gd/gd.c
Comment on lines +381 to +383
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if ext/gd is compiled against a libgd which does not support lossless WebP encoding? Furthermore, there is currently no way to detect whether it is supported. So maybe:

Suggested change
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);
#ifdefgdWebpLossless
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", gdWebpLossless, CONST_CS | CONST_PERSISTENT);
#endif

(and ditch the definition of PHP_IMG_WEBP_LOSSLESS)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I get it. Going forward, libgd will require a version of libwebp that's recent enough to support lossless WebP encoding. But your concern is that someone might use an older version of libgd. That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

If I'm getting that right, I think this solution is pretty clever.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

Right.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I see precedent for this in the code:

#ifdefgdEffectMultiplyREGISTER_LONG_CONSTANT("IMG_EFFECT_MULTIPLY", gdEffectMultiply, CONST_CS | CONST_PERSISTENT);
#endif

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! gdEffectMultiply is defined as of libgd 2.1.1, but we're still supporting 2.1.0. Still, something that can be removed sometime in the future.

@cmb69cmb69 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@morsssss

Copy link
Copy Markdown
ContributorAuthor

Thank you!

Thank you for your consistent patience and good cheer!

@cmb69cmb69 closed this in eb6c9ebAug 12, 2021
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Includes unit tests.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
jrfnl added a commit to jrfnl/doc-en that referenced this pull request Mar 11, 2022
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@morsssss@andypost@nikic@cmb69@krakjoe
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Lossless conversion for webp - #7348

Closed
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless
Closed

Lossless conversion for webp#7348
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless

Conversation

@morsssss

Copy link
Copy Markdown
Contributor

I'm propagating lossless conversion from libgd to our bundled gd.

I've also changed "quantization" to "quality", as it is in libgd, making our webp code just a little closer to what's in libgd.

Added test as well.

Note that this change adds a new possible value of 101 for quality, indicating losslessness. This will need to be documented.

/cc @adamsilverstein

Propagating lossless conversion from libgd to our bundled gd.
Changing "quantization" to "quality" as in libgd.
Adding test.
Comment threadext/gd/tests/webp_basic.phpt
@morsssss

Copy link
Copy Markdown
ContributorAuthor

P.S. This is the parallel PR in libgd: libgd/libgd#698

Comment threadext/gd/tests/webp_basic.phpt Outdated

@nikicnikic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @cmb69?

Comment threadext/gd/gd.c
Comment on lines +381 to +383
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if ext/gd is compiled against a libgd which does not support lossless WebP encoding? Furthermore, there is currently no way to detect whether it is supported. So maybe:

Suggested change
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);
#ifdefgdWebpLossless
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", gdWebpLossless, CONST_CS | CONST_PERSISTENT);
#endif

(and ditch the definition of PHP_IMG_WEBP_LOSSLESS)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I get it. Going forward, libgd will require a version of libwebp that's recent enough to support lossless WebP encoding. But your concern is that someone might use an older version of libgd. That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

If I'm getting that right, I think this solution is pretty clever.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

Right.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I see precedent for this in the code:

#ifdefgdEffectMultiplyREGISTER_LONG_CONSTANT("IMG_EFFECT_MULTIPLY", gdEffectMultiply, CONST_CS | CONST_PERSISTENT);
#endif

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! gdEffectMultiply is defined as of libgd 2.1.1, but we're still supporting 2.1.0. Still, something that can be removed sometime in the future.

@cmb69cmb69 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@morsssss

Copy link
Copy Markdown
ContributorAuthor

Thank you!

Thank you for your consistent patience and good cheer!

@cmb69cmb69 closed this in eb6c9ebAug 12, 2021
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Includes unit tests.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
jrfnl added a commit to jrfnl/doc-en that referenced this pull request Mar 11, 2022
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@morsssss@andypost@nikic@cmb69@krakjoe
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Lossless conversion for webp - #7348

Closed
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless
Closed

Lossless conversion for webp#7348
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless

Conversation

@morsssss

Copy link
Copy Markdown
Contributor

I'm propagating lossless conversion from libgd to our bundled gd.

I've also changed "quantization" to "quality", as it is in libgd, making our webp code just a little closer to what's in libgd.

Added test as well.

Note that this change adds a new possible value of 101 for quality, indicating losslessness. This will need to be documented.

/cc @adamsilverstein

Propagating lossless conversion from libgd to our bundled gd.
Changing "quantization" to "quality" as in libgd.
Adding test.
Comment threadext/gd/tests/webp_basic.phpt
@morsssss

Copy link
Copy Markdown
ContributorAuthor

P.S. This is the parallel PR in libgd: libgd/libgd#698

Comment threadext/gd/tests/webp_basic.phpt Outdated

@nikicnikic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @cmb69?

Comment threadext/gd/gd.c
Comment on lines +381 to +383
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if ext/gd is compiled against a libgd which does not support lossless WebP encoding? Furthermore, there is currently no way to detect whether it is supported. So maybe:

Suggested change
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);
#ifdefgdWebpLossless
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", gdWebpLossless, CONST_CS | CONST_PERSISTENT);
#endif

(and ditch the definition of PHP_IMG_WEBP_LOSSLESS)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I get it. Going forward, libgd will require a version of libwebp that's recent enough to support lossless WebP encoding. But your concern is that someone might use an older version of libgd. That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

If I'm getting that right, I think this solution is pretty clever.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

Right.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I see precedent for this in the code:

#ifdefgdEffectMultiplyREGISTER_LONG_CONSTANT("IMG_EFFECT_MULTIPLY", gdEffectMultiply, CONST_CS | CONST_PERSISTENT);
#endif

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! gdEffectMultiply is defined as of libgd 2.1.1, but we're still supporting 2.1.0. Still, something that can be removed sometime in the future.

@cmb69cmb69 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@morsssss

Copy link
Copy Markdown
ContributorAuthor

Thank you!

Thank you for your consistent patience and good cheer!

@cmb69cmb69 closed this in eb6c9ebAug 12, 2021
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Includes unit tests.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
jrfnl added a commit to jrfnl/doc-en that referenced this pull request Mar 11, 2022
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@morsssss@andypost@nikic@cmb69@krakjoe
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Lossless conversion for webp - #7348

Closed
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless
Closed

Lossless conversion for webp#7348
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless

Conversation

@morsssss

Copy link
Copy Markdown
Contributor

I'm propagating lossless conversion from libgd to our bundled gd.

I've also changed "quantization" to "quality", as it is in libgd, making our webp code just a little closer to what's in libgd.

Added test as well.

Note that this change adds a new possible value of 101 for quality, indicating losslessness. This will need to be documented.

/cc @adamsilverstein

Propagating lossless conversion from libgd to our bundled gd.
Changing "quantization" to "quality" as in libgd.
Adding test.
Comment threadext/gd/tests/webp_basic.phpt
@morsssss

Copy link
Copy Markdown
ContributorAuthor

P.S. This is the parallel PR in libgd: libgd/libgd#698

Comment threadext/gd/tests/webp_basic.phpt Outdated

@nikicnikic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @cmb69?

Comment threadext/gd/gd.c
Comment on lines +381 to +383
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if ext/gd is compiled against a libgd which does not support lossless WebP encoding? Furthermore, there is currently no way to detect whether it is supported. So maybe:

Suggested change
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);
#ifdefgdWebpLossless
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", gdWebpLossless, CONST_CS | CONST_PERSISTENT);
#endif

(and ditch the definition of PHP_IMG_WEBP_LOSSLESS)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I get it. Going forward, libgd will require a version of libwebp that's recent enough to support lossless WebP encoding. But your concern is that someone might use an older version of libgd. That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

If I'm getting that right, I think this solution is pretty clever.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

Right.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I see precedent for this in the code:

#ifdefgdEffectMultiplyREGISTER_LONG_CONSTANT("IMG_EFFECT_MULTIPLY", gdEffectMultiply, CONST_CS | CONST_PERSISTENT);
#endif

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! gdEffectMultiply is defined as of libgd 2.1.1, but we're still supporting 2.1.0. Still, something that can be removed sometime in the future.

@cmb69cmb69 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@morsssss

Copy link
Copy Markdown
ContributorAuthor

Thank you!

Thank you for your consistent patience and good cheer!

@cmb69cmb69 closed this in eb6c9ebAug 12, 2021
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Includes unit tests.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
jrfnl added a commit to jrfnl/doc-en that referenced this pull request Mar 11, 2022
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@morsssss@andypost@nikic@cmb69@krakjoe
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Lossless conversion for webp - #7348

Closed
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless
Closed

Lossless conversion for webp#7348
morsssss wants to merge 3 commits into
php:masterfrom
morsssss:webp-lossless

Conversation

@morsssss

Copy link
Copy Markdown
Contributor

I'm propagating lossless conversion from libgd to our bundled gd.

I've also changed "quantization" to "quality", as it is in libgd, making our webp code just a little closer to what's in libgd.

Added test as well.

Note that this change adds a new possible value of 101 for quality, indicating losslessness. This will need to be documented.

/cc @adamsilverstein

Propagating lossless conversion from libgd to our bundled gd.
Changing "quantization" to "quality" as in libgd.
Adding test.
Comment threadext/gd/tests/webp_basic.phpt
@morsssss

Copy link
Copy Markdown
ContributorAuthor

P.S. This is the parallel PR in libgd: libgd/libgd#698

Comment threadext/gd/tests/webp_basic.phpt Outdated

@nikicnikic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @cmb69?

Comment threadext/gd/gd.c
Comment on lines +381 to +383
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if ext/gd is compiled against a libgd which does not support lossless WebP encoding? Furthermore, there is currently no way to detect whether it is supported. So maybe:

Suggested change
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", PHP_IMG_WEBP_LOSSLESS, CONST_CS | CONST_PERSISTENT);
#ifdefgdWebpLossless
/* constant for webp encoding */
REGISTER_LONG_CONSTANT("IMG_WEBP_LOSSLESS", gdWebpLossless, CONST_CS | CONST_PERSISTENT);
#endif

(and ditch the definition of PHP_IMG_WEBP_LOSSLESS)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I get it. Going forward, libgd will require a version of libwebp that's recent enough to support lossless WebP encoding. But your concern is that someone might use an older version of libgd. That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

If I'm getting that right, I think this solution is pretty clever.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That older version wouldn't support losselss WebP... but it also wouldn't have the new gdWebpLossless defined.

Right.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I see precedent for this in the code:

#ifdefgdEffectMultiplyREGISTER_LONG_CONSTANT("IMG_EFFECT_MULTIPLY", gdEffectMultiply, CONST_CS | CONST_PERSISTENT);
#endif

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! gdEffectMultiply is defined as of libgd 2.1.1, but we're still supporting 2.1.0. Still, something that can be removed sometime in the future.

@cmb69cmb69 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@morsssss

Copy link
Copy Markdown
ContributorAuthor

Thank you!

Thank you for your consistent patience and good cheer!

@cmb69cmb69 closed this in eb6c9ebAug 12, 2021
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Includes unit tests.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
jrfnl added a commit to jrfnl/doc-en that referenced this pull request Mar 11, 2022
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
cmb69 pushed a commit to php/doc-en that referenced this pull request Mar 11, 2022
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesGH-1449.
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
* PHP 8.1 | MigrationGuide/New constants: add missing constants [1]
> * Added CURLOPT_DOH_URL option
> * Added certificate blob options when for libcurl >= 7.71.0:
>
> CURLOPT_ISSUERCERT_BLOB
> CURLOPT_PROXY_ISSUERCERT
> CURLOPT_PROXY_ISSUERCERT_BLOB
> CURLOPT_PROXY_SSLCERT_BLOB
> CURLOPT_PROXY_SSLKEY_BLOB
> CURLOPT_SSLCERT_BLOB
> CURLOPT_SSLKEY_BLOB
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L220-L229
* php/php-src#6612
* php/php-src@3dad63b
* php/php-src#7194
* php/php-src@b11785c
* PHP 8.1 | MigrationGuide/New constants: add missing constants [2]
> GD:
> * Avif support is now available through the `imagecreatefromavif()` and
> `imageavif()` functions, if libgd has been built with avif support.
While not mentioned in the changelog entry, the commit to PHP does contain a new constant declaration...
Refs:
* https://github.com/php/php-src/blob/f67986a9218f4889d9352a87c29337a5b6eaa4bd/UPGRADING#L245-L247
* php/php-src#7026
* php/php-src@81f6d36#diff-00d1efef2247b288c86a6c3bfefac111a4774fbc5453fdc02dcf36c4a23da283R373
> GD:
> * `imagewebp()` can do lossless WebP encoding by passing `IMG_WEBP_LOSSLESS` as
> quality. This constant is only defined, if a libgd is used which supports
> lossless WebP encoding.
Refs:
* https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L568-L571
* php/php-src#7348
* php/php-src@eb6c9eb
* PHP 8.1 | MigrationGuide/New constants: add missing constants [3]
> Added `POSIX_RLIMIT_KQUEUES` and `POSIX_RLIMIT_NPTS`. These rlimits are only available on FreeBSD.
Refs:
* https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.posix
* php/php-src#6608
* php/php-src@ebca8de
* PHP 8.1 | MigrationGuide/New constants: add missing constants [4]
Refs:
* https://wiki.php.net/rfc/readonly_properties_v2
* php/php-src#7089
* php/php-src@6780aaa
* PHP 8.1 | MigrationGuide/New constants: add missing constants [5]
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium xchacha* functions, does declare a couple of new constants as well...
Refs:
* php/php-src#6868
* php/php-src@f7f1f7f#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR352-R357
While not mentioned anywhere at all, the commit to PHP itself adding support for the Sodium ristretto255* functions, also declares a number of new constants as well...
Refs:
* php/php-src#6922
* php/php-src@9b794f8#diff-3fe4027560fd299248af1dc1efe04287cc2b6418e8f01755c05c9db64b668b1eR368-R381
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
ClosesphpGH-1449.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@morsssss@andypost@nikic@cmb69@krakjoe