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

Preview providers should take maxX and maxY into consideration #13607

Closed
6 tasks done
oparoz opened this issue Jan 22, 2015 · 8 comments
Closed
6 tasks done

Preview providers should take maxX and maxY into consideration #13607

oparoz opened this issue Jan 22, 2015 · 8 comments

Comments

@oparoz
Copy link
Contributor

oparoz commented Jan 22, 2015

Problem

The interface includes maX and maxY for a reason and yet, none of the providers use it.

A 5000x5000, 140MB TIFF is currently being converted to a 5000x5000 50MB PNG.
Fortunately, it will soon be possible to avoid multiple roundtrips to ImageMagick (via #13674) , but each resizing request of that 50MB PNG is still going to take some time (and waste memory) and that will only partially be fixed once #8196 is implemented.

Solution

Make sure preview providers take maxX and maxY into consideration when generating previews. With #13674 in place, each provider should only be called once.

I'm going to fix Bitmap, but the other providers should be fixed as well.

  • Bitmap
  • Image
  • Txt
  • MP3
  • SVG
  • Movie
@oparoz
Copy link
Contributor Author

oparoz commented Jan 23, 2015

Job done for all files using the bitmap preview class: #13635

@oparoz
Copy link
Contributor Author

oparoz commented May 14, 2015

Re-opening, as the Image provider is not taking the max dimensions into consideration

@oparoz
Copy link
Contributor Author

oparoz commented May 18, 2015

Fix for all remaining providers is in #16382

@PVince81
Copy link
Contributor

You tagged this as regression, so this worked before ?

@oparoz
Copy link
Contributor Author

oparoz commented May 26, 2015

Yes, before the max preview, each provider would be queried for each request and thus provide a preview matching the asked dimension. Since they're now only use to generate the max preview, we need to make sure those previews keep the aspect ratio of the original image.

@DeepDiver1975
Copy link
Member

@oparoz we can close this now that #16382 is merged? THX

@oparoz
Copy link
Contributor Author

oparoz commented Jun 8, 2015

Yes, the API should now return the expected previews.

@oparoz oparoz closed this as completed Jun 8, 2015
@DeepDiver1975
Copy link
Member

Yes, the API should now return the expected previews.

awesome - thanks @oparoz for your contribution - very much welcome!

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

No branches or pull requests

4 participants