Problem/Motivation

PHP bugfix https://github.com/php/php-src/commit/22c487616f132ee7ebfa838bce9d14c924... has changed the logic to calculate image dimensions for rotated images, causing Drupal's critical #2918570: Drupal\KernelTests\Core\Image\ToolkitGdTest fails on PHP 7.1.x-dev and 7.0.x-dev following a PHP bugfix.

See also:
libgd fix - https://github.com/libgd/libgd/issues/225

Proposed resolution

Fix the Rectangle class to adapt the same calculation logic as GD in libgd 2.2.2 and above. This way images rotated in Drupal will always take the same dimensions, regardless of the libgd library actually linked to the PHP instance in use.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#73 2921123-73.patch221.76 KBravi.shankar
#36 interdiff_32-35.txt6.82 KBmondrake
#36 2921123-35.patch220.8 KBmondrake
#33 2921123-33.patch217.24 KBmondrake
#33 interdiff_32-33.txt4.91 KBmondrake
#32 interdiff_27-32.txt5.21 KBmondrake
#32 2921123-32.patch212.33 KBmondrake
#30 2921123-30.patch2.53 KBmondrake
#27 interdiff_25-27.txt4.54 KBmondrake
#27 2921123-27.patch210.05 KBmondrake
#25 interdiff_21-25.txt5.81 KBmondrake
#25 2921123-25.patch207.92 KBmondrake
#24 2921123-24.patch207.92 KBmondrake
#24 interdiff_21-24.txt5.8 KBmondrake
#21 2921123-21.patch205.27 KBmondrake
#21 interdiff_13-21.txt5.27 KBmondrake
#13 2921123-13.patch204.29 KBmondrake
#13 interdiff_11-13.txt2.2 KBmondrake
#11 2921123-11.patch206.49 KBmondrake
#5 interdiff_2-4.txt8.67 KBmondrake
#5 2921123-4.patch204.27 KBmondrake
#2 2921123-2-test-only.patch195.6 KBmondrake

Issue fork drupal-2921123

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mondrake created an issue. See original summary.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Active » Needs review
StatusFileSize
new 2921123-2-test-only.patch195.6 KB

Let's start by getting a new data sample for RectangleTest.

This was obtained by re-running the calculation routine from http://cgit.drupalcode.org/drupal/tree/core/tests/Drupal/Tests/Component..., reading as input the test sample http://cgit.drupalcode.org/drupal/tree/core/tests/Drupal/Tests/Component... and rewriting the width/height results, on a PHP 7.0.25 compiled with GD library 2.1.1-dev.

The patch is test-only and gives us the target to get to in adjusting the calculation for Rectangle.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 2: 2921123-2-test-only.patch, failed testing. View results

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review
StatusFileSize
new 2921123-4.patch204.27 KB
new interdiff_2-4.txt8.67 KB

Turns out fixing this is easier than expected: just implemented some PHP equivalents of the GD library affine transormation functions seem to do the trick.

This will now fail on ToolkitGdTest and ImageDimensionsTest, because the expected dimensions coded in the tests are still the PHP 5.5 ones.

So I'd postpone this on #2918570: Drupal\KernelTests\Core\Image\ToolkitGdTest fails on PHP 7.1.x-dev and 7.0.x-dev following a PHP bugfix.

Status: Needs review » Needs work

The last submitted patch, 5: 2921123-4.patch, failed testing. View results

mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: Adjust Rectangle class to calculate rotated image dimensions according to PHP bugfix #65148 » [PP-1] Adjust Rectangle class to calculate rotated image dimensions according to PHP bugfix #65148
Status: Needs work » Postponed

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Postponed » Needs review
StatusFileSize
new 2921123-11.patch206.49 KB

Reroll.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: [PP-1] Adjust Rectangle class to calculate rotated image dimensions according to PHP bugfix #65148 » Adjust Rectangle class to calculate rotated image dimensions according to PHP 7+
Parent issue: #2918570: Drupal\KernelTests\Core\Image\ToolkitGdTest fails on PHP 7.1.x-dev and 7.0.x-dev following a PHP bugfix » #3053363: Remove support for PHP 5 in Drupal 8.8

The hotfix in #2918570-11: Drupal\KernelTests\Core\Image\ToolkitGdTest fails on PHP 7.1.x-dev and 7.0.x-dev following a PHP bugfix skipped the image dimensions assertions, and the code is still commented out, making the patch of this issue actually pass. So this issue is independent from the other and can be unpostponed.

Reparenting to #3053363: Remove support for PHP 5 in Drupal 8.8.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes
StatusFileSize
new interdiff_11-13.txt2.2 KB
new 2921123-13.patch204.29 KB

We do not need to touch the Rotate GD image toolkit operation here.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@mondrake do we need to provide a rectangle class anymore? There are no usages in core. Can't we just deprecate it and be done.

mondrake’s picture

mondrake
Location 🇮🇹
commented

@alexpott it is used in core in https://git.drupalcode.org/project/drupal/-/blob/9.2.x/core/modules/imag..., and the ImageMagick module relies on it.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@mondrake ah yeah you're right I though #13 was removing it due to the interdiff but I got RotateImageEffect and Rotate confused.

That's a shame because the change here is not being done in BC compatible manor. The protected methods on \Drupal\Component\Utility\Rectangle should have been private. This issue can't remove them as it is doing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Needs work

This issue needs to prove it works by removing the version compare in 

    // Systems using non-bundled GD2 don't have imagerotate. Test if available.
    // @todo Remove the version check once
    //   https://www.drupal.org/project/drupal/issues/2670966 is resolved.
    if (function_exists('imagerotate') && (version_compare(phpversion(), '7.0.26') < 0)) {

\Drupal\KernelTests\Core\Image\ToolkitGdTest::testManipulations

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff_13-21.txt5.27 KB
new 2921123-21.patch205.27 KB

Let's see this... the no longer used protected methods in Rectangle will have to be deprecated properly, then,

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

#21 looks better from a BC pov - and yeah we need to deprecate those old methods.

+++ b/core/lib/Drupal/Component/Utility/Rectangle.php
@@ -81,46 +81,110 @@ public function __construct($width, $height) {
+  private function gdAffineRotate($angle) {
...
+  private function gdAffineApplyToPointF(array $src, array $affine) {
...
+  private function gdTransformAffineBoundingBox($width, $height, $affine) {

Can now use PHP 7.3 typehints and return types. And +1 for private here. There should be no protected API for anything that wants to extend Rectangle.

Status: Needs review » Needs work

The last submitted patch, 21: 2921123-21.patch, failed testing. View results

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review
Issue tags: +Needs change record
StatusFileSize
new interdiff_21-24.txt5.8 KB
new 2921123-24.patch207.92 KB

Addressed #22 and added deprecation, left for todo the CR. Are deprecation test needed here given that there's no way now to get the methods being executed from public? Will need reflection.

mondrake’s picture

mondrake
Location 🇮🇹
commented
StatusFileSize
new 2921123-25.patch207.92 KB
new interdiff_21-25.txt5.81 KB

Status: Needs review » Needs work

The last submitted patch, 25: 2921123-25.patch, failed testing. View results

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review
StatusFileSize
new 2921123-27.patch210.05 KB
new interdiff_25-27.txt4.54 KB

This should pass on PHP 7.4. However, it looks like GD has changed *again* the algo for rotation leading to different resulting dimensions again. So #2, that was obtained with PHP 7.0.25, should be resampled again with 7.3+.

tanubansal’s picture

tanubansal commented

@mondrake : Please suggest if this can be tested on 9.2. Do we need to have PHP 7.4 configured?

mondrake’s picture

mondrake
Location 🇮🇹
commented

#27 so the problem here is with the libgd linked into the PHP executable, not with PHP itself.

GD rotate functions were overhauled in libgd 2.2.2, https://github.com/libgd/libgd/releases/tag/gd-2.2.2

The new algo in Rectangle matches that logic of calculation. I just re-extracted the sample data with PHP 7.3.19 and libgd 2.2.5 and it matches exactly with the one in the patch.

However, if PHP links lower versions of libgd (it's the case on TravisCI for instance, that is 2.1.0 compatible; I'll post a test patch here for checking DrupalCI), gd images will be rotated according to old logic. This leads to the weirdness below

+++ b/core/modules/image/tests/src/Functional/ImageDimensionsTest.php
@@ -221,18 +221,14 @@ public function testImageDimensions() {
     $style->save();
-    // @todo Uncomment this once
-    //   https://www.drupal.org/project/drupal/issues/2670966 is resolved.
-    // $this->assertEqual($this->getImageTag($variables), '<img src="' . $url . '" width="41" height="41" alt="" class="image-style-test" />');
+    $this->assertEqual($this->getImageTag($variables), '<img src="' . $url . '" width="43" height="43" alt="" loading="lazy" class="image-style-test" />');
     $this->assertFileNotExists($generated_uri);
     $this->drupalGet($this->getAbsoluteUrl($url));
     $this->assertSession()->statusCodeEquals(200);
     $this->assertFileExists($generated_uri);
     $image_file = $image_factory->get($generated_uri);
-    // @todo Uncomment this once
-    //   https://www.drupal.org/project/drupal/issues/2670966 is resolved.
-    // $this->assertEqual($image_file->getWidth(), 41);
-    // $this->assertEqual($image_file->getHeight(), 41);
+    $this->assertEqual($image_file->getWidth(), 40);
+    $this->assertEqual($image_file->getHeight(), 41);

i.e. that transformDimensions calculates a different set of dimensions than the one actually returned by the rotate operation.

IMHO we need to go back to the concept in #2918570: Drupal\KernelTests\Core\Image\ToolkitGdTest fails on PHP 7.1.x-dev and 7.0.x-dev following a PHP bugfix and adjust the gd rotate plugin to adjust the image to the dimensions that we calculate in Rectangle - so that we have one version of 'our' truth across the board and do not have to play catch up.

mondrake’s picture

mondrake
Location 🇮🇹
commented
StatusFileSize
new 2921123-30.patch2.53 KB

A patch to sort out what libgd versions are in use in the DrupalCI containers.

Status: Needs review » Needs work

The last submitted patch, 30: 2921123-30.patch, failed testing. View results

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review
StatusFileSize
new 2921123-32.patch212.33 KB
new interdiff_27-32.txt5.21 KB

So DrupalCI are on libgd 2.1.0, too.

Adding a test that checks the validity of the sample data against the actual GD calculation, that is only applicable if libgd is 2.2.2+. It would be good to have a DrupalCI container with a PHP with that version.

mondrake’s picture

mondrake
Location 🇮🇹
commented
StatusFileSize
new interdiff_32-33.txt4.91 KB
new 2921123-33.patch217.24 KB

Adding hunks from #2918570: Drupal\KernelTests\Core\Image\ToolkitGdTest fails on PHP 7.1.x-dev and 7.0.x-dev following a PHP bugfix to make the GDToolkit rotate operation abide to the dimensions calculated by Rectangle.

The last submitted patch, 32: 2921123-32.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 33: 2921123-33.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review
StatusFileSize
new 2921123-35.patch220.8 KB
new interdiff_32-35.txt6.82 KB

Fixing failure and improving the GD toolkit test.

mondrake’s picture

mondrake
Location 🇮🇹
commented 
+++ b/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Rotate.php
@@ -93,10 +89,21 @@ protected function execute(array $arguments) {
+    // In Drupal we rotate anti-clockwise whereas GD rotates clockwise. We need
+    // to reconcile the value in Drupal with the argument to be passed to
+    // rotate.

Note to self: it's actually the other way round, https://www.php.net/manual/en/function.imagerotate.php

angle
Rotation angle, in degrees. The rotation angle is interpreted as the number of degrees to rotate the image anticlockwise.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: Adjust Rectangle class to calculate rotated image dimensions according to PHP 7+ » Adjust Rectangle class to calculate rotated image dimensions according to libgd 2.2.2+
Issue summary: View changes
mondrake’s picture

mondrake
Location 🇮🇹
commented

Change to MR workflow.

mondrake opened merge request !267

mondrake’s picture

mondrake
Location 🇮🇹
commented

Rerolled

mondrake’s picture

mondrake
Location 🇮🇹
commented

#37 still open

mondrake’s picture

mondrake
Location 🇮🇹
commented

Rerolled and addressed #37.

daffie’s picture

daffie commented
Status: Needs review » Needs work
  1. @mondrake: For the code changes in lib/Drupal/Component/Utility/Rectangle.php and modules/system/src/Plugin/ImageToolkit/Operation/gd/Rotate.php. Where do all these code changes come from? Did you create this code yourself or did you copied it from somewhere?
  2. Is this really something we want to do in core? This is not what Drupal is about. Can we not just use the code from libgd and live with the fact that something changed with rotating images in version 2.2.2?
mondrake’s picture

mondrake
Location 🇮🇹
commented
Related issues: +#2918570: Drupal\KernelTests\Core\Image\ToolkitGdTest fails on PHP 7.1.x-dev and 7.0.x-dev following a PHP bugfix, +#1551686: Image rotate dimensions callback can calculate new dimensions for every angle

@daffie I am maintaining and, in part, developed the Image Effects module. Some of the testing code and the image overlying code in the Rotate operation in case of images not fitting the expected dimensions is from there. The changes in the Rectangle class are the PHP equivalent of C++ code from libgd, see the @see annotations in the code.

#44.2 in ImageStyle::transformDimensions we calculate image dimensions 'in abstract' i.e. without a real image being processed, to speed up the determination of the width and height attributes of <img> tags. This calculation is done in the Rectangle class, https://www.drupal.org/node/2645822. When the Rectangle class and the 'real' gd library on a site produce different results, we end up with the problem here. #2918570: Drupal\KernelTests\Core\Image\ToolkitGdTest fails on PHP 7.1.x-dev and 7.0.x-dev following a PHP bugfix happened to be critical at a point in time because a change of PHP version started failing all tests on DrupalCI.

edit - right now in HEAD the Rectangle class calculates dimensions as PHP was doing in PHP 5.5. This is way outdated and ALL <img> with rotations have tags with height/width attributes different than real... with tests that were catching that now disabled. #2918570-42: Drupal\KernelTests\Core\Image\ToolkitGdTest fails on PHP 7.1.x-dev and 7.0.x-dev following a PHP bugfix.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review
mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue tags: -Needs change record
daffie’s picture

daffie commented
Status: Needs review » Needs work

Back to needs work for the unresolved threads.

mondrake’s picture

mondrake
Location 🇮🇹
commented
daffie’s picture

daffie commented

So if I understand it correctly, we can update the class Drupal\Component\Utility\Rectangle, when every PHP version the Drupal core supports has the libgd with version 2.2.2 or higher. Could we add @todo in the patch for that and maybe create a followup issue for it.

mondrake’s picture

mondrake
Location 🇮🇹
commented

@daffie re #50, no actually that is the real point: we do not want to make vodoo dances every time the logic in PHP or GD changes. So we set 'our own' version of the rotation calculation algo (incidentally, with this patch, equal to the algo used in libgd 2.2.2), and (a) use that in transformDimensions, and (b) adjust the result of a real rotation in the Rotate gd image toolkit operation to comply to that if the dimensions are different from our calculation. So that we only change the Rectangle class when we want, and any Drupal version in any PHP version with any libgd version will behave the same way.

BTW. DrupalCI on PHP 7.3 and 7.4 still has an old version of libgd (2.1.0), see #30. This results in testGdRotate being actually skipped, even if it is reported successful.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review
daffie’s picture

daffie commented
Status: Needs review » Reviewed & tested by the community

All code changes look good to me.
The deprecation messages have no testing, because the methods are all protected methods.
There is testing that the implementation of class Drupal\Component\Utility\Rectangle has the same result as the implementation of libgd version 2.2.2 or higher.
There is already testing in core for the rotation of images.
For me it is RTBC.

mondrake’s picture

mondrake
Location 🇮🇹
commented

Thank you for your reviews, @daffie @andypost and @alexpott, really appreciated.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Is there a way to run PHP 8 CI on the MR? It could have fresher libgd

mondrake’s picture

mondrake
Location 🇮🇹
commented

No it still has 2.1.0, see #30

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

I think this issue is blocked by not having any test coverage on DrupalCI because there's no bot with a high enough libgd.

If I run the core/tests/Drupal/Tests/Component/Utility/RectangleTest.php locally I get 3484 fails!

3484) Drupal\Tests\Component\Utility\RectangleTest::testGdRotate with data set #4220 (553, 8, 300, 285, 484)
Failed asserting that 284 is identical to 285.

/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:118
/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/Constraint/IsIdentical.php:93
/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/Assert.php:2887
/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/Assert.php:1807
/Users/alex/dev/sites/drupal8alt.dev/core/tests/Drupal/Tests/Component/Utility/RectangleTest.php:91
/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/TestResult.php:691
/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/TestSuite.php:601
/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/TestSuite.php:601
/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:633
/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/TextUI/Command.php:204
/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/TextUI/Command.php:163

^^ is the last one.

Here's the output of my gd_info()...

Psy Shell v0.10.6 (PHP 8.0.0 — cli) by Justin Hileman
>>> gd_info()
=> [
     "GD Version" => "2.3.0",
     "FreeType Support" => true,
     "FreeType Linkage" => "with freetype",
     "GIF Read Support" => true,
     "GIF Create Support" => true,
     "JPEG Support" => true,
     "PNG Support" => true,
     "WBMP Support" => true,
     "XPM Support" => true,
     "XBM Support" => true,
     "WebP Support" => true,
     "BMP Support" => true,
     "TGA Read Support" => true,
     "JIS-mapped Japanese Font Support" => false,
   ]

Has the calculation changed again in 2.3.0? If we commit the patch as is it feels like something that will cause fails in the future when the PHP builds are updated.

mondrake’s picture

mondrake
Location 🇮🇹
commented

It's a never ending story :(... I will check again with 2.2.5.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@mondrake I keep on wondering if trying to normalise this is really core's job...

mondrake’s picture

mondrake
Location 🇮🇹
commented

if not, then what? Revert #1551686: Image rotate dimensions callback can calculate new dimensions for every angle?

mondrake’s picture

mondrake
Location 🇮🇹
commented

Works for me...

$ sudo -u www-data ../vendor/phpunit/phpunit/phpunit -v tests/Drupal/Tests/Component/Utility/RectangleTest.php 
PHPUnit 9.5.2 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.3.19-1~deb10u1
Configuration: /var/www/d91/core/phpunit.xml.dist
Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

Testing Drupal\Tests\Component\Utility\RectangleTest
.............................................................   61 / 8444 (  0%)
.............................................................  122 / 8444 (  1%)
.............................................................  183 / 8444 (  2%)
.............................................................  244 / 8444 (  2%)
.............................................................  305 / 8444 (  3%)
.............................................................  366 / 8444 (  4%)
.............................................................  427 / 8444 (  5%)
.............................................................  488 / 8444 (  5%)
.............................................................  549 / 8444 (  6%)
.............................................................  610 / 8444 (  7%)
.............................................................  671 / 8444 (  7%)
.............................................................  732 / 8444 (  8%)
.............................................................  793 / 8444 (  9%)
.............................................................  854 / 8444 ( 10%)
.............................................................  915 / 8444 ( 10%)
.............................................................  976 / 8444 ( 11%)
............................................................. 1037 / 8444 ( 12%)
............................................................. 1098 / 8444 ( 13%)
............................................................. 1159 / 8444 ( 13%)
............................................................. 1220 / 8444 ( 14%)
............................................................. 1281 / 8444 ( 15%)
............................................................. 1342 / 8444 ( 15%)
............................................................. 1403 / 8444 ( 16%)
............................................................. 1464 / 8444 ( 17%)
............................................................. 1525 / 8444 ( 18%)
............................................................. 1586 / 8444 ( 18%)
............................................................. 1647 / 8444 ( 19%)
............................................................. 1708 / 8444 ( 20%)
............................................................. 1769 / 8444 ( 20%)
............................................................. 1830 / 8444 ( 21%)
............................................................. 1891 / 8444 ( 22%)
............................................................. 1952 / 8444 ( 23%)
............................................................. 2013 / 8444 ( 23%)
............................................................. 2074 / 8444 ( 24%)
............................................................. 2135 / 8444 ( 25%)
............................................................. 2196 / 8444 ( 26%)
............................................................. 2257 / 8444 ( 26%)
............................................................. 2318 / 8444 ( 27%)
............................................................. 2379 / 8444 ( 28%)
............................................................. 2440 / 8444 ( 28%)
............................................................. 2501 / 8444 ( 29%)
............................................................. 2562 / 8444 ( 30%)
............................................................. 2623 / 8444 ( 31%)
............................................................. 2684 / 8444 ( 31%)
............................................................. 2745 / 8444 ( 32%)
............................................................. 2806 / 8444 ( 33%)
............................................................. 2867 / 8444 ( 33%)
............................................................. 2928 / 8444 ( 34%)
............................................................. 2989 / 8444 ( 35%)
............................................................. 3050 / 8444 ( 36%)
............................................................. 3111 / 8444 ( 36%)
............................................................. 3172 / 8444 ( 37%)
............................................................. 3233 / 8444 ( 38%)
............................................................. 3294 / 8444 ( 39%)
............................................................. 3355 / 8444 ( 39%)
............................................................. 3416 / 8444 ( 40%)
............................................................. 3477 / 8444 ( 41%)
............................................................. 3538 / 8444 ( 41%)
............................................................. 3599 / 8444 ( 42%)
............................................................. 3660 / 8444 ( 43%)
............................................................. 3721 / 8444 ( 44%)
............................................................. 3782 / 8444 ( 44%)
............................................................. 3843 / 8444 ( 45%)
............................................................. 3904 / 8444 ( 46%)
............................................................. 3965 / 8444 ( 46%)
............................................................. 4026 / 8444 ( 47%)
............................................................. 4087 / 8444 ( 48%)
............................................................. 4148 / 8444 ( 49%)
............................................................. 4209 / 8444 ( 49%)
............................................................. 4270 / 8444 ( 50%)
............................................................. 4331 / 8444 ( 51%)
............................................................. 4392 / 8444 ( 52%)
............................................................. 4453 / 8444 ( 52%)
............................................................. 4514 / 8444 ( 53%)
............................................................. 4575 / 8444 ( 54%)
............................................................. 4636 / 8444 ( 54%)
............................................................. 4697 / 8444 ( 55%)
............................................................. 4758 / 8444 ( 56%)
............................................................. 4819 / 8444 ( 57%)
............................................................. 4880 / 8444 ( 57%)
............................................................. 4941 / 8444 ( 58%)
............................................................. 5002 / 8444 ( 59%)
............................................................. 5063 / 8444 ( 59%)
............................................................. 5124 / 8444 ( 60%)
............................................................. 5185 / 8444 ( 61%)
............................................................. 5246 / 8444 ( 62%)
............................................................. 5307 / 8444 ( 62%)
............................................................. 5368 / 8444 ( 63%)
............................................................. 5429 / 8444 ( 64%)
............................................................. 5490 / 8444 ( 65%)
............................................................. 5551 / 8444 ( 65%)
............................................................. 5612 / 8444 ( 66%)
............................................................. 5673 / 8444 ( 67%)
............................................................. 5734 / 8444 ( 67%)
............................................................. 5795 / 8444 ( 68%)
............................................................. 5856 / 8444 ( 69%)
............................................................. 5917 / 8444 ( 70%)
............................................................. 5978 / 8444 ( 70%)
............................................................. 6039 / 8444 ( 71%)
............................................................. 6100 / 8444 ( 72%)
............................................................. 6161 / 8444 ( 72%)
............................................................. 6222 / 8444 ( 73%)
............................................................. 6283 / 8444 ( 74%)
............................................................. 6344 / 8444 ( 75%)
............................................................. 6405 / 8444 ( 75%)
............................................................. 6466 / 8444 ( 76%)
............................................................. 6527 / 8444 ( 77%)
............................................................. 6588 / 8444 ( 78%)
............................................................. 6649 / 8444 ( 78%)
............................................................. 6710 / 8444 ( 79%)
............................................................. 6771 / 8444 ( 80%)
............................................................. 6832 / 8444 ( 80%)
............................................................. 6893 / 8444 ( 81%)
............................................................. 6954 / 8444 ( 82%)
............................................................. 7015 / 8444 ( 83%)
............................................................. 7076 / 8444 ( 83%)
............................................................. 7137 / 8444 ( 84%)
............................................................. 7198 / 8444 ( 85%)
............................................................. 7259 / 8444 ( 85%)
............................................................. 7320 / 8444 ( 86%)
............................................................. 7381 / 8444 ( 87%)
............................................................. 7442 / 8444 ( 88%)
............................................................. 7503 / 8444 ( 88%)
............................................................. 7564 / 8444 ( 89%)
............................................................. 7625 / 8444 ( 90%)
............................................................. 7686 / 8444 ( 91%)
............................................................. 7747 / 8444 ( 91%)
............................................................. 7808 / 8444 ( 92%)
............................................................. 7869 / 8444 ( 93%)
............................................................. 7930 / 8444 ( 93%)
............................................................. 7991 / 8444 ( 94%)
............................................................. 8052 / 8444 ( 95%)
............................................................. 8113 / 8444 ( 96%)
............................................................. 8174 / 8444 ( 96%)
............................................................. 8235 / 8444 ( 97%)
............................................................. 8296 / 8444 ( 98%)
............................................................. 8357 / 8444 ( 98%)
............................................................. 8418 / 8444 ( 99%)
..........................                                    8444 / 8444 (100%)

Time: 03:18.267, Memory: 26.00 MB

OK (8444 tests, 16886 assertions)

on 

sudo ../vendor/bin/drush rq
+--------------------------------+----------+--------------------------------------------------------------------------------+
| Title                          | Severity | Summary                                                                        |
+--------------------------------+----------+--------------------------------------------------------------------------------+
| GD library PNG support         | Info     | 2.2.5                                                                          |
|                                |          |                                                                                |
| Configuration files            | Info     | Protected                                                                      |
|                                |          |                                                                                |
| Cron maintenance tasks         | Info     | Last run 2 hours 36 minutes ago                                                |
|                                |          |                                                                                |
| Database system                | Info     | Doctrine DBAL on sqlite/3.27.2 via pdo_sqlite                                  |
|                                |          |                                                                                |
| Database system version        | Info     | 3.0.0                                                                          |
|                                |          |                                                                                |
| Drupal                         | Info     | 9.2.0-dev                                                                      |
|                                |          |                                                                                |
| Entity/field definitions       | Info     | Up to date                                                                     |
|                                |          |                                                                                |
| Experimental themes enabled    | Warning  | Experimental themes found: /Claro, Olivero/. Experimental themes are provided  |
|                                |          | for testing purposes only. Use at your own risk.                               |
|                                |          |                                                                                |
| File system                    | Info     | Writable (/public/ download method)                                            |
|                                |          |                                                                                |
| Upload progress                | Info     | Not enabled                                                                    |
|                                |          |                                                                                |
| Image toolkit                  | Info     | gd                                                                             |
|                                |          |                                                                                |
| GD library                     | Info     | 2.2.5                                                                          |
|                                |          |                                                                                |
| Limited date range             | Warning  | Your PHP installation has a limited date range.                                |
|                                |          |                                                                                |
| MIME type guessing             | Info     | Sophron                                                                        |
|                                |          |                                                                                |
| Node Access Permissions        | Info     | Disabled                                                                       |
|                                |          |                                                                                |
| PHP                            | Info     | 7.3.19-1~deb10u1 (more information [1])                                        |
|                                |          |                                                                                |
|                                |          | [1] http://default/admin/reports/status/php                                    |
|                                |          |                                                                                |
| PHP extensions                 | Info     | Enabled                                                                        |
|                                |          |                                                                                |
| PHP memory limit               | Info     | -1 (Unlimited)                                                                 |
|                                |          |                                                                                |
| PHP OPcode caching             | Info     | Enabled                                                                        |
|                                |          |                                                                                |
| Random number generation       | Info     | Successful                                                                     |
|                                |          |                                                                                |
| Search index progress          | Info     | 75% (1 remaining)                                                              |
|                                |          |                                                                                |
| Trusted Host Settings          | Error    | Not enabled                                                                    |
|                                |          |                                                                                |
| Database updates               | Info     | Up to date                                                                     |
|                                |          |                                                                                |
| Access to update.php           | Info     | Protected                                                                      |
|                                |          |                                                                                |
| Update notifications           | Info     | Enabled                                                                        |
|                                |          |                                                                                |
| Module and theme update status | Info     | Up to date [1]                                                                 |
|                                |          |                                                                                |
|                                |          | [1] http://default/admin/reports/updates                                       |
|                                |          |                                                                                |
| Drupal core update status      | Warning  | Unknown release date (version 9.1.4 available) [1]                             |
|                                |          |                                                                                |
|                                |          | [1] http://default/admin/reports/updates                                       |
|                                |          |                                                                                |
| Web server                     | Info     |                                                                                |
+--------------------------------+----------+--------------------------------------------------------------------------------+
daffie’s picture

daffie commented

@mondrake: Does it also pass with libgd 2.3.0 or higher. When @alexpott tested it with that version it failed.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review

I tested D9.2 on PHP 7.3, 7.4 and 8.0 with libgd 2.3.0 on Github Actions and I have full passes in all cases, https://github.com/mondrake/d8-unit/actions/runs/569429199

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Needs work

needs re-roll after #3193955: Swap assertEqual arguments in preparation to replace with assertEquals

used to run test on Ubuntu 20.10

$ php -v
PHP 7.4.9 (cli) (built: Oct 26 2020 15:17:14) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.9, Copyright (c), by Zend Technologies
    with Xdebug v2.9.6, Copyright (c) 2002-2020, by Derick Rethans

$ php --ri gd

gd

GD Support => enabled
GD headers Version => 2.3.0
GD library Version => 2.3.0
FreeType Support => enabled
FreeType Linkage => with freetype
GIF Read Support => enabled
GIF Create Support => enabled
JPEG Support => enabled
PNG Support => enabled
WBMP Support => enabled
XPM Support => enabled
XBM Support => enabled
WebP Support => enabled
BMP Support => enabled
TGA Read Support => enabled

Directive => Local Value => Master Value
gd.jpeg_ignore_warning => 1 => 1

The final result of SIMPLETEST_DB=sqlite://./db/tests.db SIMPLETEST_BASE_URL=http://nginx BROWSERTEST_OUTPUT_DIRECTORY=db vendor/bin/phpunit -c core/phpunit.xml.dist --colors=always -v core/tests/Drupal/Tests/Component/Utility/RectangleTest.php is

FAILURES!
Tests: 8444, Assertions: 13402, Failures: 3484.

Results are the same as #57 (last 3 cases)

3482) Drupal\Tests\Component\Utility\RectangleTest::testGdRotate with data set #4217 (439, 430, 210, 597, 593)
Failed asserting that 596 is identical to 597.

...
3483) Drupal\Tests\Component\Utility\RectangleTest::testGdRotate with data set #4219 (112, 474, -300, 468, 335)
Failed asserting that 467 is identical to 468.

...
3484) Drupal\Tests\Component\Utility\RectangleTest::testGdRotate with data set #4220 (553, 8, 300, 285, 484)
Failed asserting that 284 is identical to 285.
andypost’s picture

andypost
he/him
Primary language Russian
commented

Moreover

- the bundled (shipped with PHP) version is 2.0.35 https://github.com/php/php-src/blob/master/ext/gd/libgd/gd.h#L14-L18
- last time I used to run php tests using --with-external-gd option it has a lot of failures, will check again tonight
- 2.3.0 is security release and has a lot of changes https://github.com/libgd/libgd/blob/gd-2.3.1/CHANGELOG.md

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review

#64 I do not understand what reroll is needed, can you please clarify.

It would be great if someone having the failure with 2.3.0 could test with 2.2.2. Git blamed the code and looked at the changelog, no apprent changes in the logic for rotate after 2.2.2. Maybe something else influences results, no clue.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Not sure about re-roll because I got failures with $ curl https://git.drupalcode.org/project/drupal/-/merge_requests/267.patch |patch -p1

but using diff (curl https://git.drupalcode.org/project/drupal/-/merge_requests/267.diff |patch -p1) it applies cleanly

@mondrake as your Github tests shows it requires to rebuild PHP itself with externally linked libgd which is very tricky

EDIT using external GD shows 21 failed test in Alpinelinux (latest 2.3.1 version) https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/18321#note...

One of failed tests is Bug #65148 (imagerotate may alter image dimensions) [ext/gd/tests/bug65148.phpt]

andypost’s picture

andypost
he/him
Primary language Russian
commented

The failure caused by https://bugs.php.net/bug.php?id=65148 still depends on how PHP is build

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Needs work

The MR needs rebase to 10.x or 9.5.x at least

ravi.shankar’s picture

ravi.shankar commented
StatusFileSize
new 2921123-73.patch221.76 KB

Added a patch for Drupal 9.5.x.

mondrake closed merge request !267

mondrake’s picture

mondrake
Location 🇮🇹
commented
Related issues: +#3240390: [policy, no patch] Move image rotation code from core into contrib

Note #3240390: [policy, no patch] Move image rotation code from core into contrib.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mstrelan’s picture

mstrelan commented
Status: Needs work » Postponed (maintainer needs more info)

I came here from #2829040: [meta] Known intermittent, random, and environment-specific test failures and I'm struggling to see where this is up to. There is discussion on whether we should do this at all, then there is an MR, but then the MR was closed and a related issue was linked. The related issue is now closed (won't fix) so where does that leave this issue? Setting to PMNMI so we can get clarification on what's required.

mondrake changed the visibility of the branch 2921123-adjust-rectangle-class to active.

mondrake changed the visibility of the branch 2921123-adjust-rectangle-class to hidden.

mondrake’s picture

mondrake
Location 🇮🇹
commented

Yeah this is messy now, probably better close this and start with a new issue aiming at fixing the test failure (if we want to, this is probably a rather edge case).

The original intent here was to refresh the Rectangle class with an algo that is more aligned with PHP 8. The calculations done currently are mirroring PHP 5 - this means that TransformDimensions consistently produce results that differ from the actual results of processing images via GD.

The main problem is that it’s difficult to target a ‘standard’ algo because PHP relies on the GD library to perform the actual processing, different versions return different results, and you cannot predict which version of GD is used by the PHP instance used for testing.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Postponed (maintainer needs more info) » Needs review

NR for #81

andypost’s picture

andypost
he/him
Primary language Russian
commented

IIRC this change was done in PHP 7.1 so it's safe enough to use new approach, moreover there's not a lot of distros which shipping old GD library.
Moreover the most of PHP builds using bundled GD library

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Closed (won't fix)

So let's close this for now, also based on feedback in https://drupal.slack.com/archives/C079NQPQUEN/p1742553484074889

mondrake’s picture

mondrake
Location 🇮🇹
commented

Filed #3514699: [consistent test failure] ImageDimensionsTest::testImageDimensions() as a replacement of this one.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Related in terms of PHP 8.5 #3540531: Fix ToolkitGdTest for PHP 8.5