ScreamingHawk / android-slideshow

A simple slideshow application for your mobile.
MIT License
44 stars 44 forks source link

Issue with Exif tags, glide strategy and Auto #135

Open mettyw opened 5 years ago

mettyw commented 5 years ago

It seems to me that AUTO_ROTATE_DIMEN only works if device in portrait mode. My understanding would be, that when this option is selected, the image always fills the maximum amount of screen space, no matter in what orientation the device currently is?

Based on that I have the following suggestions:

1) I think within GlideRotateDimenTransformation there must be another if statement checking whether toTransform.getHeight() >= toTransform.getWidth() and both cases must be handled separately.

2) Related to the above, I sometimes saw weird orientations of images when rotating the device during slideshow display. I think GlideRotateDimenTransformation.getID() then must be "more" unique because the Id is used for caching according to the Javadoc. If it is cached, the transform(..) method will not be called again and the need for transformation will not be calculated again based on the current rotation.

mettyw commented 5 years ago

The following code seems to fix the issues I had, but I am not sure whether this is a proper fix, especially the Math.random() part in getId() ...

@Override
protected Bitmap transform(BitmapPool pool, Bitmap toTransform, int outWidth, int outHeight) {
    Log.d(TAG, String.format("Height: %d Width: %d", toTransform.getHeight(), toTransform.getWidth()));
    if ( outWidth <= outHeight) {
        if (toTransform.getHeight() >= toTransform.getWidth()){
            // Perform fit center here on un-rotated image.
            toTransform = TransformationUtils.fitCenter(toTransform, pool, outWidth, outHeight);
            return toTransform;
        } else {
            // Fit center using largest side (width) for both to reduce computation for rotate
            //noinspection SuspiciousNameCombination
            toTransform = TransformationUtils.fitCenter(toTransform, pool, outWidth, outWidth);
            return TransformationUtils.rotateImage(toTransform, 90);
        }
    }
    else {
        // note: width and height are switched in this if statement
        if (toTransform.getWidth() >= toTransform.getHeight()){
            // Perform fit center here on un-rotated image.
            toTransform = TransformationUtils.fitCenter(toTransform, pool, outWidth, outHeight);
            return toTransform;
        } else {
            // Fit center using largest side (height) for both to reduce computation for rotate
            //noinspection SuspiciousNameCombination
            toTransform = TransformationUtils.fitCenter(toTransform, pool, outHeight, outHeight);
            return TransformationUtils.rotateImage(toTransform, 90);
        }
    }
}

@Override
public String getId() {
    return TAG + Math.random(); //FIXME
}
ScreamingHawk commented 5 years ago

Optimised:

@Override
protected Bitmap transform(BitmapPool pool, Bitmap toTransform, int outWidth, int outHeight) {
    Log.d(TAG, String.format("Height: %d Width: %d", toTransform.getHeight(), toTransform.getWidth()));
    boolean largerHeight = outHeight > outWidth;
    if ((largerHeight && toTransform.getHeight() >= toTransform.getWidth()) ||
            (!largerHeight && toTransform.getWidth() >= toTransform.getHeight())){
        // Perform fit center here on un-rotated image.
        toTransform = TransformationUtils.fitCenter(toTransform, pool, outWidth, outHeight);
        return toTransform;
    } else {
        int largest = largerHeight ? outHeight : outWidth;
        // Fit center using largest side for both to reduce computation for rotate
        //noinspection SuspiciousNameCombination
        toTransform = TransformationUtils.fitCenter(toTransform, pool, largest, largest);
        return TransformationUtils.rotateImage(toTransform, 90);
    }
}

The Id just has to be unique, so Math.random is fine.

This seems like a good approach, so let's go with it.