jenkins-bot has submitted this change and it was merged.
Change subject: Reduce Share A Fact memory and fix lead font face
......................................................................
Reduce Share A Fact memory and fix lead font face
* Replace LeadImagesHandler.getBitmapFromView() with
ImageViewWithFace.getImageBitmap(). This method returns a previously
set Bitmap or null to reduce the number of images in memory. It is
used indirectly by Share A Fact.
* Replace lead title and subtitle android:fontFamily attribute with
Java. This gives more consistent appearance and leading on API 15
which doesn't support the attribute.
* Move large Runnable.run() implementation to private method,
LeadImagesHandler.detectFace(). This translation likely requires diff
stitching. No other changes intended other than those mentioned in the
bullets.
* Move anonymous inline SaneAsyncTask from
ImageViewWithFace.onBitmapLoaded() to private nested class,
FaceDetectionTask.
* Change OnImageLoadListener.onImageLoaded() face detection failure
value from negative to null.
* Rename anonymous ImageViewWithFace SaneAsyncTask subclass local
variable from tempbmp to testBmp; extract method for generating test
bitmap, new565ScaledBitmap(); move face detection eligibility check
before spawning the task.
* Minor reordering of ImageViewWithFace members.
* Replace nullable OnImageLoadListener with null pattern.
* Replace ImageViewWithFace Log.* calls with L.*.
* Add helper method for setting lead image layout parameters,
setImageLayoutParams().
* Add IDE hint annotations.
* Move "faceBoost", the y-axis offset to the nose, to
face_detection_nose_y_offset.
Change-Id: Ibc793ab778691b9a2d182527e14a6b9fb2703ef3
---
M app/src/main/java/org/wikipedia/page/leadimages/ImageViewWithFace.java
M app/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java
M app/src/main/res/layout/fragment_page.xml
M app/src/main/res/values/dimens.xml
4 files changed, 179 insertions(+), 157 deletions(-)
Approvals:
Dbrant: Looks good to me, approved
jenkins-bot: Verified
diff --git
a/app/src/main/java/org/wikipedia/page/leadimages/ImageViewWithFace.java
b/app/src/main/java/org/wikipedia/page/leadimages/ImageViewWithFace.java
index 63ba0e4..161c086 100644
--- a/app/src/main/java/org/wikipedia/page/leadimages/ImageViewWithFace.java
+++ b/app/src/main/java/org/wikipedia/page/leadimages/ImageViewWithFace.java
@@ -7,26 +7,27 @@
import android.graphics.Paint;
import android.graphics.PointF;
import android.graphics.Rect;
+import android.graphics.drawable.BitmapDrawable;
import android.graphics.drawable.Drawable;
import android.media.FaceDetector;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
import android.util.AttributeSet;
-import android.util.Log;
import android.widget.ImageView;
import com.squareup.picasso.Picasso;
import com.squareup.picasso.Target;
import org.wikipedia.concurrency.SaneAsyncTask;
+import org.wikipedia.util.log.L;
public class ImageViewWithFace extends ImageView implements Target {
- private static final String TAG = "ImageViewWithFace";
+ public interface OnImageLoadListener {
+ void onImageLoaded(Bitmap bitmap, @Nullable PointF faceLocation);
+ void onImageFailed();
+ }
- // Width to which to reduce image copy on which face detection is
performed in onBitMapLoaded()
- // (with height reduced proportionally there). Performing face detection
on a scaled-down
- // image copy improves speed and memory use.
- //
- // Also, note that the face detector requires that the image width be even.
- private static final int BITMAP_COPY_WIDTH = 200;
+ @NonNull private OnImageLoadListener listener = new DefaultListener();
public ImageViewWithFace(Context context) {
super(context);
@@ -40,91 +41,121 @@
super(context, attrs, defStyle);
}
+ public void setOnImageLoadListener(@Nullable OnImageLoadListener listener)
{
+ this.listener = listener == null ? new DefaultListener() : listener;
+ }
+
+ @Nullable public Bitmap getImageBitmap() {
+ return getDrawable() instanceof BitmapDrawable
+ ? ((BitmapDrawable) getDrawable()).getBitmap()
+ : null;
+ }
+
@Override
- public void onBitmapLoaded(final Bitmap bitmap, Picasso.LoadedFrom from) {
- (new SaneAsyncTask<Void>(SaneAsyncTask.LOW_CONCURRENCY) {
- @Override
- public Void performTask() throws Throwable {
- final int minSize = 64;
- // don't load the image if it's terrible resolution
- // (or indeed, if it's zero width or height)
- if (bitmap.getWidth() < minSize || bitmap.getHeight() <
minSize) {
- if (onImageLoadListener != null) {
- onImageLoadListener.onImageFailed();
- }
- return null;
- }
- // boost this thread's priority a bit
- Thread.currentThread().setPriority(Thread.MAX_PRIORITY - 1);
- long millis = System.currentTimeMillis();
- // create a new bitmap onto which we'll draw the original
bitmap,
- // because the FaceDetector requires it to be a 565 bitmap,
and it
- // must also be even width. Reduce size of copy for
performance.
- Bitmap tempbmp = Bitmap.createBitmap(BITMAP_COPY_WIDTH,
- ((bitmap.getHeight() * BITMAP_COPY_WIDTH) /
bitmap.getWidth()), Bitmap.Config.RGB_565);
- Canvas canvas = new Canvas(tempbmp);
- Rect srcRect = new Rect(0, 0, bitmap.getWidth(),
bitmap.getHeight());
- Rect destRect = new Rect(0, 0, BITMAP_COPY_WIDTH,
tempbmp.getHeight());
- Paint paint = new Paint();
- paint.setColor(Color.BLACK);
- canvas.drawBitmap(bitmap, srcRect, destRect, paint);
+ public void onBitmapLoaded(Bitmap bitmap, Picasso.LoadedFrom from) {
+ if (isBitmapEligibleForFaceDetection(bitmap)) {
+ spawnFaceDetectionTask(bitmap);
+ } else {
+ listener.onImageFailed();
+ }
- // initialize the face detector, and look for only one face...
- FaceDetector fd = new FaceDetector(tempbmp.getWidth(),
tempbmp.getHeight(), 1);
- FaceDetector.Face[] faces = new FaceDetector.Face[1];
- PointF facePos = new PointF();
- int numFound = fd.findFaces(tempbmp, faces);
-
- if (numFound > 0) {
- faces[0].getMidPoint(facePos);
- // scale back to proportions of original image
- facePos.x = (facePos.x * bitmap.getWidth() /
BITMAP_COPY_WIDTH);
- facePos.y = (facePos.y * bitmap.getHeight() /
tempbmp.getHeight());
- Log.d(TAG, "Found face at " + facePos.x + ", " +
facePos.y);
- }
- // free our temporary bitmap
- tempbmp.recycle();
-
- Log.d(TAG, "Face detection took " +
(System.currentTimeMillis() - millis) + "ms");
-
- if (onImageLoadListener != null) {
- onImageLoadListener.onImageLoaded(bitmap, facePos);
- }
- return null;
- }
- @Override
- public void onCatch(Throwable caught) {
- // it's not super important to do anything if face detection
fails,
- // but let our listener know about it anyway:
- if (onImageLoadListener != null) {
- onImageLoadListener.onImageFailed();
- }
- }
- }).execute();
// and, of course, set the original bitmap as our image
- this.setImageBitmap(bitmap);
+ setImageBitmap(bitmap);
}
@Override
public void onBitmapFailed(Drawable errorDrawable) {
- if (onImageLoadListener != null) {
- onImageLoadListener.onImageFailed();
+ listener.onImageFailed();
+ }
+
+ @Override public void onPrepareLoad(Drawable placeHolderDrawable) { }
+
+ private boolean isBitmapEligibleForFaceDetection(Bitmap bitmap) {
+ final int minSize = 64;
+ return bitmap.getWidth() >= minSize && bitmap.getHeight() >= minSize;
+ }
+
+ private void spawnFaceDetectionTask(@NonNull final Bitmap bitmap) {
+ new FaceDetectionTask(SaneAsyncTask.LOW_CONCURRENCY, bitmap) {
+ @Override
+ public PointF performTask() {
+ PointF facePos = super.performTask();
+ listener.onImageLoaded(bitmap, facePos);
+ return facePos;
+ }
+
+ @Override
+ public void onCatch(Throwable caught) {
+ // it's not super important to do anything if face detection
fails,
+ // but let our listener know about it anyway:
+ listener.onImageFailed();
+ }
+ }.execute();
+ }
+
+ private static class FaceDetectionTask extends SaneAsyncTask<PointF> {
+ // Width to which to reduce image copy on which face detection is
performed in onBitMapLoaded()
+ // (with height reduced proportionally there). Performing face
detection on a scaled-down
+ // image copy improves speed and memory use.
+ //
+ // Also, note that the face detector requires that the image width be
even.
+ private static final int BITMAP_COPY_WIDTH = 200;
+
+ @NonNull private final Bitmap srcBitmap;
+
+ public FaceDetectionTask(int concurrency, @NonNull Bitmap bitmap) {
+ super(concurrency);
+ srcBitmap = bitmap;
+ }
+
+ @Override
+ @Nullable
+ public PointF performTask() {
+ // boost this thread's priority a bit
+ Thread.currentThread().setPriority(Thread.MAX_PRIORITY - 1);
+ long millis = System.currentTimeMillis();
+ // create a new bitmap onto which we'll draw the original bitmap,
+ // because the FaceDetector requires it to be a 565 bitmap, and it
+ // must also be even width. Reduce size of copy for performance.
+ Bitmap testBmp = new565ScaledBitmap(srcBitmap);
+
+ // initialize the face detector, and look for only one face...
+ FaceDetector fd = new FaceDetector(testBmp.getWidth(),
testBmp.getHeight(), 1);
+ FaceDetector.Face[] faces = new FaceDetector.Face[1];
+ int numFound = fd.findFaces(testBmp, faces);
+
+ PointF facePos = null;
+ if (numFound > 0) {
+ facePos = new PointF();
+ faces[0].getMidPoint(facePos);
+ // scale back to proportions of original image
+ facePos.x = (facePos.x * srcBitmap.getWidth() /
BITMAP_COPY_WIDTH);
+ facePos.y = (facePos.y * srcBitmap.getHeight() /
testBmp.getHeight());
+ L.d("Found face at " + facePos.x + ", " + facePos.y);
+ }
+ // free our temporary bitmap
+ testBmp.recycle();
+
+ L.d("Face detection took " + (System.currentTimeMillis() - millis)
+ "ms");
+
+ return facePos;
+ }
+
+ @NonNull private Bitmap new565ScaledBitmap(Bitmap src) {
+ Bitmap copy = Bitmap.createBitmap(BITMAP_COPY_WIDTH,
+ (src.getHeight() * BITMAP_COPY_WIDTH) / src.getWidth(),
Bitmap.Config.RGB_565);
+ Canvas canvas = new Canvas(copy);
+ Rect srcRect = new Rect(0, 0, src.getWidth(), src.getHeight());
+ Rect destRect = new Rect(0, 0, BITMAP_COPY_WIDTH,
copy.getHeight());
+ Paint paint = new Paint();
+ paint.setColor(Color.BLACK);
+ canvas.drawBitmap(src, srcRect, destRect, paint);
+ return copy;
}
}
- @Override
- public void onPrepareLoad(Drawable placeHolderDrawable) {
- }
-
-
- public interface OnImageLoadListener {
- void onImageLoaded(Bitmap bitmap, PointF faceLocation);
- void onImageFailed();
- }
-
- private OnImageLoadListener onImageLoadListener;
-
- public void setOnImageLoadListener(OnImageLoadListener listener) {
- onImageLoadListener = listener;
+ private static class DefaultListener implements OnImageLoadListener {
+ @Override public void onImageLoaded(Bitmap bitmap, PointF
faceLocation) { }
+ @Override public void onImageFailed() { }
}
}
\ No newline at end of file
diff --git
a/app/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java
b/app/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java
index ea15954..d7f43ae 100755
--- a/app/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java
+++ b/app/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java
@@ -2,8 +2,8 @@
import android.content.res.Resources;
import android.graphics.Bitmap;
-import android.graphics.Canvas;
import android.graphics.Color;
+import android.graphics.Typeface;
import android.graphics.drawable.Drawable;
import android.support.annotation.ColorInt;
import android.support.annotation.ColorRes;
@@ -130,6 +130,7 @@
findViewsById(imageContainer);
pageTitleGradient =
GradientUtil.getCubicGradient(getColor(R.color.lead_gradient_start),
Gravity.BOTTOM);
+ pageTitleText.setTypeface(Typeface.create(Typeface.SERIF,
Typeface.NORMAL));
initDisplayDimensions();
@@ -152,26 +153,12 @@
imageContainer.setVisibility(View.INVISIBLE);
}
- @Nullable
- public Bitmap getLeadImageBitmap() {
- return leadImagesEnabled ? getBitmapFromView(image) : null;
+ @Nullable public Bitmap getLeadImageBitmap() {
+ return image.getImageBitmap();
}
public boolean isLeadImageEnabled() {
return leadImagesEnabled;
- }
-
- // ideas from:
- //
http://stackoverflow.com/questions/2801116/converting-a-view-to-bitmap-without-displaying-it-in-android
- // View has to be already displayed
- private Bitmap getBitmapFromView(ImageView view) {
- // Define a bitmap with the same size as the view
- Bitmap returnedBitmap
- = Bitmap.createBitmap(view.getWidth(), view.getHeight(),
Bitmap.Config.ARGB_8888);
- // Bind a canvas to it
- Canvas canvas = new Canvas(returnedBitmap);
- view.draw(canvas);
- return returnedBitmap;
}
/**
@@ -225,6 +212,8 @@
// mathematical diagrams or animations that won't look good as
a lead image.
// TODO: retrieve the MIME type of the lead image, instead of
relying on file name.
leadImagesEnabled = !thumbUrl.endsWith(".gif");
+
+ // TODO: what's the harm in always setting the background to
white unconditionally.
// also, if the image is not a JPG (i.e. it's a PNG or SVG)
and might have
// transparency, give it a white background.
if (!thumbUrl.endsWith(".jpg")) {
@@ -306,6 +295,7 @@
if (!isFragmentAdded()) {
return;
}
+
int titleContainerHeight;
if (isMainPage()) {
@@ -466,6 +456,57 @@
displayHeightDp = (int) (displayHeightPx / displayDensity);
}
+ private void detectFace(int bmpHeight, float aspect, @Nullable PointF
faceLocation) {
+ int newWidth = image.getWidth();
+ int newHeight = (int) (newWidth * aspect);
+
+ // give our image an offset based on the location of the face,
+ // relative to the image container
+ float scale = (float) newHeight / (float) bmpHeight;
+ if (faceLocation != null) {
+ int faceY = (int) (faceLocation.y * scale);
+ // if we have a face, then offset to the face location
+ imageBaseYOffset = -(faceY - (imagePlaceholder.getHeight() / 2));
+ // Adjust the face position by a slight amount.
+ // The face recognizer gives the location of the *eyes*, whereas
we actually
+ // want to center on the *nose*...
+ imageBaseYOffset +=
getDimension(R.dimen.face_detection_nose_y_offset);
+ faceYOffsetNormalized = faceLocation.y / bmpHeight;
+ } else {
+ // No face, so we'll just chop the top 25% off rather than
centering
+ final float oneQuarter = 0.25f;
+ imageBaseYOffset = -(int) ((newHeight -
imagePlaceholder.getHeight()) * oneQuarter);
+ faceYOffsetNormalized = oneQuarter;
+ }
+
+ // is the offset too far to the top?
+ imageBaseYOffset = Math.min(0, imageBaseYOffset);
+
+ // is the offset too far to the bottom?
+ imageBaseYOffset = Math.max(imageBaseYOffset,
imagePlaceholder.getHeight() - newHeight);
+
+ // resize our image to have the same proportions as the acquired bitmap
+ if (newHeight < imagePlaceholder.getHeight()) {
+ // if the height of the image is less than the container, then just
+ // make it the same height as the placeholder.
+ setImageLayoutParams(LinearLayout.LayoutParams.MATCH_PARENT,
imagePlaceholder.getHeight());
+ imageBaseYOffset = 0;
+ } else {
+ setImageLayoutParams(LinearLayout.LayoutParams.MATCH_PARENT,
newHeight);
+ }
+
+ forceRefreshWebView();
+
+ // fade in the new image!
+ ViewAnimations.crossFade(imagePlaceholder, image);
+
+ startKenBurnsAnimation();
+ }
+
+ private void setImageLayoutParams(int width, int height) {
+ image.setLayoutParams(new LinearLayout.LayoutParams(width, height));
+ }
+
private void loadLeadImage() {
loadLeadImage(getLeadImageUrl());
}
@@ -555,7 +596,6 @@
return parentFragment.isAdded();
}
-
private float getDimension(@DimenRes int id) {
return getResources().getDimension(id);
}
@@ -597,64 +637,15 @@
private class ImageLoadListener implements
ImageViewWithFace.OnImageLoadListener {
@Override
- public void onImageLoaded(Bitmap bitmap, final PointF faceLocation) {
+ public void onImageLoaded(Bitmap bitmap, @Nullable final PointF
faceLocation) {
final int bmpHeight = bitmap.getHeight();
final float aspect = (float) bitmap.getHeight() / (float)
bitmap.getWidth();
imageContainer.post(new Runnable() {
@Override
public void run() {
- if (!isFragmentAdded()) {
- return;
+ if (isFragmentAdded()) {
+ detectFace(bmpHeight, aspect, faceLocation);
}
- int newWidth = image.getWidth();
- int newHeight = (int) (newWidth * aspect);
-
- // give our image an offset based on the location of the
face,
- // relative to the image container
- float scale = (float) newHeight / (float) bmpHeight;
- if (faceLocation.y > 0.0f) {
- int faceY = (int) (faceLocation.y * scale);
- // if we have a face, then offset to the face location
- imageBaseYOffset = -(faceY -
(imagePlaceholder.getHeight() / 2));
- // Adjust the face position by a slight amount.
- // The face recognizer gives the location of the
*eyes*, whereas we actually
- // want to center on the *nose*...
- final int faceBoost = 24;
- imageBaseYOffset -= (faceBoost * displayDensity);
- faceYOffsetNormalized = faceLocation.y / bmpHeight;
- } else {
- // No face, so we'll just chop the top 25% off rather
than centering
- final float oneQuarter = 0.25f;
- imageBaseYOffset = -(int) ((newHeight -
imagePlaceholder.getHeight()) * oneQuarter);
- faceYOffsetNormalized = oneQuarter;
- }
-
- // is the offset too far to the top?
- imageBaseYOffset = Math.min(0, imageBaseYOffset);
-
- // is the offset too far to the bottom?
- imageBaseYOffset = Math.max(imageBaseYOffset,
imagePlaceholder.getHeight() - newHeight);
-
- // resize our image to have the same proportions as the
acquired bitmap
- if (newHeight < imagePlaceholder.getHeight()) {
- // if the height of the image is less than the
container, then just
- // make it the same height as the placeholder.
- image.setLayoutParams(
- new
LinearLayout.LayoutParams(LinearLayout.LayoutParams.MATCH_PARENT,
- imagePlaceholder.getHeight()));
- imageBaseYOffset = 0;
- } else {
- image.setLayoutParams(
- new
LinearLayout.LayoutParams(LinearLayout.LayoutParams.MATCH_PARENT,
- newHeight));
- }
-
- forceRefreshWebView();
-
- // fade in the new image!
- ViewAnimations.crossFade(imagePlaceholder, image);
-
- startKenBurnsAnimation();
}
});
}
diff --git a/app/src/main/res/layout/fragment_page.xml
b/app/src/main/res/layout/fragment_page.xml
index 2f25146..ffe3c46 100644
--- a/app/src/main/res/layout/fragment_page.xml
+++ b/app/src/main/res/layout/fragment_page.xml
@@ -67,7 +67,6 @@
android:layout_width="match_parent"
android:layout_height="wrap_content"
style="@style/RtlAwareTextView"
- android:fontFamily="serif"
android:paddingTop="16dp"
android:paddingRight="@dimen/activity_horizontal_margin"
android:paddingLeft="@dimen/activity_horizontal_margin"/>
@@ -78,7 +77,6 @@
style="@style/RtlAwareTextView"
android:layout_gravity="bottom"
android:visibility="invisible"
- android:fontFamily="serif"
android:textSize="@dimen/descriptionTextSize"
android:paddingBottom="16dp"
android:paddingRight="@dimen/activity_horizontal_margin"
diff --git a/app/src/main/res/values/dimens.xml
b/app/src/main/res/values/dimens.xml
index 90de7fe..e2f72c8 100644
--- a/app/src/main/res/values/dimens.xml
+++ b/app/src/main/res/values/dimens.xml
@@ -54,4 +54,6 @@
<dimen name="floating_action_button_margin_right">8dp</dimen>
<dimen name="floating_action_button_margin_bottom">0dp</dimen>
-</resources>
+ <dimen name="face_detection_nose_y_offset">-24dp</dimen>
+
+</resources>
\ No newline at end of file
--
To view, visit https://gerrit.wikimedia.org/r/239314
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibc793ab778691b9a2d182527e14a6b9fb2703ef3
Gerrit-PatchSet: 2
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Niedzielski <[email protected]>
Gerrit-Reviewer: Dbrant <[email protected]>
Gerrit-Reviewer: Niedzielski <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
