From 8864415e2a29daf504b52afe911eb5c5a8b03fc0 Mon Sep 17 00:00:00 2001
From: Jeff Brown <jeffbrown@google.com>
Date: Fri, 22 Aug 2014 03:42:47 -0700
Subject: Fix matrix multiply in accessiblity display adjustments.

Fixed a bug caused by a missing temporary when swapping two
matrices.  This could produce undefined results in the case where
both display inversion and the greyscale daltonizer were enabled
simultaneously.

Tightened up the code a little to make this kind of error
less likely by removing redundant state.

Removed the native prefix on some methods since they are not
native at all.

Change-Id: I716ffc8fbe76a304c60d45870074340c0121059f
---
 .../accessibility/DisplayAdjustmentUtils.java      | 69 +++++++++-------------
 1 file changed, 29 insertions(+), 40 deletions(-)

(limited to 'services')

diff --git a/services/accessibility/java/com/android/server/accessibility/DisplayAdjustmentUtils.java b/services/accessibility/java/com/android/server/accessibility/DisplayAdjustmentUtils.java
index 394c196..38827d0 100644
--- a/services/accessibility/java/com/android/server/accessibility/DisplayAdjustmentUtils.java
+++ b/services/accessibility/java/com/android/server/accessibility/DisplayAdjustmentUtils.java
@@ -60,15 +60,17 @@ class DisplayAdjustmentUtils {
     public static boolean hasAdjustments(Context context, int userId) {
         final ContentResolver cr = context.getContentResolver();
 
-        boolean hasColorTransform = Settings.Secure.getIntForUser(
-                cr, Settings.Secure.ACCESSIBILITY_DISPLAY_INVERSION_ENABLED, 0, userId) == 1;
+        if (Settings.Secure.getIntForUser(cr,
+                Settings.Secure.ACCESSIBILITY_DISPLAY_INVERSION_ENABLED, 0, userId) != 0) {
+            return true;
+        }
 
-        if (!hasColorTransform) {
-            hasColorTransform |= Settings.Secure.getIntForUser(
-                cr, Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER_ENABLED, 0, userId) == 1;
+        if (Settings.Secure.getIntForUser(cr,
+                Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER_ENABLED, 0, userId) != 0) {
+            return true;
         }
 
-        return hasColorTransform;
+        return false;
     }
 
     /**
@@ -76,52 +78,39 @@ class DisplayAdjustmentUtils {
      */
     public static void applyAdjustments(Context context, int userId) {
         final ContentResolver cr = context.getContentResolver();
-        float[] colorMatrix = new float[16];
-        float[] outputMatrix = new float[16];
-        boolean hasColorTransform = false;
-
-        Matrix.setIdentityM(colorMatrix, 0);
-
-        final boolean inversionEnabled = Settings.Secure.getIntForUser(
-                cr, Settings.Secure.ACCESSIBILITY_DISPLAY_INVERSION_ENABLED, 0, userId) == 1;
-        if (inversionEnabled) {
-            final float[] inversionMatrix = INVERSION_MATRIX_VALUE_ONLY;
-            Matrix.multiplyMM(outputMatrix, 0, colorMatrix, 0, inversionMatrix, 0);
+        float[] colorMatrix = null;
 
-            colorMatrix = outputMatrix;
-            outputMatrix = colorMatrix;
-
-            hasColorTransform = true;
+        if (Settings.Secure.getIntForUser(cr,
+                Settings.Secure.ACCESSIBILITY_DISPLAY_INVERSION_ENABLED, 0, userId) != 0) {
+            colorMatrix = multiply(colorMatrix, INVERSION_MATRIX_VALUE_ONLY);
         }
 
-        final boolean daltonizerEnabled = Settings.Secure.getIntForUser(
-                cr, Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER_ENABLED, 0, userId) != 0;
-        if (daltonizerEnabled) {
+        if (Settings.Secure.getIntForUser(cr,
+                Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER_ENABLED, 0, userId) != 0) {
             final int daltonizerMode = Settings.Secure.getIntForUser(cr,
                     Settings.Secure.ACCESSIBILITY_DISPLAY_DALTONIZER, DEFAULT_DISPLAY_DALTONIZER,
                     userId);
             // Monochromacy isn't supported by the native Daltonizer.
             if (daltonizerMode == AccessibilityManager.DALTONIZER_SIMULATE_MONOCHROMACY) {
-                Matrix.multiplyMM(outputMatrix, 0, colorMatrix, 0, GRAYSCALE_MATRIX, 0);
-
-                final float[] temp = colorMatrix;
-                colorMatrix = outputMatrix;
-                outputMatrix = temp;
-
-                hasColorTransform = true;
-                nativeSetDaltonizerMode(AccessibilityManager.DALTONIZER_DISABLED);
+                colorMatrix = multiply(colorMatrix, GRAYSCALE_MATRIX);
+                setDaltonizerMode(AccessibilityManager.DALTONIZER_DISABLED);
             } else {
-                nativeSetDaltonizerMode(daltonizerMode);
+                setDaltonizerMode(daltonizerMode);
             }
         } else {
-            nativeSetDaltonizerMode(AccessibilityManager.DALTONIZER_DISABLED);
+            setDaltonizerMode(AccessibilityManager.DALTONIZER_DISABLED);
         }
 
-        if (hasColorTransform) {
-            nativeSetColorTransform(colorMatrix);
-        } else {
-            nativeSetColorTransform(null);
+        setColorTransform(colorMatrix);
+    }
+
+    private static float[] multiply(float[] matrix, float[] other) {
+        if (matrix == null) {
+            return other;
         }
+        float[] result = new float[16];
+        Matrix.multiplyMM(result, 0, matrix, 0, other, 0);
+        return result;
     }
 
     /**
@@ -130,7 +119,7 @@ class DisplayAdjustmentUtils {
      *
      * @param mode new Daltonization mode
      */
-    private static void nativeSetDaltonizerMode(int mode) {
+    private static void setDaltonizerMode(int mode) {
         try {
             final IBinder flinger = ServiceManager.getService("SurfaceFlinger");
             if (flinger != null) {
@@ -152,7 +141,7 @@ class DisplayAdjustmentUtils {
      * @param m the float array that holds the transformation matrix, or null to
      *            disable transformation
      */
-    private static void nativeSetColorTransform(float[] m) {
+    private static void setColorTransform(float[] m) {
         try {
             final IBinder flinger = ServiceManager.getService("SurfaceFlinger");
             if (flinger != null) {
-- 
cgit v1.1