Resized images #1599

tobiasKaminsky · 2016-04-09T19:55:41Z

--------- edited by @davivel

BLOCKED by owncloud/core#16250

--------- edited by @jabarros
TASKs:

Changes after CR @jabarros
[QA] Create 'resizedImages' branch in QA Repo @jesmrec
[QA] Merge 'resizedImages' branch in QA Repo
[QA] Create test plan @jesmrec
[QA] Validate test plan @jesmrec [WIP]

bugs and improvements:

Unsupported image formats Resized images #1599 (comment)
Bad preview of landscape images Resized images #1599 (comment) @davivel [WIP]

This PR is now against master.

As this PR introduces a rather complex feature I will summarize it here.

Clicking on an image loads a reduced version to the cache.
The width and height is determinated by the screen.
the benefit is a very small, but yet usable preview of the image.
it can be used to send via whatsapp, or as a file to another app (like k9) with the correct filename.
resized images are not treated as downloaded files. If the user wants to have the real file, he has to long-click and select download.
clicking on an image shows first a square thumbnail (if available) until the resized image is loaded

ToDo

Zooming a resized image with at least 2x (which can get blurry)
- show snackbar "automatically download full image on zoom: yes"
- Clicking "yes" after second time will offer option "always" to prevent showing the question.
- time out the snackbar the second time by just waiting will show "never download full image" -> "yes".
- after both show second snackbar without action "default behaviour can be changed in settings"
In setting: new entry "click on image downloads ..."
- full image to storage
- resized image to cache

mention-bot · 2016-04-09T19:55:43Z

By analyzing the blame information on this pull request, we identified @davivel, @jabarros and @purigarcia to be potential reviewers

tobiasKaminsky · 2016-04-09T20:42:43Z

Snackbar is only allowing one action.
So I have adapted the behaviour.
@jancborchardt @AndyScherzinger @davivel

tobiasKaminsky · 2016-04-10T07:24:23Z

@jancborchardt @AndyScherzinger
After thinking about it I think a snackbar (or similar) with two options would be better, or?

jancborchardt · 2016-04-10T11:59:04Z

I thought we said zooming should always download the image? Without showing any snackbar or something like that?

tobiasKaminsky · 2016-04-10T12:05:12Z

No, please read the other PR.
There me and other said that this against the expectation to save bandwidth and storage. So this should stay optional.
And with the snackbar approach every user can decide on its own how the default behaviour should be.

AndyScherzinger · 2016-04-11T07:13:41Z

@tobiasKaminsky your approach works for me since you are right, the snackbar is defined with 0-1 action. For more than 1 action we would need to use dialogs or something else.

davivel · 2016-05-11T08:02:55Z

Having a look now, just read the comments in the thread.

First ideas:

zoom behaviour should be in a separate PR to limit the scope of testing.
about sending preview image to other apps instead of the full size image without the user being aware of it: will this behaviour fit the expectations of most of the users?

Going on with the review.

davivel · 2016-05-11T08:05:10Z

AndroidManifest.xml

@@ -114,7 +114,7 @@
            android:name=".providers.FileContentProvider"
            android:authorities="@string/authority"
            android:enabled="true"
-            android:exported="false"
+            android:exported="true"


Why is this needed in this PR?

I have no idea :/
A quick test shows no problems if I revert this.

tobiasKaminsky · 2016-05-13T16:03:30Z

zoom behaviour should be in a separate PR to limit the scope of testing.

This is a good idea.

about sending preview image to other apps instead of the full size image without the user being aware of it: will this behaviour fit the expectations of most of the users?

If the image is not downloaded I assume that users know that the resized version is used.
The user has to explicit download a file to use this.
But maybe we can show this to the user by using "send resized image".
And use "send original size" when downloaded.

jabarros · 2016-06-06T11:11:57Z

src/com/owncloud/android/datamodel/ThumbnailsCacheManager.java

