Get rid of Key.setIcon(Drawable)

Change-Id: If6ebbdefcbfd0cc14c72858f9bb44861e8abe079

java/res/values/attrs.xml

@@ -281,7 +281,7 @@
         </attr>
         <!-- The icon to display on the key instead of the label. -->
         <attr name="keyIcon" format="enum">
-            <!-- This should be aligned with the KeyboardIcons.ICONS_TO_ATTRS_MAP -->
+            <!-- This should be aligned with the KeyboardIconsSet.ICON_* -->
             <enum name="iconShiftKey" value="1" />
             <enum name="iconDeleteKey" value="2" />
             <enum name="iconSettingsKey" value="3" />
@@ -296,12 +296,12 @@
         </attr>
         <!-- The icon for disabled key -->
         <attr name="keyIconDisabled" format="enum">
-            <!-- This should be aligned with the KeyboardIcons.ICONS_TO_ATTRS_MAP -->
+            <!-- This should be aligned with the KeyboardIconsSet.ICON_* -->
             <enum name="iconDisabledShortcutKey" value="12" />
         </attr>
         <!-- The icon to show in the popup preview. -->
         <attr name="keyIconPreview" format="enum">
-            <!-- This should be aligned with the KeyboardIcons.ICONS_TO_ATTRS_MAP -->
+            <!-- This should be aligned with the KeyboardIconsSet.ICON_* -->
             <enum name="iconPreviewTabKey" value="13" />
         </attr>
         <!-- The key style to specify a set of key attributes defined by <key_style/> -->

java/src/com/android/inputmethod/keyboard/Key.java

@@ -25,10 +25,10 @@ import android.text.TextUtils;
 import android.util.Log;
 import android.util.Xml;
 
+import com.android.inputmethod.keyboard.internal.KeySpecParser;
 import com.android.inputmethod.keyboard.internal.KeyStyles;
 import com.android.inputmethod.keyboard.internal.KeyStyles.KeyStyle;
 import com.android.inputmethod.keyboard.internal.KeyboardIconsSet;
-import com.android.inputmethod.keyboard.internal.KeySpecParser;
 import com.android.inputmethod.latin.R;
 import com.android.inputmethod.latin.Utils;
 import com.android.inputmethod.latin.XmlParseUtils;
@@ -74,13 +74,11 @@ public class Key {
     private static final int LABEL_FLAGS_SHIFTED_LETTER_ACTIVATED = 0x10000;
 
     /** Icon to display instead of a label. Icon takes precedence over a label */
-    private final int mIconAttrId;
+    private final int mIconId;
-    // TODO: Remove this variable.
-    private Drawable mIcon;
     /** Icon for disabled state */
-    private final int mDisabledIconAttrId;
+    private final int mDisabledIconId;
     /** Preview version of the icon, for the preview popup */
-    public final int mPreviewIconAttrId;
+    private final int mPreviewIconId;
 
     /** Width of the key, not including the gap */
     public final int mWidth;
@@ -128,21 +126,13 @@ public class Key {
     /** Key is enabled and responds on press */
     private boolean mEnabled = true;
 
-    private static Drawable getIcon(Keyboard.Params params, String moreKeySpec) {
-        final int iconAttrId = KeySpecParser.getIconAttrId(moreKeySpec);
-        if (iconAttrId == KeyboardIconsSet.ICON_UNDEFINED) {
-            return null;
-        } else {
-            return params.mIconsSet.getIconByAttrId(iconAttrId);
-        }
-    }
-
     /**
      * This constructor is being used only for key in more keys keyboard.
      */
     public Key(Resources res, Keyboard.Params params, String moreKeySpec,
             int x, int y, int width, int height) {
-        this(params, KeySpecParser.getLabel(moreKeySpec), null, getIcon(params, moreKeySpec),
+        this(params, KeySpecParser.getLabel(moreKeySpec), null,
+                KeySpecParser.getIconId(moreKeySpec),
                 KeySpecParser.getCode(res, moreKeySpec),
                 KeySpecParser.getOutputText(moreKeySpec),
                 x, y, width, height);
@@ -151,7 +141,7 @@ public class Key {
     /**
      * This constructor is being used only for key in popup suggestions pane.
      */
-    public Key(Keyboard.Params params, String label, String hintLabel, Drawable icon,
+    public Key(Keyboard.Params params, String label, String hintLabel, int iconId,
             int code, CharSequence outputText, int x, int y, int width, int height) {
         mHeight = height - params.mVerticalGap;
         mHorizontalGap = params.mHorizontalGap;
@@ -168,10 +158,9 @@ public class Key {
         mOutputText = outputText;
         mCode = code;
         mAltCode = Keyboard.CODE_UNSPECIFIED;
-        mIconAttrId = KeyboardIconsSet.ATTR_UNDEFINED;
+        mIconId = iconId;
-        mIcon = icon;
+        mDisabledIconId = KeyboardIconsSet.ICON_UNDEFINED;
-        mDisabledIconAttrId = KeyboardIconsSet.ATTR_UNDEFINED;
+        mPreviewIconId = KeyboardIconsSet.ICON_UNDEFINED;
-        mPreviewIconAttrId = KeyboardIconsSet.ATTR_UNDEFINED;
         // Horizontal gap is divided equally to both sides of the key.
         mX = x + mHorizontalGap / 2;
         mY = y;
@@ -228,18 +217,16 @@ public class Key {
         mBackgroundType = style.getInt(keyAttr,
                 R.styleable.Keyboard_Key_backgroundType, BACKGROUND_TYPE_NORMAL);
 
-        final KeyboardIconsSet iconsSet = params.mIconsSet;
         mVisualInsetsLeft = (int) Keyboard.Builder.getDimensionOrFraction(keyAttr,
                 R.styleable.Keyboard_Key_visualInsetsLeft, params.mBaseWidth, 0);
         mVisualInsetsRight = (int) Keyboard.Builder.getDimensionOrFraction(keyAttr,
                 R.styleable.Keyboard_Key_visualInsetsRight, params.mBaseWidth, 0);
-        mPreviewIconAttrId = KeyboardIconsSet.getIconAttrId(style.getInt(keyAttr,
+        mPreviewIconId = style.getInt(keyAttr,
-                R.styleable.Keyboard_Key_keyIconPreview, KeyboardIconsSet.ICON_UNDEFINED));
+                R.styleable.Keyboard_Key_keyIconPreview, KeyboardIconsSet.ICON_UNDEFINED);
-        mIconAttrId = KeyboardIconsSet.getIconAttrId(style.getInt(keyAttr,
+        mIconId = style.getInt(keyAttr,
-                R.styleable.Keyboard_Key_keyIcon, KeyboardIconsSet.ICON_UNDEFINED));
+                R.styleable.Keyboard_Key_keyIcon, KeyboardIconsSet.ICON_UNDEFINED);
-        mIcon = iconsSet.getIconByAttrId(mIconAttrId);
+        mDisabledIconId = style.getInt(keyAttr,
-        mDisabledIconAttrId = KeyboardIconsSet.getIconAttrId(style.getInt(keyAttr,
+                R.styleable.Keyboard_Key_keyIconDisabled, KeyboardIconsSet.ICON_UNDEFINED);
-                R.styleable.Keyboard_Key_keyIconDisabled, KeyboardIconsSet.ICON_UNDEFINED));
 
         mLabelFlags = style.getFlag(keyAttr, R.styleable.Keyboard_Key_keyLabelFlags, 0);
         final boolean preserveCase = (mLabelFlags & LABEL_FLAGS_PRESERVE_CASE) != 0;
@@ -336,16 +323,15 @@ public class Key {
                 key.mCode,
                 key.mLabel,
                 key.mHintLabel,
-                key.mIconAttrId,
+                key.mIconId,
                 key.mBackgroundType,
                 // Key can be distinguishable without the following members.
                 // key.mAltCode,
                 // key.mOutputText,
                 // key.mActionFlags,
                 // key.mLabelFlags,
-                // key.mIcon,
+                // key.mDisabledIconId,
-                // key.mDisabledIconAttrId,
+                // key.mPreviewIconId,
-                // key.mPreviewIconAttrId,
                 // key.mHorizontalGap,
                 // key.mVerticalGap,
                 // key.mVisualInsetLeft,
@@ -364,7 +350,7 @@ public class Key {
                 && o.mCode == mCode
                 && TextUtils.equals(o.mLabel, mLabel)
                 && TextUtils.equals(o.mHintLabel, mHintLabel)
-                && o.mIconAttrId == mIconAttrId
+                && o.mIconId == mIconId
                 && o.mBackgroundType == mBackgroundType;
     }
 
@@ -382,7 +368,7 @@ public class Key {
     public String toString() {
         return String.format("%s/%s %d,%d %dx%d %s/%s/%s",
                 Keyboard.printableCode(mCode), mLabel, mX, mY, mWidth, mHeight, mHintLabel,
-                KeyboardIconsSet.getIconName(mIconAttrId), backgroundName(mBackgroundType));
+                KeyboardIconsSet.getIconName(mIconId), backgroundName(mBackgroundType));
     }
 
     private static String backgroundName(int backgroundType) {
@@ -412,8 +398,8 @@ public class Key {
         mHitBox.bottom = params.mOccupiedHeight + params.mBottomPadding;
     }
 
-    public boolean isSpacer() {
+    public final boolean isSpacer() {
-        return false;
+        return this instanceof Spacer;
     }
 
     public boolean isShift() {
@@ -507,14 +493,18 @@ public class Key {
         return (mLabelFlags & LABEL_FLAGS_SHIFTED_LETTER_ACTIVATED) != 0;
     }
 
-    // TODO: Get rid of this method.
     public Drawable getIcon(KeyboardIconsSet iconSet) {
-        return mEnabled ? mIcon : iconSet.getIconByAttrId(mDisabledIconAttrId);
+        return iconSet.getIconDrawable(mIconId);
     }
 
-    // TODO: Get rid of this method.
+    public Drawable getDisabledIcon(KeyboardIconsSet iconSet) {
-    public void setIcon(Drawable icon) {
+        return iconSet.getIconDrawable(mDisabledIconId);
-        mIcon = icon;
+    }
+
+    public Drawable getPreviewIcon(KeyboardIconsSet iconSet) {
+        return mPreviewIconId != KeyboardIconsSet.ICON_UNDEFINED
+                ? iconSet.getIconDrawable(mPreviewIconId)
+                : iconSet.getIconDrawable(mIconId);
     }
 
     /**
@@ -651,13 +641,9 @@ public class Key {
         /**
          * This constructor is being used only for divider in more keys keyboard.
          */
-        public Spacer(Keyboard.Params params, Drawable icon, int x, int y, int width, int height) {
+        protected Spacer(Keyboard.Params params, int x, int y, int width, int height) {
-            super(params, null, null, icon, Keyboard.CODE_UNSPECIFIED, null, x, y, width, height);
+            super(params, null, null, KeyboardIconsSet.ICON_UNDEFINED, Keyboard.CODE_UNSPECIFIED,
-        }
+                    null, x, y, width, height);
-
-        @Override
-        public boolean isSpacer() {
-            return true;
         }
     }
 }

java/src/com/android/inputmethod/keyboard/KeyboardView.java

@@ -185,7 +185,7 @@ public class KeyboardView extends View implements PointerTracker.DrawingProxy {
         public final int mKeyShiftedLetterHintInactivatedColor;
         public final int mKeyShiftedLetterHintActivatedColor;
 
-        private final float mKeyLetterRatio;
+        /* package */ final float mKeyLetterRatio;
         private final float mKeyLargeLetterRatio;
         private final float mKeyLabelRatio;
         private final float mKeyHintLetterRatio;
@@ -486,8 +486,9 @@ public class KeyboardView extends View implements PointerTracker.DrawingProxy {
     }
 
     private void onDrawKey(Key key, Canvas canvas, Paint paint, KeyDrawParams params) {
-        if (key.isSpacer()) return;
+        if (!key.isSpacer()) {
             onDrawKeyBackground(key, canvas, params);
+        }
         onDrawKeyTopVisuals(key, canvas, paint, params);
     }
 
@@ -861,10 +862,8 @@ public class KeyboardView extends View implements PointerTracker.DrawingProxy {
             }
             previewText.setText(key.mLabel);
         } else {
-            final Drawable previewIcon = mKeyboard.mIconsSet.getIconByAttrId(
-                    key.mPreviewIconAttrId);
             previewText.setCompoundDrawables(null, null, null,
-                    previewIcon != null ? previewIcon : key.getIcon(mKeyboard.mIconsSet));
+                    key.getPreviewIcon(mKeyboard.mIconsSet));
             previewText.setText(null);
         }
         previewText.setBackgroundDrawable(params.mPreviewBackground);

java/src/com/android/inputmethod/keyboard/LatinKeyboardView.java

@@ -42,6 +42,7 @@ import com.android.inputmethod.accessibility.AccessibleKeyboardViewProxy;
 import com.android.inputmethod.deprecated.VoiceProxy;
 import com.android.inputmethod.keyboard.PointerTracker.DrawingProxy;
 import com.android.inputmethod.keyboard.PointerTracker.TimerProxy;
+import com.android.inputmethod.keyboard.internal.KeyboardIconsSet;
 import com.android.inputmethod.latin.LatinIME;
 import com.android.inputmethod.latin.LatinImeLogger;
 import com.android.inputmethod.latin.R;
@@ -390,7 +391,7 @@ public class LatinKeyboardView extends KeyboardView implements PointerTracker.Ke
         mMoreKeysPanelCache.clear();
 
         mSpaceKey = keyboard.getKey(Keyboard.CODE_SPACE);
-        mSpaceIcon = keyboard.mIconsSet.getIconByAttrId(R.styleable.Keyboard_iconSpaceKey);
+        mSpaceIcon = keyboard.mIconsSet.getIconDrawable(KeyboardIconsSet.ICON_SPACE);
         final int keyHeight = keyboard.mMostCommonKeyHeight - keyboard.mVerticalGap;
         mSpacebarTextSize = keyHeight * mSpacebarTextRatio;
         mSpacebarLocale = keyboard.mId.mLocale;

java/src/com/android/inputmethod/keyboard/internal/KeySpecParser.java

@@ -179,11 +179,11 @@ public class KeySpecParser {
         return Keyboard.CODE_OUTPUT_TEXT;
     }
 
-    public static int getIconAttrId(String moreKeySpec) {
+    public static int getIconId(String moreKeySpec) {
         if (hasIcon(moreKeySpec)) {
             final int end = moreKeySpec.indexOf(LABEL_END, PREFIX_ICON.length());
             final String name = moreKeySpec.substring(PREFIX_ICON.length(), end);
-            return KeyboardIconsSet.getIconAttrId(name);
+            return KeyboardIconsSet.getIconId(name);
         }
         return KeyboardIconsSet.ICON_UNDEFINED;
     }

java/src/com/android/inputmethod/keyboard/internal/KeyboardIconsSet.java

@@ -23,7 +23,6 @@ import android.util.Log;
 
 import com.android.inputmethod.latin.R;
 
-import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -31,17 +30,20 @@ public class KeyboardIconsSet {
     private static final String TAG = KeyboardIconsSet.class.getSimpleName();
 
     public static final int ICON_UNDEFINED = 0;
-    public static final int ATTR_UNDEFINED = 0;
+    // The value should be aligned with the enum value of Key.keyIcon.
+    public static final int ICON_SPACE = 4;
+    private static final int NUM_ICONS = 13;
 
-    private final Map<Integer, Drawable> mIcons = new HashMap<Integer, Drawable>();
+    private final Drawable[] mIcons = new Drawable[NUM_ICONS + 1];
 
-    // The key value should be aligned with the enum value of Keyboard.icon*.
+    private static final Map<Integer, Integer> ATTR_ID_TO_ICON_ID = new HashMap<Integer, Integer>();
-    private static final Map<Integer, Integer> ID_TO_ATTR_MAP = new HashMap<Integer, Integer>();
+    private static final Map<String, Integer> NAME_TO_ICON_ID = new HashMap<String, Integer>();
-    private static final Map<String, Integer> NAME_TO_ATTR_MAP = new HashMap<String, Integer>();
+    private static final String[] ICON_NAMES = new String[NUM_ICONS + 1];
-    private static final Map<Integer, String> ATTR_TO_NAME_MAP = new HashMap<Integer, String>();
-    private static final Collection<Integer> VALID_ATTRS;
 
+    private static final int ATTR_UNDEFINED = 0;
     static {
+        // The key value should be aligned with the enum value of Key.keyIcon.
+        addIconIdMap(0, "undefined", ATTR_UNDEFINED);
         addIconIdMap(1, "shiftKey", R.styleable.Keyboard_iconShiftKey);
         addIconIdMap(2, "deleteKey", R.styleable.Keyboard_iconDeleteKey);
         addIconIdMap(3, "settingsKey", R.styleable.Keyboard_iconSettingsKey);
@@ -56,22 +58,23 @@ public class KeyboardIconsSet {
         addIconIdMap(11, "shiftKeyShifted", R.styleable.Keyboard_iconShiftKeyShifted);
         addIconIdMap(12, "disabledShortcurKey", R.styleable.Keyboard_iconDisabledShortcutKey);
         addIconIdMap(13, "previewTabKey", R.styleable.Keyboard_iconPreviewTabKey);
-        VALID_ATTRS = ID_TO_ATTR_MAP.values();
     }
 
-    private static void addIconIdMap(int iconId, String name, Integer attrId) {
+    private static void addIconIdMap(int iconId, String name, int attrId) {
-        ID_TO_ATTR_MAP.put(iconId, attrId);
+        if (attrId != ATTR_UNDEFINED) {
-        NAME_TO_ATTR_MAP.put(name, attrId);
+            ATTR_ID_TO_ICON_ID.put(attrId,  iconId);
-        ATTR_TO_NAME_MAP.put(attrId, name);
+        }
+        NAME_TO_ICON_ID.put(name, iconId);
+        ICON_NAMES[iconId] = name;
     }
 
     public void loadIcons(final TypedArray keyboardAttrs) {
-        for (final Integer attrId : VALID_ATTRS) {
+        for (final Integer attrId : ATTR_ID_TO_ICON_ID.keySet()) {
             try {
                 final Drawable icon = keyboardAttrs.getDrawable(attrId);
-                if (icon == null) continue;
                 setDefaultBounds(icon);
-                mIcons.put(attrId, icon);
+                final Integer iconId = ATTR_ID_TO_ICON_ID.get(attrId);
+                mIcons[iconId] = icon;
             } catch (Resources.NotFoundException e) {
                 Log.w(TAG, "Drawable resource for icon #"
                         + keyboardAttrs.getResources().getResourceEntryName(attrId)
@@ -80,49 +83,32 @@ public class KeyboardIconsSet {
         }
     }
 
-    public static int getIconAttrId(final Integer iconId) {
+    private static boolean isValidIconId(final int iconId) {
-        if (iconId == ICON_UNDEFINED) {
+        return iconId >= 0 && iconId < ICON_NAMES.length;
-            return ATTR_UNDEFINED;
-        }
-        final Integer attrId = ID_TO_ATTR_MAP.get(iconId);
-        if (attrId == null) {
-            throw new IllegalArgumentException("icon id is out of range: " + iconId);
-        }
-        return attrId;
     }
 
-    public static int getIconAttrId(final String iconName) {
+    public static String getIconName(final int iconId) {
-        final Integer attrId = NAME_TO_ATTR_MAP.get(iconName);
+        return isValidIconId(iconId) ? ICON_NAMES[iconId] : "unknown<" + iconId + ">";
-        if (attrId == null) {
-            throw new IllegalArgumentException("unknown icon name: " + iconName);
-        }
-        return attrId;
     }
 
-    public static String getIconName(final int attrId) {
+    public static int getIconId(final String name) {
-        if (attrId == ATTR_UNDEFINED) {
+        final Integer iconId = NAME_TO_ICON_ID.get(name);
-            return "null";
+        if (iconId != null) {
+            return iconId;
         }
-        if (ATTR_TO_NAME_MAP.containsKey(attrId)) {
+        throw new RuntimeException("unknown icon name: " + name);
-            return ATTR_TO_NAME_MAP.get(attrId);
-        }
-        return String.format("unknown<0x%08x>", attrId);
     }
 
-    public Drawable getIconByAttrId(final Integer attrId) {
+    public Drawable getIconDrawable(final int iconId) {
-        if (attrId == ATTR_UNDEFINED) {
+        if (isValidIconId(iconId)) {
-            return null;
+            return mIcons[iconId];
         }
-        if (!VALID_ATTRS.contains(attrId)) {
+        throw new RuntimeException("unknown icon id: " + getIconName(iconId));
-            throw new IllegalArgumentException("unknown icon attribute id: " + attrId);
-        }
-        return mIcons.get(attrId);
     }
 
-    private static Drawable setDefaultBounds(final Drawable icon)  {
+    private static void setDefaultBounds(final Drawable icon)  {
         if (icon != null) {
             icon.setBounds(0, 0, icon.getIntrinsicWidth(), icon.getIntrinsicHeight());
         }
-        return icon;
     }
 }

java/src/com/android/inputmethod/latin/suggestions/MoreSuggestions.java

@@ -25,6 +25,7 @@ import com.android.inputmethod.keyboard.Key;
 import com.android.inputmethod.keyboard.Keyboard;
 import com.android.inputmethod.keyboard.KeyboardSwitcher;
 import com.android.inputmethod.keyboard.KeyboardView;
+import com.android.inputmethod.keyboard.internal.KeyboardIconsSet;
 import com.android.inputmethod.latin.LatinImeLogger;
 import com.android.inputmethod.latin.R;
 import com.android.inputmethod.latin.SuggestedWords;
@@ -199,6 +200,21 @@ public class MoreSuggestions extends Keyboard {
             return info;
         }
 
+        private static class Divider extends Key.Spacer {
+            private final Drawable mIcon;
+
+            public Divider(Keyboard.Params params, Drawable icon, int x, int y, int width,
+                    int height) {
+                super(params, x, y, width, height);
+                mIcon = icon;
+            }
+
+            @Override
+            public Drawable getIcon(KeyboardIconsSet iconSet) {
+                return mIcon;
+            }
+        }
+
         @Override
         public MoreSuggestions build() {
             final MoreSuggestionsParam params = mParams;
@@ -210,16 +226,16 @@ public class MoreSuggestions extends Keyboard {
                 final String info = getDebugInfo(mSuggestions, pos);
                 final int index = pos + SUGGESTION_CODE_BASE;
                 final Key key = new Key(
-                        params, word, info, null, index, null, x, y, width,
+                        params, word, info, KeyboardIconsSet.ICON_UNDEFINED, index, null, x, y,
-                        params.mDefaultRowHeight);
+                        width, params.mDefaultRowHeight);
                 params.markAsEdgeKey(key, pos);
                 params.onAddKey(key);
                 final int columnNumber = params.getColumnNumber(pos);
                 final int numColumnInRow = params.getNumColumnInRow(pos);
                 if (columnNumber < numColumnInRow - 1) {
-                    final Key.Spacer spacer = new Key.Spacer(params, params.mDivider, x + width, y,
+                    final Divider divider = new Divider(params, params.mDivider, x + width, y,
                             params.mDividerWidth, params.mDefaultRowHeight);
-                    params.onAddKey(spacer);
+                    params.onAddKey(divider);
                 }
             }
             return new MoreSuggestions(params);

tests/src/com/android/inputmethod/keyboard/internal/KeySpecParserTests.java

@@ -55,7 +55,7 @@ public class KeySpecParserTests extends AndroidTestCase {
         String actualOutputText = KeySpecParser.getOutputText(moreKeySpec);
         assertEquals(message + ": ouptputText:", expectedOutputText, actualOutputText);
 
-        int actualIcon = KeySpecParser.getIconAttrId(moreKeySpec);
+        int actualIcon = KeySpecParser.getIconId(moreKeySpec);
         assertEquals(message + ": icon:", expectedIcon, actualIcon);
 
         int actualCode = KeySpecParser.getCode(mRes, moreKeySpec);