From bb49fb9801be251ccc59e75cc9b63ecc7754e590 Mon Sep 17 00:00:00 2001 From: Dmitry Brant Date: Fri, 19 May 2017 15:08:05 +0200 Subject: [PATCH] Clean up image loading code, and switch to using Fresco. --- app/build.gradle | 1 + .../free/nrw/commons/CommonsApplication.java | 48 +--- .../nrw/commons/MediaThumbnailFetchTask.java | 27 +++ .../free/nrw/commons/MediaWikiImageView.java | 222 +++--------------- .../fr/free/nrw/commons/WelcomeActivity.java | 4 +- .../ContributionsListAdapter.java | 14 +- .../commons/media/MediaDetailFragment.java | 6 +- 7 files changed, 68 insertions(+), 254 deletions(-) create mode 100644 app/src/main/java/fr/free/nrw/commons/MediaThumbnailFetchTask.java diff --git a/app/build.gradle b/app/build.gradle index 3e848d539..d212c5a50 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -20,6 +20,7 @@ dependencies { compile ('com.mapbox.mapboxsdk:mapbox-android-sdk:5.0.2@aar'){ transitive=true } + compile 'com.facebook.fresco:fresco:1.3.0' compile 'com.facebook.stetho:stetho:1.5.0' testCompile 'junit:junit:4.12' diff --git a/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java b/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java index bc815b9b7..696561b6a 100644 --- a/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java +++ b/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java @@ -6,14 +6,13 @@ import android.accounts.AuthenticatorException; import android.accounts.OperationCanceledException; import android.app.Application; import android.content.pm.PackageManager; -import android.graphics.Bitmap; import android.os.Build; -import android.support.v4.util.LruCache; import com.android.volley.RequestQueue; import com.android.volley.toolbox.BasicNetwork; import com.android.volley.toolbox.DiskBasedCache; import com.android.volley.toolbox.HurlStack; +import com.facebook.drawee.backends.pipeline.Fresco; import com.facebook.stetho.Stetho; import com.nostra13.universalimageloader.cache.disc.impl.TotalSizeLimitedDiscCache; import com.nostra13.universalimageloader.core.ImageLoader; @@ -112,31 +111,11 @@ public class CommonsApplication extends Application { .build(); ImageLoader.getInstance().init(imageLoaderConfiguration); + Fresco.initialize(this); + // Initialize EventLogging EventLog.setApp(this); - // based off https://developer.android.com/training/displaying-bitmaps/cache-bitmap.html - // Cache for 1/8th of available VM memory - long maxMem = Runtime.getRuntime().maxMemory(); - if (maxMem < 48L * 1024L * 1024L) { - // Cache only one bitmap if VM memory is too small (such as Nexus One); - Timber.d("Skipping bitmap cache; max mem is: %d", maxMem); - imageCache = new LruCache<>(1); - } else { - int cacheSize = (int) (maxMem / (1024 * 8)); - Timber.d("Bitmap cache size %d from max mem %d", cacheSize, maxMem); - imageCache = new LruCache(cacheSize) { - @Override - protected int sizeOf(String key, Bitmap bitmap) { - int bitmapSize; - bitmapSize = bitmap.getByteCount(); - - // The cache size will be measured in kilobytes rather than number of items. - return bitmapSize / 1024; - } - }; - } - //For caching area -> categories cacheData = new CacheController(); @@ -145,27 +124,6 @@ public class CommonsApplication extends Application { volleyQueue.start(); } - private com.android.volley.toolbox.ImageLoader imageLoader; - private LruCache imageCache; - - public com.android.volley.toolbox.ImageLoader getImageLoader() { - if(imageLoader == null) { - imageLoader = new com.android.volley.toolbox.ImageLoader(volleyQueue, new com.android.volley.toolbox.ImageLoader.ImageCache() { - @Override - public Bitmap getBitmap(String key) { - return imageCache.get(key); - } - - @Override - public void putBitmap(String key, Bitmap bitmap) { - imageCache.put(key, bitmap); - } - }); - imageLoader.setBatchedResponseDelay(0); - } - return imageLoader; - } - public MWApi getApi() { return api; } diff --git a/app/src/main/java/fr/free/nrw/commons/MediaThumbnailFetchTask.java b/app/src/main/java/fr/free/nrw/commons/MediaThumbnailFetchTask.java new file mode 100644 index 000000000..e5076faed --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/MediaThumbnailFetchTask.java @@ -0,0 +1,27 @@ +package fr.free.nrw.commons; + +import android.os.AsyncTask; + +import org.mediawiki.api.ApiResult; + +class MediaThumbnailFetchTask extends AsyncTask { + private static final String THUMB_SIZE = "640"; + + @Override + protected String doInBackground(String... params) { + try { + MWApi api = CommonsApplication.app.getApi(); + ApiResult result =api.action("query") + .param("format", "xml") + .param("prop", "imageinfo") + .param("iiprop", "url") + .param("iiurlwidth", THUMB_SIZE) + .param("titles", params[0]) + .get(); + return result.getString("/api/query/pages/page/imageinfo/ii/@thumburl"); + } catch (Exception e) { + // Do something better! + } + return null; + } +} diff --git a/app/src/main/java/fr/free/nrw/commons/MediaWikiImageView.java b/app/src/main/java/fr/free/nrw/commons/MediaWikiImageView.java index 02b69b260..d0913edc3 100644 --- a/app/src/main/java/fr/free/nrw/commons/MediaWikiImageView.java +++ b/app/src/main/java/fr/free/nrw/commons/MediaWikiImageView.java @@ -1,47 +1,16 @@ -/** - * Copyright (C) 2013 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ package fr.free.nrw.commons; import android.content.Context; -import android.content.res.TypedArray; -import android.graphics.drawable.BitmapDrawable; -import android.text.TextUtils; +import android.support.annotation.Nullable; +import android.support.v4.util.LruCache; import android.util.AttributeSet; -import android.view.View; -import android.widget.ImageView; -import com.android.volley.VolleyError; -import com.android.volley.toolbox.ImageLoader; -import com.android.volley.toolbox.ImageLoader.ImageContainer; -import com.android.volley.toolbox.ImageLoader.ImageListener; +import com.facebook.drawee.view.SimpleDraweeView; -import fr.free.nrw.commons.contributions.Contribution; -import fr.free.nrw.commons.contributions.ContributionsContentProvider; +public class MediaWikiImageView extends SimpleDraweeView { + private ThumbnailFetchTask currentThumbnailTask; + LruCache thumbnailUrlCache = new LruCache<>(1024); -public class MediaWikiImageView extends ImageView { - - private Media mMedia; - - private ImageLoader mImageLoader; - - private ImageContainer mImageContainer; - - private View loadingView; - - private boolean isThumbnail; public MediaWikiImageView(Context context) { this(context, null); @@ -49,179 +18,48 @@ public class MediaWikiImageView extends ImageView { public MediaWikiImageView(Context context, AttributeSet attrs) { this(context, attrs, 0); - TypedArray actualAttrs = context.getTheme().obtainStyledAttributes(attrs, R.styleable.MediaWikiImageView, 0, 0); - isThumbnail = actualAttrs.getBoolean(0, false); - actualAttrs.recycle(); } public MediaWikiImageView(Context context, AttributeSet attrs, int defStyle) { super(context, attrs, defStyle); } - public void setMedia(Media media, ImageLoader imageLoader) { - this.mMedia = media; - mImageLoader = imageLoader; - loadImageIfNecessary(false); - } - - public void setLoadingView(View loadingView) { - this.loadingView = loadingView; - } - - public View getLoadingView() { - return loadingView; - } - - private void loadImageIfNecessary(final boolean isInLayoutPass) { - loadImageIfNecessary(isInLayoutPass, false); - } - - private void loadImageIfNecessary(final boolean isInLayoutPass, final boolean tryOriginal) { - int width = getWidth(); - int height = getHeight(); - - // if the view's bounds aren't known yet, hold off on loading the image. - if (width == 0 && height == 0) { + public void setMedia(Media media) { + if (currentThumbnailTask != null) { + currentThumbnailTask.cancel(true); + } + setImageURI((String) null); + if(media == null) { return; } - if(mMedia == null) { - return; - } - - // Do not count for density when loading thumbnails. - // FIXME: Use another 'algorithm' that doesn't punish low res devices - if(isThumbnail) { - float dpFactor = Math.max(getResources().getDisplayMetrics().density, 1.0f); - width = (int) (width / dpFactor); - height = (int) (height / dpFactor); - } - - final String mUrl; - if(tryOriginal) { - mUrl = mMedia.getImageUrl(); + if (thumbnailUrlCache.get(media.getFilename()) != null) { + setImageUrl(thumbnailUrlCache.get(media.getFilename())); } else { - // Round it to the nearest 320 - // Possible a similar size image has already been generated. - // Reduces Server cache fragmentation, also increases chance of cache hit - // If width is less than 320, we round up to 320 - int bucketedWidth = width <= 320 ? 320 : Math.round((float)width / 320.0f) * 320; - if(mMedia.getWidth() != 0 && mMedia.getWidth() < bucketedWidth) { - // If we know that the width of the image is lesser than the required width - // We don't even try to load the thumbnai, go directly to the source - loadImageIfNecessary(isInLayoutPass, true); - return; - } else { - mUrl = mMedia.getThumbnailUrl(bucketedWidth); - } + currentThumbnailTask = new ThumbnailFetchTask(); + currentThumbnailTask.execute(media.getFilename()); } - - // if the URL to be loaded in this view is empty, cancel any old requests and clear the - // currently loaded image. - if (TextUtils.isEmpty(mUrl)) { - if (mImageContainer != null) { - mImageContainer.cancelRequest(); - mImageContainer = null; - } - setImageBitmap(null); - return; - } - - // Don't repeat work. Prevents onLayout cascades - // We ignore it if the image request was for either the current URL of for the full URL - // Since the full URL is always the second, and - if (mImageContainer != null && mImageContainer.getRequestUrl() != null) { - if (mImageContainer.getRequestUrl().equals(mMedia.getImageUrl()) || mImageContainer.getRequestUrl().equals(mUrl)) { - return; - } else { - // if there is a pre-existing request, cancel it if it's fetching a different URL. - mImageContainer.cancelRequest(); - BitmapDrawable actualDrawable = (BitmapDrawable)getDrawable(); - if(actualDrawable != null && actualDrawable.getBitmap() != null) { - setImageBitmap(null); - if(loadingView != null) { - loadingView.setVisibility(View.VISIBLE); - } - } - } - } - - // The pre-existing content of this view didn't match the current URL. Load the new image - // from the network. - ImageContainer newContainer = mImageLoader.get(mUrl, - new ImageListener() { - @Override - public void onErrorResponse(final VolleyError error) { - if(!tryOriginal) { - post(new Runnable() { - @Override - public void run() { - loadImageIfNecessary(false, true); - } - }); - } - } - - @Override - public void onResponse(final ImageContainer response, boolean isImmediate) { - // If this was an immediate response that was delivered inside of a layout - // pass do not set the image immediately as it will trigger a requestLayout - // inside of a layout. Instead, defer setting the image by posting back to - // the main thread. - if (isImmediate && isInLayoutPass) { - post(new Runnable() { - @Override - public void run() { - onResponse(response, false); - } - }); - return; - } - - if (response.getBitmap() != null) { - setImageBitmap(response.getBitmap()); - if(tryOriginal && mMedia instanceof Contribution && (response.getBitmap().getWidth() > mMedia.getWidth() || response.getBitmap().getHeight() > mMedia.getHeight())) { - // If there is no width information for this image, save it. This speeds up image loading massively for smaller images - mMedia.setHeight(response.getBitmap().getHeight()); - mMedia.setWidth(response.getBitmap().getWidth()); - ((Contribution)mMedia).setContentProviderClient(MediaWikiImageView.this.getContext().getContentResolver().acquireContentProviderClient(ContributionsContentProvider.AUTHORITY)); - ((Contribution)mMedia).save(); - } - if(loadingView != null) { - loadingView.setVisibility(View.GONE); - } - } else { - // I'm not really sure where this would hit but not onError - } - } - }); - - // update the ImageContainer to be the new bitmap container. - mImageContainer = newContainer; - } - - @Override - protected void onLayout(boolean changed, int left, int top, int right, int bottom) { - super.onLayout(changed, left, top, right, bottom); - loadImageIfNecessary(true); } @Override protected void onDetachedFromWindow() { - if (mImageContainer != null) { - // If the view was bound to an image request, cancel it and clear - // out the image from the view. - mImageContainer.cancelRequest(); - setImageBitmap(null); - // also clear out the container so we can reload the image if necessary. - mImageContainer = null; + if (currentThumbnailTask != null) { + currentThumbnailTask.cancel(true); } super.onDetachedFromWindow(); } - @Override - protected void drawableStateChanged() { - super.drawableStateChanged(); - invalidate(); + private void setImageUrl(@Nullable String url) { + setImageURI(url); + } + + private class ThumbnailFetchTask extends MediaThumbnailFetchTask { + @Override + protected void onPostExecute(String result) { + if (isCancelled()) { + return; + } + setImageUrl(result); + } } } diff --git a/app/src/main/java/fr/free/nrw/commons/WelcomeActivity.java b/app/src/main/java/fr/free/nrw/commons/WelcomeActivity.java index 30c8baf84..6889d4ef2 100644 --- a/app/src/main/java/fr/free/nrw/commons/WelcomeActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/WelcomeActivity.java @@ -20,7 +20,9 @@ public class WelcomeActivity extends BaseActivity { super.onCreate(savedInstanceState); setContentView(R.layout.activity_welcome); - getSupportActionBar().hide(); + if (getSupportActionBar() != null) { + getSupportActionBar().hide(); + } ButterKnife.bind(this); setUpAdapter(); diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListAdapter.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListAdapter.java index 3a6aa72c3..cb23ac955 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListAdapter.java @@ -4,7 +4,6 @@ import android.app.Activity; import android.content.Context; import android.database.Cursor; import android.graphics.Bitmap; -import android.graphics.drawable.BitmapDrawable; import android.support.v4.widget.CursorAdapter; import android.text.TextUtils; import android.view.View; @@ -15,7 +14,6 @@ import com.nostra13.universalimageloader.core.ImageLoader; import com.nostra13.universalimageloader.core.assist.FailReason; import com.nostra13.universalimageloader.core.assist.SimpleImageLoadingListener; -import fr.free.nrw.commons.CommonsApplication; import fr.free.nrw.commons.MediaWikiImageView; import fr.free.nrw.commons.R; import fr.free.nrw.commons.Utils; @@ -49,9 +47,10 @@ class ContributionsListAdapter extends CursorAdapter { if(views.url == null || !views.url.equals(actualUrl)) { if(actualUrl.startsWith("http")) { MediaWikiImageView mwImageView = views.imageView; - mwImageView.setMedia(contribution, ((CommonsApplication) activity.getApplicationContext()).getImageLoader()); + mwImageView.setMedia(contribution); // FIXME: For transparent images } else { + ImageLoader.getInstance().displayImage(actualUrl, views.imageView, contributionDisplayOptions, new SimpleImageLoadingListener() { @Override @@ -66,20 +65,13 @@ class ContributionsListAdapter extends CursorAdapter { public void onLoadingFailed(String imageUri, View view, FailReason failReason) { super.onLoadingFailed(imageUri, view, failReason); MediaWikiImageView mwImageView = views.imageView; - mwImageView.setMedia(contribution, ((CommonsApplication) activity.getApplicationContext()).getImageLoader()); + mwImageView.setMedia(contribution); } }); } views.url = actualUrl; } - BitmapDrawable actualImageDrawable = (BitmapDrawable)views.imageView.getDrawable(); - if(actualImageDrawable != null && actualImageDrawable.getBitmap() != null && actualImageDrawable.getBitmap().hasAlpha()) { - views.imageView.setBackgroundResource(android.R.color.white); - } else { - views.imageView.setBackgroundDrawable(null); - } - views.titleView.setText(contribution.getDisplayTitle()); views.seqNumView.setText(String.valueOf(cursor.getPosition() + 1)); diff --git a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java index 6d46da801..7cd168478 100644 --- a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java @@ -17,7 +17,6 @@ import android.widget.ProgressBar; import android.widget.ScrollView; import android.widget.TextView; -import com.android.volley.toolbox.ImageLoader; import com.nostra13.universalimageloader.core.DisplayImageOptions; import com.nostra13.universalimageloader.core.assist.FailReason; import com.nostra13.universalimageloader.core.assist.ImageLoadingListener; @@ -27,7 +26,6 @@ import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Date; -import fr.free.nrw.commons.CommonsApplication; import fr.free.nrw.commons.License; import fr.free.nrw.commons.LicenseList; import fr.free.nrw.commons.Media; @@ -194,10 +192,8 @@ public class MediaDetailFragment extends Fragment { if(actualUrl.startsWith("http")) { Timber.d("Actual URL starts with http and is: %s", actualUrl); - ImageLoader loader = ((CommonsApplication)getActivity().getApplicationContext()).getImageLoader(); MediaWikiImageView mwImage = (MediaWikiImageView)image; - mwImage.setLoadingView(loadingProgress); //FIXME: Set this as an attribute - mwImage.setMedia(media, loader); + mwImage.setMedia(media); // FIXME: For transparent images // FIXME: keep the spinner going while we load data