Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for thumbnail creation on File upload OC version 81.beta #16355

Closed
wants to merge 1 commit into from
Closed

Fix for thumbnail creation on File upload OC version 81.beta #16355

wants to merge 1 commit into from

Conversation

libasys
Copy link
Contributor

@libasys libasys commented May 14, 2015

If you upload an image that is greater than the config value of preview_max_x/preview_max_y (2048) you run in an issue that the big image also get cropped to 2048 x 2048 cause the first small thumb creation is square 36/36 so the keepAspect is false. with this addition you ge sure the aspect ratio is set to true!

If you upload an image that is greater than the config value of preview_max_x/preview_max_y (2048) you run in an issue that the big image also get cropped to 2048 x 2048 cause the first small thumb creation is square 36/36 so the keepAspect is false. with this addition you ge sure the aspect ratio is set to true!
@scrutinizer-notifier
Copy link

A new inspection was created.

@BernhardPosselt
Copy link
Contributor

@oparoz

@oparoz
Copy link
Contributor

oparoz commented May 14, 2015

@libasys - You're right. It's only a problem with the image provider though, so maybe it's best to only fix that provider so that it actually resizes images instead of just encapsulating them in a OC_Image object?

You could add
$image->fitIn($maxX, $maxY);
to
https://github.com/libasys/core/blob/patch-2/lib/private/preview/image.php#L56

What do you think @georgehrke ?

Also, the test needs fixing and there should be an additional file format, like a psd file, because the providers are not the same.

@libasys
Copy link
Contributor Author

libasys commented May 14, 2015

@oparoz that's a problem of the files app, if you upload a new image in the file app, it automatically creates the 2 images 36&36 px and the max size, or i am wrong?

@oparoz
Copy link
Contributor

oparoz commented May 14, 2015

When you upload a file, the Preview class will ask the registered provider to provide a preview of the file and that provider should return a preview which fits the max dimensions and keeps the aspect ratio.
The Image provider does not (it doesn't use the given dimension). I should have realised that when writing the tests.

The mechanism is described here: #13607

@LukasReschke
Copy link
Member

Unit tests are failing:

08:59:47 There were 3 failures:
08:59:47 
08:59:47 1) Test\Preview::testIsMaxSizeWorking
08:59:47 Failed asserting that 1024 matches expected 640.
08:59:47 
08:59:47 /var/jenkins/workspace/pull-request-analyser-ng-simple/label/SLAVE/tests/lib/preview.php:84
08:59:47 
08:59:47 2) Test\Preview::testIsPreviewDeleted
08:59:47 Failed asserting that true matches expected false.
08:59:47 
08:59:47 /var/jenkins/workspace/pull-request-analyser-ng-simple/label/SLAVE/tests/lib/preview.php:141
08:59:47 
08:59:47 3) Test\Preview::testCreationFromCached
08:59:47 Failed asserting that true matches expected false.
08:59:47 
08:59:47 /var/jenkins/workspace/pull-request-analyser-ng-simple/label/SLAVE/tests/lib/preview.php:222

@libasys
Copy link
Contributor Author

libasys commented May 15, 2015

@oparoz can you fix this issue you are more familiar to the preview class! I think it is really important, cause many users upload pictures to the files app and wondering why there images if the maxX or maxY is greater than the config value are cropped and not resized! Thanks in advance

@oparoz
Copy link
Contributor

oparoz commented May 15, 2015

OK, I will, but you will miss your opportunity to learn more about that class ;)

@libasys
Copy link
Contributor Author

libasys commented May 15, 2015

lol at the moment i maintain about 7 apps, and make sure that's enough work for myself ;)

@oparoz oparoz closed this May 15, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Aug 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants