Android + Kotlin advanced color picker (HSV and RGB)

Question

For almost the past year, I have been working on a pixel art editor for Android.

My pixel art editor was relying on an external library for its color picker. For 0.2.0, I wanted to change this by working on my own color picker.

This was quite a challenging task, but I got it done in the end.

Here were my objectives for the end user:

• Create a simple looking yet complex color picker
• RGB and HSV support
• Give the user the option of editing individual HSV and RGB channel values (similar to how it's done in GIMP/Photoshop)
• Spectrum mode
• Good performance, the user should not notice any delays when changing channel values

Here were my objectives for the codebase:

• The codebase should be as clean/readable as possible
• There shouldn't be any duplicated pieces of code in the codebase
• The codebase should be optimized for performance

The codebase isn't perfectly polished, but it's good enough that it's usable.

Below is the finished code for the color picker (enjoy!):

/*
 * Color Picker
 * Copyright 2022  therealbluepandabear
 *
 * This program is free software: you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation, either version 3 of the License, or
 * (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */

package com.example.colorpicker

import android.content.Context
import android.graphics.*
import android.util.AttributeSet
import android.util.Log
import android.view.MotionEvent
import android.view.View

class ColorPicker @JvmOverloads constructor(
    context: Context, attrs: AttributeSet? = null, defStyleAttr: Int = 0
) : View(context, attrs, defStyleAttr) {
    private lateinit var colorPickerViewBitmap: Bitmap
    private lateinit var boundingRect: Rect

    private val circlePickerPaint = Paint().apply {
        style = Paint.Style.FILL
    }
    private val whiteCirclePaint = Paint().apply {
        style = Paint.Style.FILL
        color = Color.WHITE
    }

    private val hsvArr = FloatArray(3)

    private var prevHue = -1f
    private var hue = 0f

    private var prevSaturation = -1f
    private var saturation = 0f

    private var prevValue = -1f
    private var value = 0f

    private var prevRed = -1f
    private var red = 0f

    private var prevGreen = -1f
    private var green = 0f

    private var prevBlue = -1f
    private var blue = 0f

    private var currentChannel: ColorChannel = ColorChannel.Hue

    private var touchCoordinates = Coordinates(0, 0)
    private var colorAtTouchCoordinates = Color.WHITE

    private var scaleWidth = 0f
    private var scaleHeight = 0f

    private var pixelsArr: IntArray = IntArray(0)

    private var onColorTapped: (Int) -> Unit = { }

    fun setOnColorTapped(onColorTapped: (Int) ->  Unit) {
        this.onColorTapped = onColorTapped
    }

    private fun getCurrentChannelValue() = when (currentChannel) {
        ColorChannel.Hue -> hue
        ColorChannel.Saturation -> saturation
        ColorChannel.Value -> value

        ColorChannel.Red -> red
        ColorChannel.Green -> green
        else -> blue
    }

    private fun getPrevChannelValue() = when (currentChannel) {
        ColorChannel.Hue -> prevHue
        ColorChannel.Saturation -> prevSaturation
        ColorChannel.Value -> prevValue

        ColorChannel.Red -> prevRed
        ColorChannel.Green -> prevGreen
        else -> prevBlue
    }

    private fun getBitmapSpec(): Pair<Int, Int> = when (currentChannel) {
        ColorChannel.Hue -> Pair(101, 101)
        ColorChannel.Saturation, ColorChannel.Value -> Pair(361, 101)
        ColorChannel.Red, ColorChannel.Green, ColorChannel.Blue -> Pair(256, 256)
        else -> Pair(361, 201)
    }

    private fun resetChannelValues() {
        prevHue = -1f
        hue = 0f

        prevSaturation = -1f
        saturation = 0f

        prevValue = -1f
        value = 0f
    }

    private fun initPixelsArr() {
        pixelsArr = IntArray(colorPickerViewBitmap.width * colorPickerViewBitmap.height)
    }

    private fun updateColorAtTouchCoordinates() {
        colorAtTouchCoordinates = colorPickerViewBitmap.getPixel((touchCoordinates.x / scaleWidth).toInt(), (touchCoordinates.y / scaleHeight).toInt())
    }

    private fun setScaleWidthHeight() {
        scaleWidth = measuredWidth.toFloat() / colorPickerViewBitmap.width.toFloat()
        scaleHeight = measuredHeight.toFloat() / colorPickerViewBitmap.height.toFloat()
    }

    fun updateHue(hue: Float) {
        this.hue = hue
        updateColorAtTouchCoordinates()
        invalidate()
    }

    fun updateSaturation(saturation: Float) {
        this.saturation = saturation
        updateColorAtTouchCoordinates()
        invalidate()
    }

    fun updateValue(value: Float) {
        this.value = value
        updateColorAtTouchCoordinates()
        invalidate()
    }

    fun updateRed(red: Float) {
        this.red = red
        updateColorAtTouchCoordinates()
        invalidate()
    }

    fun updateGreen(green: Float) {
        this.green = green
        updateColorAtTouchCoordinates()
        invalidate()
    }

    fun updateBlue(blue: Float) {
        this.blue = blue
        updateColorAtTouchCoordinates()
        invalidate()
    }

    fun updateCurrent(current: ColorChannel) {
        colorPickerViewBitmap.recycle()

        this.currentChannel = current
        val spec = getBitmapSpec()
        colorPickerViewBitmap = Bitmap.createBitmap(spec.first, spec.second, Bitmap.Config.ARGB_8888)

        resetChannelValues()
        initPixelsArr()
        setScaleWidthHeight()
        invalidate()
    }

    override fun onSizeChanged(w: Int, h: Int, oldw: Int, oldh: Int) {
        super.onSizeChanged(w, h, oldw, oldh)

        if (::colorPickerViewBitmap.isInitialized) {
            colorPickerViewBitmap.recycle()
        }

        colorPickerViewBitmap = Bitmap.createBitmap(101, 101, Bitmap.Config.ARGB_8888)
        boundingRect = Rect(0, 0, measuredWidth, measuredHeight)
        initPixelsArr()

        setScaleWidthHeight()
    }

    private fun doOnTouchEvent(event: MotionEvent) {
        touchCoordinates.x = event.x.toInt()
        touchCoordinates.y = event.y.toInt()

        val coordinatesX = touchCoordinates.x / scaleWidth
        val coordinatesY = touchCoordinates.y / scaleHeight

        when {
            coordinatesX < 0 && coordinatesY < 0 -> {
                touchCoordinates.x = 0
                touchCoordinates.y = 0

                colorAtTouchCoordinates = colorPickerViewBitmap.getPixel(0, 0)
            }

            coordinatesX < 0 && coordinatesY >= colorPickerViewBitmap.height -> {
                touchCoordinates.x = 0
                touchCoordinates.y = height - 1

                colorAtTouchCoordinates = colorPickerViewBitmap.getPixel(0, colorPickerViewBitmap.height - 1)
            }

            coordinatesX >= colorPickerViewBitmap.width && coordinatesY >= colorPickerViewBitmap.height -> {
                touchCoordinates.x = width - 1
                touchCoordinates.y = height - 1

                colorAtTouchCoordinates = colorPickerViewBitmap.getPixel(colorPickerViewBitmap.width - 1, colorPickerViewBitmap.height - 1)
            }

            coordinatesX >= colorPickerViewBitmap.width && coordinatesY < 0 -> {
                touchCoordinates.x = width - 1
                touchCoordinates.y = 0

                colorAtTouchCoordinates = colorPickerViewBitmap.getPixel(colorPickerViewBitmap.width - 1, 0)
            }

            coordinatesX < 0 -> {
                touchCoordinates.x = 0

                colorAtTouchCoordinates = colorPickerViewBitmap.getPixel(0, coordinatesY.toInt())
            }

            coordinatesX >= colorPickerViewBitmap.width -> {
                touchCoordinates.x = width - 1

                colorAtTouchCoordinates = colorPickerViewBitmap.getPixel(colorPickerViewBitmap.width - 1, coordinatesY.toInt())
            }

            coordinatesY < 0 -> {
                touchCoordinates.y = 0

                colorAtTouchCoordinates = colorPickerViewBitmap.getPixel(coordinatesX.toInt(), 0)
            }

            coordinatesY >= colorPickerViewBitmap.height -> {
                touchCoordinates.y = height - 1

                colorAtTouchCoordinates = colorPickerViewBitmap.getPixel(coordinatesX.toInt(), colorPickerViewBitmap.height - 1)
            }

            else -> {
                colorAtTouchCoordinates = colorPickerViewBitmap.getPixel(coordinatesX.toInt(), coordinatesY.toInt())
            }
        }

        onColorTapped.invoke(colorAtTouchCoordinates)
        invalidate()
    }

    override fun onTouchEvent(event: MotionEvent): Boolean {
        when (event.actionMasked) {
            MotionEvent.ACTION_DOWN -> {
                doOnTouchEvent(event)
            }

            MotionEvent.ACTION_MOVE -> {
                doOnTouchEvent(event)
            }
        }

        return true
    }

    override fun onDraw(canvas: Canvas) {
        if (::colorPickerViewBitmap.isInitialized) {
            if (getCurrentChannelValue() != getPrevChannelValue()) {
                prevHue = hue
                prevValue = value
                prevSaturation = saturation

                prevRed = red
                prevGreen = green
                prevBlue = blue

                colorPickerViewBitmap.getPixels(pixelsArr, 0, colorPickerViewBitmap.width, 0, 0, colorPickerViewBitmap.width, colorPickerViewBitmap.height)

                for (i in pixelsArr.indices) {
                    val x = i % colorPickerViewBitmap.width
                    val y = (i / colorPickerViewBitmap.width)

                    when (currentChannel) {
                        ColorChannel.Hue -> {
                            hsvArr[0] = hue
                            hsvArr[1] = x / 100f
                            hsvArr[2] = (100 - y) / 100f
                        }

                        ColorChannel.Saturation -> {
                            hsvArr[0] = x.toFloat()
                            hsvArr[1] = saturation / 100f
                            hsvArr[2] = (100 - y) / 100f
                        }

                        ColorChannel.Value -> {
                            hsvArr[0] = x.toFloat()
                            hsvArr[1] = (100 - y) / 100f
                            hsvArr[2] = value / 100f
                        }

                        ColorChannel.Red -> {
                            pixelsArr[i] = Color.argb(255, red.toInt(), x, (255 - y))
                        }

                        ColorChannel.Green -> {
                            pixelsArr[i] = Color.argb(255, x, green.toInt(), (255 - y))
                        }

                        ColorChannel.Blue -> {
                            pixelsArr[i] = Color.argb(255, x, (255 - y), blue.toInt())
                        }

                        else -> {
                            hsvArr[0] = x.toFloat()

                            if (y < 101) {
                                hsvArr[1] = y.toFloat() / 100f
                            } else {
                                hsvArr[1] = 1f
                            }


                            if (y < 101) {
                                hsvArr[2] = 1f
                            } else {
                                hsvArr[2] = (200 - y).toFloat() / 100f
                            }
                        }
                    }

                    if (currentChannel.colorSpace == ColorChannel.ColorSpace.HSV) {
                        pixelsArr[i] = Color.HSVToColor(hsvArr)
                    }
                }
            }

            val spec = getBitmapSpec()
            colorPickerViewBitmap = Bitmap.createBitmap(pixelsArr, spec.first, spec.second, Bitmap.Config.ARGB_8888)

            canvas.drawBitmap(colorPickerViewBitmap, null, boundingRect, null)

            circlePickerPaint.color = colorAtTouchCoordinates
            canvas.drawCircle(touchCoordinates.x.toFloat(), touchCoordinates.y.toFloat(), 60f, whiteCirclePaint)
            canvas.drawCircle(touchCoordinates.x.toFloat(), touchCoordinates.y.toFloat(), 50f, circlePickerPaint)
        }
    }
}

---

About the code

1. Throughout the code I utilize getPixels for good performance. I am weighing whether or not to switch to an NDK based implementation for iterating over the bitmap and whether or not the performance gains are worth it with the added cost of having to write lower level C/C++ code. I am also curious whether or not there are faster methods of doing what I want without an NDK based implementation.

2. I only update the current HSV/RGB values in the onDraw method if the previous channel value is different than the current channel value. This is to ensure that when the user moves their finger across the canvas it doesn't update the channel values.

---

Performance

I have used the measureTimeMillis function from Kotlin to measure the speed of the color picker. Whether or not this is a wise choice for measuring performance is something I'm also willing to get feedback on.

Changing hue value (bitmap size: 101x101)

Around 1000 onDraw iterations give an average time value of 8.412 milliseconds

Changing saturation value (bitmap size: 361x101)

Around 1000 onDraw iterations give an average time value of 28.878 milliseconds (not so good)

Changing 'value (V)' value (bitmap size: 361x101)

Around 1000 onDraw iterations give an average time value of 29.875 milliseconds (not so good)

Changing red value (bitmap size: 256x256)

Around 1000 onDraw iterations give an average time value of 3.068 milliseconds (this is fast because there is no HSV conversion)

Changing green value (bitmap size: 256x256)

Around 1000 onDraw iterations give an average time value of 2.92 milliseconds

Changing blue value (bitmap size: 256x256)

Around 1000 onDraw iterations give an average time value of 3.297 milliseconds

The spectrum speed is irrelevant as should only be drawn once to the canvas.

Screenshots

---

Here are the two types of feedback I am looking for:

1. I am interested to know how the code quality can be improved so it's as readable as possible.
2. I am interested to know ways that I can optimize this code so it works as quickly as possible -- currently it's fast, but not extremely fast.

If there are any issues with this question or room for improvement, do let me know.

Answer 1

• Reduce nesting of if's, that surround whole method, I suggest you instead use negative if's with return in onDraw. Or you can at least merge them into single if (or combine as I did):

    if (!(::colorPickerViewBitmap.isInitialized) || getCurrentChannelValue() == getPrevChannelValue()) return
  

• onTouchEvent can be simplified. Either using in operator (you can also extract this set into static constant), or simpler when:

    if (event.actionMasked in setOf(MotionEvent.ACTION_DOWN, MotionEvent.ACTION_MOVE)) {
        doOnTouchEvent(event)
    }
  
  
    //OR
  
    when (event.actionMasked) {
        MotionEvent.ACTION_DOWN, MotionEvent.ACTION_MOVE -> {
            doOnTouchEvent(event)
        }
    }
  

• I would extract the contents of your for loop in onDraw. It's hard to keep track of what are local variables and what are class variables in this big method. This will nicely show, what are actual parameters in every loop (not giving example, because I don't know the parameters of the method :) ). You can use inline keyword on the extracted function to ensure performance won't get worse.

• Your updateXX methods with 1 parameter are basically setters. You can use kotlin properties to make it more obvious and easier to read your API.

• Duplicate last 2 lines in those updateXX methods, I would consider extracting them to a method.

• Consider putting integer values for colour numbers into data structures. Either structure, that can hold "hue, saturation, value, red,.." and then have 2 instances of "current" and "previous". Or a structure, that can hold 2 values - current and previous.