@@ -281,18 +329,25 @@ private Bitmap doOCFileInBackground() {
                            try {
                                String uri = mClient.getBaseUri() + "" +
                                        "/index.php/apps/files/api/v1/thumbnail/" +


Please, set a constant for this

jabarros · 2016-06-20T11:11:26Z

@tobiasKaminsky In a short term, QA team is going to validate this functionality. Could you take a look to the CR changes and do a rebase on master?

jesmrec · 2016-06-29T15:53:09Z

The images in format .tiff are not previewed, because they are not supported (thumbnail is not shown). If you tap on a tiff image, a spinner appears forever on a black background. That can be confusing for the user, so that he/she does not know if there is a problem or not. If this image format (and another ones) are not supported, it would be better to show an alert or similar indicating that the image .tiff (or whatever) can not be previewed.

cc @tobiasKaminsky @owncloud/android-developers

jesmrec · 2016-06-29T16:11:02Z

if you tap on a "landscape" image, is not correctly resized.

This is the downloaded image:

The previewed (not dowloaded) in portrait:

The previewed (nor downloaded) in landscape:

In this case, this is a "landscape" image (longer than widther), so the preview should fit with the device landscape orientation. The device in portrait orientation should show something like the first attached file.

cc @tobiasKaminsky @owncloud/android-developers

jabarros · 2016-06-30T06:45:29Z

@owncloud/android-developers

due to high number of conflits detected during the rebase from master, I pushed the rebase with the fixes into a new one (resizedImages_fix_rebase) in order to avoid break this PR if some conflicts were not fixed okay.

So, if anyone will work here, please, better move to new resizedImages_fix_rebase branch.

davivel · 2016-07-05T07:11:44Z

Will resume where @jabarros left it, in the branch resizedImages_fix_rebase, fixing the second bug.

davivel · 2016-07-05T09:36:45Z

AndroidManifest.xml

@@ -129,9 +129,9 @@
            android:name=".providers.FileContentProvider"
            android:authorities="@string/authority"
            android:enabled="true"
-            android:exported="true"
+            android:exported="false"


Self-note: this was set to true with addition of Document Provider.

davivel · 2016-07-05T09:38:27Z

Rebase double checked and pushed here; no need to go to other branch.

davivel · 2016-07-05T10:29:44Z

The images in format .tiff are not previewed, because they are not supported (thumbnail is not shown). If you tap on a tiff image, a spinner appears forever on a black background. That can be confusing for the user, so that he/she does not know if there is a problem or not. If this image format (and another ones) are not supported, it would be better to show an alert or similar indicating that the image .tiff (or whatever) can not be previewed.

Even more: if a resized preview can't be got, I'd say the file should be downloaded instead.

davivel · 2016-07-05T10:32:57Z

In this case, this is a "landscape" image (longer than widther), so the preview should fit with the device landscape orientation. The device in portrait orientation should show something like the first attached file.

Previews should use the same rules to adapt the size of the image to the screen that those used to render the full image. In your sample the preview is resulting in a clip of the image, but it should be a rescaled version.

davivel · 2016-07-05T11:15:49Z

src/com/owncloud/android/ui/preview/PreviewImageFragment.java

+
+            if (mShowResizedImage){
+                Bitmap resizedImage = ThumbnailsCacheManager.getBitmapFromDiskCache(
+                        String.valueOf("r" + getFile().getRemoteId()));


Prefix "r" in the identifier to request a resized preview, and prefix "t" in the identifier to request a thumbnail? Is not too tricky? Seems that ThumbnailsCacheManager needs different methods for both cases.

davivel · 2016-07-05T12:02:52Z

Previews should use the same rules to adapt the size of the image to the screen that those used to render the full image. In your sample the preview is resulting in a clip of the image, but it should be a rescaled version.

And in the very end, the problem is what @tobiasKaminsky already stated in the very beginning:

Remaining problem:
owncloud/core#16250

IMHO, this is blocking. 'Til that issue is not addressed, this shouldn't be merged.

zgmnkv · 2017-08-11T20:37:30Z

Hi guys, any update on this one?
Downloading all the images that I viewed consumes a lot of memory on my device.

jesmrec · 2017-08-23T08:42:41Z

not for the moment... you know, we are opened to contributions!!! :)

jesmrec · 2019-08-28T14:12:18Z

Will check with next core generation.

tobiasKaminsky mentioned this pull request Apr 9, 2016

Resized images #1044

Closed

davivel reviewed May 11, 2016
View reviewed changes

This was referenced May 18, 2016

mobile Client: Thumbnail Gallery owncloud/core#24711

Closed

ownCloud Platform - Planning owncloud/core#24684

Closed

Top Open PRs #1454

Closed

tobiasKaminsky mentioned this pull request May 25, 2016

Speed loading of pictures #1689

Closed

jabarros reviewed Jun 6, 2016
View reviewed changes

jesmrec mentioned this pull request Jun 21, 2016

[PR 1599] Resized images owncloud/QA#256

Open

davivel self-assigned this Jul 5, 2016

tobiasKaminsky added 5 commits July 5, 2016 10:42

resized image is loaded instead of full sized image.

876fab3

fixed storing thumbnails by correct imageKey

f8a4776

exported must be set to true

8122ee8

updated for future use with later PRs

49b7489

fixed showing thumbnails in local file list (uploader)

84976d1

tobiasKaminsky and others added 4 commits July 5, 2016 10:42

wip

7d901b1

revert exporting FileContentProvider

d189859

revert wip

04d4dbf

Changes after CR

bab67ce

davivel mentioned this pull request Jul 5, 2016

Resized images (rebased) #1732

Closed

davivel force-pushed the resizedImages branch from 2d0ae20 to bab67ce Compare July 5, 2016 09:35

davivel reviewed Jul 5, 2016
View reviewed changes

AndyScherzinger mentioned this pull request Aug 9, 2016

Resized images nextcloud/android#69

Closed

16 tasks

davivel mentioned this pull request Aug 16, 2016

View Images before downloading #1769

Closed

tobiasKaminsky mentioned this pull request Oct 24, 2017

Resized images - updated nextcloud/android#1710

Merged

16 tasks

michaelstingl mentioned this pull request May 8, 2018

Preview API: different aspect ratio / scale options owncloud/core#31268

Open

tobiasKaminsky closed this Dec 16, 2019

hannesa2 deleted the resizedImages branch July 12, 2021 18:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resized images #1599

Resized images #1599

tobiasKaminsky commented Apr 9, 2016 •

edited by davivel

Loading

mention-bot commented Apr 9, 2016

tobiasKaminsky commented Apr 9, 2016

tobiasKaminsky commented Apr 10, 2016

jancborchardt commented Apr 10, 2016

tobiasKaminsky commented Apr 10, 2016

AndyScherzinger commented Apr 11, 2016

davivel commented May 11, 2016 •

edited

Loading

davivel May 11, 2016

tobiasKaminsky May 13, 2016

tobiasKaminsky commented May 13, 2016

jabarros Jun 6, 2016

jabarros commented Jun 20, 2016

jesmrec commented Jun 29, 2016 •

edited

Loading

jesmrec commented Jun 29, 2016

jabarros commented Jun 30, 2016

davivel commented Jul 5, 2016

davivel Jul 5, 2016

davivel commented Jul 5, 2016 •

edited

Loading

davivel commented Jul 5, 2016 •

edited

Loading

davivel commented Jul 5, 2016

davivel Jul 5, 2016

davivel commented Jul 5, 2016

zgmnkv commented Aug 11, 2017

jesmrec commented Aug 23, 2017

jesmrec commented Aug 28, 2019

Resized images #1599

Resized images #1599

Conversation

tobiasKaminsky commented Apr 9, 2016 • edited by davivel Loading

mention-bot commented Apr 9, 2016

tobiasKaminsky commented Apr 9, 2016

tobiasKaminsky commented Apr 10, 2016

jancborchardt commented Apr 10, 2016

tobiasKaminsky commented Apr 10, 2016

AndyScherzinger commented Apr 11, 2016

davivel commented May 11, 2016 • edited Loading

davivel May 11, 2016

Choose a reason for hiding this comment

tobiasKaminsky May 13, 2016

Choose a reason for hiding this comment

tobiasKaminsky commented May 13, 2016

jabarros Jun 6, 2016

Choose a reason for hiding this comment

jabarros commented Jun 20, 2016

jesmrec commented Jun 29, 2016 • edited Loading

jesmrec commented Jun 29, 2016

jabarros commented Jun 30, 2016

davivel commented Jul 5, 2016

davivel Jul 5, 2016

Choose a reason for hiding this comment

davivel commented Jul 5, 2016 • edited Loading

davivel commented Jul 5, 2016 • edited Loading

davivel commented Jul 5, 2016

davivel Jul 5, 2016

Choose a reason for hiding this comment

davivel commented Jul 5, 2016

zgmnkv commented Aug 11, 2017

jesmrec commented Aug 23, 2017

jesmrec commented Aug 28, 2019

tobiasKaminsky commented Apr 9, 2016 •

edited by davivel

Loading

davivel commented May 11, 2016 •

edited

Loading

jesmrec commented Jun 29, 2016 •

edited

Loading

davivel commented Jul 5, 2016 •

edited

Loading

davivel commented Jul 5, 2016 •

edited

Loading