-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Introducing the maximum size preview #13582
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
Conversation
Refer to this link for build results (access rights to CI server needed): Build result: FAILURE[...truncated 13 lines...] > git config --local --remove-section credential # timeout=10 > git config remote.origin.url https://github.com/owncloud/core.git # timeout=10 > git config remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > git config remote.origin.url https://github.com/owncloud/core.git # timeout=10Fetching upstream changes from https://github.com/owncloud/core.gitusing GIT_SSH to set credentials using .gitcredentials to set credentials > git config --local credential.helper store --file=/tmp/git4573009880408482453.credentials # timeout=10 > git -c core.askpass=true fetch --tags --progress https://github.com/owncloud/core.git +refs/pull/:refs/remotes/origin/pr/ > git config --local --remove-section credential # timeout=10 > git rev-parse refs/remotes/origin/pr/13582/merge^{commit} # timeout=10 > git rev-parse refs/remotes/origin/origin/pr/13582/merge^{commit} # timeout=10Checking out Revision 493307864349c7a609668e52a6f0772745ef6816 (refs/remotes/origin/pr/13582/merge) > git config core.sparsecheckout # timeout=10 > git checkout -f 493307864349c7a609668e52a6f0772745ef6816First time build. Skipping changelog. > git remote # timeout=10 > git submodule init # timeout=10 > git submodule sync # timeout=10 > git config --get remote.origin.url # timeout=10 > git submodule update --init --recursiveTriggering pull-request-analyser-ng-simple » SLAVEpull-request-analyser-ng-simple » SLAVE completed with result FAILURESetting status of 890d31dc937ff4ac93054217461a0cd5d4e97542 to FAILURE with url https://ci.owncloud.org/job/pull-request-analyser-ng-simple/7635/ and message: Merged build finished.💣 Test FAILed. 💣 |
@oparoz thanks a lot! really awesome! Let schedule this for 8.1 |
Refer to this link for build results (access rights to CI server needed): |
👍 for the proposed solution! Thanks a lot. Will review the code asap |
@georgehrke after OC8 has been released please - THX |
} | ||
|
||
// The providers have been kind enough to give us a preview | ||
if (!is_null($preview)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use if (preview)
instead as provider's response can be boolean
Refer to this link for build results (access rights to CI server needed): |
The inspection completed: 6 new issues, 5 updated code elements |
Refer to this link for build results (access rights to CI server needed): |
Fixes
The problem
You've just uploaded a 100MB TIFF file that you need to share with a restricted audience. Here is what currently happens once the file has been uploaded.
You're starting to get it. Every time a slightly different preview size is asked, a full conversion of that 100MB TIFF to PNG is performed. This is highly inefficient.
Proposed solution
Default size is 2048x2048
The maximum size preview is keeping the aspect ratio of the original and is not upscaled.
It's is immediately stored in the cache with a special
max
tag and size identifiers based on its real size.Every time a preview of a different size is asked, we store it in the cache. This also speeds up operation at the cost of disk space.
This make a huge difference in libraries holding several large files, for people using apps such as the Gallery.