Fix distorted previews when using imaginary #35105
Conversation
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
st3iny
added
bug
3. to review
| if ($response->getHeader('X-Image-Width') && $response->getHeader('X-Image-Height')) { | ||
| $maxX = (int)$response->getHeader('X-Image-Width'); | ||
| $maxY = (int)$response->getHeader('X-Image-Height'); |
There was a problem hiding this comment.
Lets wait for @CarlSchwan to chwck if it is okay to not check for this header anymore. It might be possibke that we brake some instances otherwise
There was a problem hiding this comment.
This will break one instance using my fork of imaginary. I'll communicate this to them after my vacation
There was a problem hiding this comment.
Alternative we need to convince the imaginary maintainer that using a X- prefix for custom headers is more correct.
There was a problem hiding this comment.
I thought so. Thanks Carl! :)
st3iny
added
4. to release
|
/backport to stable25 |
|
/backport to stable24 |
|
The backport to stable25 failed. Please do this backport manually. |
|
The backport to stable24 failed. Please do this backport manually. |
|
/backport to stable24 |
|
/backport to stable25 |
|
The backport to stable24 failed. Please do this backport manually. |
|
The backport to stable25 failed. Please do this backport manually. |
|
/backport to stable25 |
|
/backport to stable24 |
|
The backport to stable25 failed. Please do this backport manually. |
|
/backport to stable25 |
Ref nextcloud/previewgenerator#323 Ref nextcloud/server#35105 Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
st3iny
removed
the
pending documentation
Ref nextcloud/previewgenerator#323 Ref nextcloud/server#35105 Co-authored-by: Simon L. <szaimen@e.mail.de> Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
Ref nextcloud/previewgenerator#323 Ref nextcloud/server#35105 Co-authored-by: Simon L. <szaimen@e.mail.de> Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
Ref nextcloud/previewgenerator#323 Ref nextcloud/server#35105 Co-authored-by: Simon L. <szaimen@e.mail.de> Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
Fix nextcloud/previewgenerator#323
Imaginary upstream has introduced the possibility to send width and height of the outputted image along with the request. However, the decision was made that this feature is optional and gated behind a cli parameter (
-return-size).Ref h2non/imaginary#382
Ref https://github.com/h2non/imaginary/pull/382/files#diff-77ed712c0c6f7b52686415504853bc522f5f0ee3e9e0399cfd60a48813aa6aaeR110
Problem
Our code relies on the presence of those headers. If they aren't present, the max values are used as default and thus previews might be distorted. Especially smaller images are affected.
Reproducability
Use both images from the linked issue and generate previews. You'll see that they are always distorted.
With my patch applied they are correct. It doesn't matter if the flag of Imaginary is enabled or not. The performance is slightly better if the flag is enabled.
Documentation
I plan to add a hint to the server tuning docs. We should recommend using the flag by default to admins.
Ref https://docs.nextcloud.com/server/latest/admin_manual/installation/server_tuning.html#previews