Reland of Android: support multiple displays on C++ side

ui/android/java/src/org/chromium/ui/display/DisplayAndroid.java

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.ui.display;
 
 import android.annotation.TargetApi;
 import android.content.Context;
+import android.graphics.PixelFormat;
 import android.graphics.Point;
 import android.os.Build;
 import android.util.DisplayMetrics;
 import android.view.Display;
+import android.view.Surface;
 
 import org.chromium.base.CommandLine;
 import org.chromium.base.Log;
+import org.chromium.base.annotations.JNINamespace;
 
 import java.util.WeakHashMap;
 
 /**
  * Chromium's object for android.view.Display. Instances of this object should be obtained
  * from WindowAndroid.
  * This class is designed to avoid leaks. It is ok to hold a strong ref of this class from
  * anywhere, as long as the corresponding WindowAndroids are destroyed. The observers are
  * held weakly so to not lead to leaks.
  */
+@JNINamespace("ui")

[mthiesse, 2016/10/26 14:49:49]

I think the native code for Displays on android should be in
DisplayAndroidManager. I think DisplayAndroid should be equivalent to the native
Display class that's usually passed by value and stay as lightweight as
possible.

[boliu, 2016/10/26 17:24:20]

Pass by value is not really a thing in java, but yeah, java DisplayAndroid
should be as light weight as possible. That doesn't mean it can't talk to native
things. As long as the common accessors don't do jni, it's just as light weight
as before.

 public class DisplayAndroid {
     /**
      * DisplayAndroidObserver interface for changes to this Display.
      */
     public interface DisplayAndroidObserver {
         /**
          * Called whenever the screen orientation changes.
          *
          * @param orientation One of Surface.ROTATION_* values.
          */
         void onRotationChanged(int rotation);
     }
 
     private static final String TAG = "DisplayAndroid";
 
     private static final DisplayAndroidObserver[] EMPTY_OBSERVER_ARRAY =
             new DisplayAndroidObserver[0];
 
     private final int mSdkDisplayId;
     private final WeakHashMap<DisplayAndroidObserver, Object /* null */> mObservers;
     // Do NOT add strong references to objects with potentially complex lifetime, like Context.
 
     // Updated by updateFromDisplay.
     private final Point mSize;
     private final Point mPhysicalSize;
     private final DisplayMetrics mDisplayMetrics;
+    private int mPixelFormatId;
     private int mRotation;
 
     // When this object exists, a positive value means that the forced DIP scale is set and
     // the zero means it is not. The non existing object (i.e. null reference) means that
     // the existence and value of the forced DIP scale has not yet been determined.
     private static Float sForcedDIPScale;
 
     private static boolean hasForcedDIPScale() {
         if (sForcedDIPScale == null) {
             String forcedScaleAsString = CommandLine.getInstance().getSwitchValue(
@@ skipping 28 matching lines @@
      * Get the DisplayAndroid for this context. It's safe to call this with any type of context
      * including the Application. However to support multi-display, prefer to use the Activity
      * context if available, or obtain DisplayAndroid from WindowAndroid instead.
      */
     public static DisplayAndroid get(Context context) {
         Display display = DisplayAndroidManager.getDisplayFromContext(context);
         return getManager().getDisplayAndroid(display);
     }
 
     /**
+     * @return Display id as defined in Android's Display.
+     */
+    public int getSdkDisplayId() {

[mthiesse, 2016/10/26 14:49:49]

If we store a mapping from window to display in both java and native
DisplayAndroidManager, we don't need to expose DisplayIds anywhere. window would
just call into DisplayAndroidManager to get the display for itself.

[boliu, 2016/10/26 17:24:20]

yeah, this is one of those weird things where we really wish to expose this to
WindowAndroid only, not the world. But maybe a comment, or
getSdkDisplayIdInternal would be good enough here.

I think I'd rather not have native side have an explicit window -> display
mapping because feels like keeping that mapping up-to-date is a lot of work.
Unless you see some advantage?

[mthiesse, 2016/10/26 17:52:49]

The main benefit is consistency between native and java sides. It's weird that
WindowAndroid java stores a cached DisplayAndroid, but native stores an ID and
gets the display from somewhere else. I'm okay with them both just storing an ID
and fetching the display from the Manager, but that requires exposing a
getDisplayById-style function in DisplayAndroidManager, which you seemed to be
against. Also public methods with names like getSdkDisplayIdInternal kind of
suck :(

I don't see storing both a native and java window->display map as being
relevantly more work than the implicit mapping done by passing a display ID
around, and it makes things much easier to reason about. Both maps would be
updated only by DisplayAndroidManager,
DisplayAndroidManager::SetDisplayForWindow would just call
nativeSetDisplayForWindow, etc.

[boliu, 2016/10/26 18:24:43]

Ok I objected to that because the current thing seems cleaner, but the
inconsistency does seem worse. How about this. Let's assume we've nailed down
the concept of the primary display, which we have to guarantee (at least in
code) that it never goes away. Then on both the java and native side, if the
display the window was referring to goes away, it just falls back to primary
one.

That still exposes display id to WindowAndroid though. I don't think it's really
necessary to hide it since it's visible from native side already. And we still
don't need a window->display map anywhere :p

[mthiesse, 2016/10/26 18:48:24]

sgtm

[Tima Vaisburd, 2016/10/27 07:55:58]

Done, except for Java side.

+        return mSdkDisplayId;
+    }
+
+    /**
      * @return Display height in physical pixels.
      */
     public int getDisplayHeight() {
         return mSize.y;
     }
 
     /**
      * @return Display width in physical pixels.
      */
     public int getDisplayWidth() {
@@ skipping 15 matching lines @@
     }
 
     /**
      * @return current orientation. One of Surface.ORIENTATION_* values.
      */
     public int getRotation() {
         return mRotation;
     }
 
     /**
+     * @return current orientation in degrees. One of the values 0, 90, 180, 270.
+     */
+    public int getRotationDegrees() {
+        switch (mRotation) {
+            case Surface.ROTATION_0:
+                return 0;
+            case Surface.ROTATION_90:
+                return 90;
+            case Surface.ROTATION_180:
+                return 180;
+            case Surface.ROTATION_270:
+                return 270;
+        }
+
+        // This should not happen.
+        assert false;
+        return 0;
+    }
+
+    /**
      * @return A scaling factor for the Density Independent Pixel unit.
      */
     public float getDIPScale() {
         return mDisplayMetrics.density;
     }
 
     /**
+     * @return Number of bits per pixel.
+     */
+    public int getBitsPerPixel() {
+        PixelFormat info = new PixelFormat();
+        PixelFormat.getPixelFormatInfo(mPixelFormatId, info);
+        return info.bitsPerPixel;
+    }
+
+    /**
+     * @return Number of bits per each color component.
+     */
+    @SuppressWarnings("deprecation")
+    public int getBitsPerComponent() {
+        switch (mPixelFormatId) {
+            case PixelFormat.RGBA_4444:
+                return 4;
+
+            case PixelFormat.RGBA_5551:
+                return 5;
+
+            case PixelFormat.RGBA_8888:
+            case PixelFormat.RGBX_8888:
+            case PixelFormat.RGB_888:
+                return 8;
+
+            case PixelFormat.RGB_332:
+                return 2;
+
+            case PixelFormat.RGB_565:
+                return 5;
+
+            // Non-RGB formats.
+            case PixelFormat.A_8:
+            case PixelFormat.LA_88:
+            case PixelFormat.L_8:
+                return 0;
+
+            // Unknown format. Use 8 as a sensible default.
+            default:
+                return 8;
+        }
+    }
+
+    /**
      * Add observer. Note repeat observers will be called only one.
      * Observers are held only weakly by Display.
      */
     public void addObserver(DisplayAndroidObserver observer) {
         mObservers.put(observer, null);
     }
 
     /**
      * Remove observer.
      */
@@ skipping 16 matching lines @@
     public static void stopAccurateListening() {
         getManager().startAccurateListening();
     }
 
     /* package */ DisplayAndroid(Display display) {
         mSdkDisplayId = display.getDisplayId();
         mObservers = new WeakHashMap<>();
         mSize = new Point();
         mPhysicalSize = new Point();
         mDisplayMetrics = new DisplayMetrics();
+        mPixelFormatId = PixelFormat.RGBA_8888;
         updateFromDisplay(display);
     }
 
+    @SuppressWarnings("deprecation")
     @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR1)
     /* package */ void updateFromDisplay(Display display) {
+        final Point oldSize = new Point(mSize);
+        final Point oldPhysicalSize = new Point(mPhysicalSize);
+        final float oldDensity = mDisplayMetrics.density;
+        final int oldPixelFormatId = mPixelFormatId;
+        final int oldRotation = mRotation;
+
         display.getSize(mSize);
         display.getMetrics(mDisplayMetrics);
 
         if (hasForcedDIPScale()) mDisplayMetrics.density = sForcedDIPScale.floatValue();
 
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR1) {
             display.getRealSize(mPhysicalSize);
         }
 
-        int newRotation = display.getRotation();
-        boolean rotationChanged = newRotation != mRotation;
-        mRotation = newRotation;
+        // JellyBean MR1 and later always uses RGBA_8888.
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1) {
+            mPixelFormatId = display.getPixelFormat();
+        }
 
-        if (rotationChanged) {
+        mRotation = display.getRotation();
+
+        final boolean noChanges = oldSize.equals(mSize) && oldPhysicalSize.equals(mPhysicalSize)
+                && oldDensity == mDisplayMetrics.density && oldPixelFormatId == mPixelFormatId
+                && oldRotation == mRotation;
+        if (noChanges) return;
+
+        updateNativeSide();
+
+        if (oldRotation != mRotation) {
             DisplayAndroidObserver[] observers = getObservers();
             for (DisplayAndroidObserver o : observers) {
                 o.onRotationChanged(mRotation);
             }
         }
     }
 
+    /* package */ void onAddedToMap() {
+        updateNativeSide();
+    }
+
+    /* package */ void onRemovedFromMap() {
+        nativeRemoveDisplayAndroid(mSdkDisplayId);
+    }
+
+    private void updateNativeSide() {
+        nativeUpdateDisplayAndroid(mSdkDisplayId, mPhysicalSize.x, mPhysicalSize.y, mSize.x,
+                mSize.y, mDisplayMetrics.density, getRotationDegrees(), getBitsPerPixel(),
+                getBitsPerComponent());
+    }
+
     private DisplayAndroidObserver[] getObservers() {
         // Makes a copy to allow concurrent edit.
         return mObservers.keySet().toArray(EMPTY_OBSERVER_ARRAY);
     }
+
+    private static native void nativeUpdateDisplayAndroid(int sdkDisplayId, int physicalWidth,
+            int physicalHeight, int width, int height, float dipScale, int rotationDegrees,
+            int bitsPerPixel, int bitsPerComponent);
+    private static native void nativeRemoveDisplayAndroid(int sdkDisplayId);
 }