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

Replace image caching system with Glide 4 #2871

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

tobiasKaminsky
Copy link
Member

@tobiasKaminsky tobiasKaminsky commented Aug 10, 2018

This replaces our old image caching system with Glide 4.
I first updated Glide 3 to 4 and then replaced our old system step by step.
Unfortunately it was not really possible to have this in smaller steps :-(

Info:

TODO:

  • video play icon overlay: on grid view bigger @AndyScherzinger maybe you have a nice idea?
  • video without generated thumbnail also has "play button", which then overlays the dummy thumbnail, @AndyScherzinger maybe?
  • external links:
    • not parsed correctly with svg, image is too small. But this is working correctly on activity stream, so this must be a problem with MenuItem
  • check if we can transfer the old thumbnails

After finishing, I will squash and rebase.

@tobiasKaminsky
Copy link
Member Author

This is first published to see what Codacy/Findbugs complain…

@tobiasKaminsky tobiasKaminsky changed the title WIP: Replace image caching system with Glide 4 Replace image caching system with Glide 4 Aug 10, 2018
@tobiasKaminsky tobiasKaminsky mentioned this pull request Aug 14, 2018
1 task
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nextcloud nextcloud deleted a comment Aug 15, 2018
@nachoparker
Copy link
Member

keep up the good work guys, let me know if I can help test this

@tobiasKaminsky
Copy link
Member Author

Thanks @nachoparker. This will be soon in dev version, so you can then give us feedback (you can install it in parallel):
https://github.com/nextcloud/android/blob/master/CONTRIBUTING.md#dev-release

@nextcloud nextcloud deleted a comment Sep 25, 2018
@nextcloud nextcloud deleted a comment Sep 25, 2018
@nextcloud nextcloud deleted a comment Sep 25, 2018
@tobiasKaminsky
Copy link
Member Author

@AndyScherzinger @mario I tried again to get this SVG working in external links, but I do not know why it fails.
When debugging it, it sometimes returns a SVG, but then it is too small.

So I would go on with review & merge it and keep the issue separate.

@AndyScherzinger
Copy link
Member

So I would go on with review & merge it and keep the issue separate.

fine by me, I'll just run another test and then I think we would be good to go.

@AndyScherzinger
Copy link
Member

@tobiasKaminsky Just tested, so the thumbnails now seems to be the right ones but scrolling+fetching doesn't seem to work. So if I scroll slowly so that some thumbnails on the shown list are already there, the next ones get fetched, if I scroll faster and stop then the thumbnails aren't fetched anymore :(

@AndyScherzinger
Copy link
Member

If you leave the folder and navigate back into it the thumbnails previously not shown do show up now but only the ones from the scroll-positions where I have been before. So it seems the fetching gets triggered but they don't get displayed navigating back/forth they are shown (i guess because they are now in the cache).

@nextcloud nextcloud deleted a comment Sep 25, 2018
@nachoparker
Copy link
Member

@tobiasKaminsky please, let me know when this is out in dev

@mario
Copy link
Contributor

mario commented Oct 24, 2018

@tobiasKaminsky what about the usage comments by @AndyScherzinger ?

Signed-off-by: tobiasKaminsky <[email protected]>
@mario
Copy link
Contributor

mario commented Dec 12, 2018

@tobiasKaminsky not ready?

@tobiasKaminsky
Copy link
Member Author

No :(
It has one nasty bug found by @AndyScherzinger. If you scroll through a very long list of images, at some point glide just stops trying to generate new thumbnails.

Unfortunately I did not had any time to dive into it :/
But I hope to tackle it before christmas.

@AndyScherzinger
Copy link
Member

needs a rebase (with lots of conflicts to be solved)

@codeRemark
Copy link

if this can use for newest server ?

@AndyScherzinger
Copy link
Member

@codeRemark the branch should work with the latest server while this branch is very outdated (see conflicts) and has quite some bugs to iron out first (see todo list in original description). So the branch is in need of a proper rebase and bugfixing. Other than that given the UI revamp that is happening at the moment there might be even more code conflicts in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants