craftcms / cms

Build bespoke content experiences with Craft.
https://craftcms.com
Other
3.28k stars 635 forks source link

Bug with image transforms when the source image is the same width as the requested transform #5772

Closed weotch closed 4 years ago

weotch commented 4 years ago

Description

I'm getting an Undefined variable: newWidth when a source image is the same width as the transform I'm requesting.

Steps to reproduce

  1. Upload this image to assets (I generated this from https://placehold.co/) 1440x1100
  2. Get the asset id
  3. Use that asset id in place of #### in the GraphQL explorer:

    query {
    asset(id:####) {
      w1440: url @transform(width: 1440, immediately: true)
    }
    }
  4. The response (for me at least) should be:
{
  "errors": [
    {
      "debugMessage": "Undefined variable: newWidth",
      "message": "Internal server error",
      "category": "internal",
      "locations": [
        {
          "line": 3,
          "column": 5
        }
      ],
      "path": [
        "asset",
        "w1440"
      ]
    }
  ],
  "data": {
    "asset": {
      "w1440": null
    }
  }
}

Additional info

brandonkelly commented 4 years ago

Please search for that error in storage/logs/ and post the stack trace that follows it.

weotch commented 4 years ago

I ran grep -r "newWidth" storage/logs and I'm not getting any results. Am I approaching this the wrong way?

andris-sevcenko commented 4 years ago

@weotch I think the GraphQL lib doesn't really log it. I'll see if I can reproduce this in a bit.

weotch commented 4 years ago

By the way, besides showing that error in the response and failing to make the image, it also slows down my graphql query from <1s to >30s.

andris-sevcenko commented 4 years ago

By the way, besides showing that error in the response and failing to make the image, it also slows down my graphql query from <1s to >30s.

Yeah, transform eager-loading via GraphQL is coming in 3.5.

In 3.4 you can use the withTransforms argument for Assets, but, unfortunately, it only supports named transforms at the time.

totov commented 4 years ago

I'm getting this bug too (the dimensions of the image that my client has uploaded are the same as the transform and it results in Undefined variable: newWidth), here's my stack trace:

2020-04-02 12:13:52 [-][-][3nnn3dfnoqejmvvk27mm6v4ksb][error][yii\base\ErrorException:8] yii\base\ErrorException: Undefined variable: newWidth in /vendor/craftcms/cms/src/image/Raster.php:299
Stack trace:
#0 /vendor/craftcms/cms/src/web/ErrorHandler.php(74): yii\base\ErrorHandler->handleError()
#1 /vendor/craftcms/cms/src/image/Raster.php(299): craft\web\ErrorHandler->handleError()
#2 /vendor/craftcms/cms/src/services/AssetTransforms.php(1436): craft\image\Raster->scaleAndCrop()
#3 /vendor/craftcms/cms/src/services/AssetTransforms.php(701): craft\services\AssetTransforms->_createTransformForAsset()
#4 /vendor/craftcms/cms/src/services/AssetTransforms.php(604): craft\services\AssetTransforms->_generateTransform()
#5 /vendor/craftcms/cms/src/controllers/AssetsController.php(1148): craft\services\AssetTransforms->ensureTransformUrlByIndexModel()
#6 [internal function]: craft\controllers\AssetsController->actionGenerateTransform()
#7 /vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array()
#8 /vendor/yiisoft/yii2/base/Controller.php(157): yii\base\InlineAction->runWithParams()
#9 /vendor/craftcms/cms/src/web/Controller.php(178): yii\base\Controller->runAction()
#10 /vendor/yiisoft/yii2/base/Module.php(528): craft\web\Controller->runAction()
#11 /vendor/craftcms/cms/src/web/Application.php(291): yii\base\Module->runAction()
#12 /vendor/craftcms/cms/src/web/Application.php(559): craft\web\Application->runAction()
#13 /vendor/craftcms/cms/src/web/Application.php(270): craft\web\Application->_processActionRequest()
#14 /vendor/yiisoft/yii2/base/Application.php(386): craft\web\Application->handleRequest()
#15 /public_html/index.php(21): yii\base\Application->run()
#16 {main}

Also, important to note that I have upscaling images set to false ('upscaleImages' => false), and when I comment this out in my config the transform works fine.

brandonkelly commented 4 years ago

This has been fixed for the next release (3120036f0bfee2ff6a4cb2135045ed7be75e4a44).

brandonkelly commented 4 years ago

…and that’s out now (3.4.13).