Fix up SVG rendering.

docs/USAGE.md

@@ -311,6 +311,10 @@ Please make sure `Default Human Language Interpreter` is set to `Rule-based Inte
 Generating charts can be taxing to the server.
 If you experience slow chart loading times and your server isn't powerful, open `Settings` and disable `High resolution charts` to improve loading times.
 
+### Icons look pixelated
+
+For good looking icons, the best approach is using SVG icons. Bitmap icons have a fixed size that doesn't scale with screen pixel density, so depending on the device, they may be scaled up by a large factor. When using SVG icons, ideally use icons that don't have a fixed size (in other words, they shouldn't have a 'width' and 'height' attribute on the root tag), as otherwise scaling might become necessary again: the app renders SVGs at their native size scaled by screen density, but scales them to a common size; when using icons without fixed size, the app can render them at precisely the needed size.
+
 ## Trademark Disclaimer
 
 Google Play and the Google Play logo are trademarks of Google Inc.

mobile/src/main/java/org/openhab/habdroid/core/NotificationHelper.kt

@@ -32,6 +32,7 @@ import org.openhab.habdroid.core.connection.ConnectionFactory
 import org.openhab.habdroid.model.CloudNotification
 import org.openhab.habdroid.ui.MainActivity
 import org.openhab.habdroid.util.HttpClient
+import org.openhab.habdroid.util.ImageConversionPolicy
 import org.openhab.habdroid.util.getNotificationTone
 import org.openhab.habdroid.util.getNotificationVibrationPattern
 import org.openhab.habdroid.util.getPrefs
@@ -125,9 +126,10 @@ class NotificationHelper constructor(private val context: Context) {
             val connection = ConnectionFactory.cloudConnectionOrNull
             if (connection != null && !context.isDataSaverActive()) {
                 try {
+                    val targetSize = context.resources.getDimensionPixelSize(R.dimen.notificationlist_icon_size)
                     iconBitmap = connection.httpClient
                         .get(message.icon.toUrl(context, true), timeoutMillis = 1000)
-                        .asBitmap(context.resources.getDimensionPixelSize(R.dimen.svg_image_default_size), false)
+                        .asBitmap(targetSize, ImageConversionPolicy.PreferTargetSize)
                         .response
                 } catch (e: HttpClient.HttpException) {
                     Log.d(TAG, "Error getting icon", e)

mobile/src/main/java/org/openhab/habdroid/ui/ChartActivity.kt

@@ -158,11 +158,6 @@ class ChartActivity : AbstractBaseActivity(), SwipeRefreshLayout.OnRefreshListen
             return
         }
 
-        if (chart.height == 0 || chart.width == 0) {
-            Log.d(TAG, "Height or width is 0")
-            return
-        }
-
         val chartUrl = widget.toChartUrl(
             getPrefs(),
             chart.width,
@@ -174,7 +169,7 @@ class ChartActivity : AbstractBaseActivity(), SwipeRefreshLayout.OnRefreshListen
         ) ?: return
 
         Log.d(TAG, "Load chart with url $chartUrl")
-        chart.setImageUrl(connection, chartUrl, chart.width, refreshDelayInMs = widget.refresh, forceLoad = force)
+        chart.setImageUrl(connection, chartUrl, refreshDelayInMs = widget.refresh, forceLoad = force)
     }
 
     private fun updateHasLegendButtonState(item: MenuItem) {

mobile/src/main/java/org/openhab/habdroid/ui/CloudNotificationAdapter.kt

@@ -112,7 +112,6 @@ class CloudNotificationAdapter(context: Context, private val loadMoreListener: (
                 iconView.setImageUrl(
                     conn,
                     notification.icon.toUrl(itemView.context, !itemView.context.isDataSaverActive()),
-                    itemView.resources.getDimensionPixelSize(R.dimen.notificationlist_icon_size),
                     timeoutMillis = 2000
                 )
             } else {

mobile/src/main/java/org/openhab/habdroid/ui/ItemPickerAdapter.kt

@@ -121,11 +121,9 @@ class ItemPickerAdapter(context: Context, private val itemClickListener: ItemCli
             val connection = ConnectionFactory.usableConnectionOrNull
             val icon = item.category.toOH2IconResource()
             if (icon != null && connection != null) {
-                val size = iconView.resources.getDimensionPixelSize(R.dimen.notificationlist_icon_size)
                 iconView.setImageUrl(
                     connection,
                     icon.toUrl(itemView.context, !itemView.context.isDataSaverActive()),
-                    size,
                     timeoutMillis = 2000
                 )
             } else {

mobile/src/main/java/org/openhab/habdroid/ui/MainActivity.kt

@@ -91,6 +91,7 @@ import org.openhab.habdroid.ui.homescreenwidget.VoiceWidgetWithIcon
 import org.openhab.habdroid.ui.preference.toItemUpdatePrefValue
 import org.openhab.habdroid.util.AsyncServiceResolver
 import org.openhab.habdroid.util.HttpClient
+import org.openhab.habdroid.util.ImageConversionPolicy
 import org.openhab.habdroid.util.PrefKeys
 import org.openhab.habdroid.util.RemoteLog
 import org.openhab.habdroid.util.ScreenLockMode
@@ -770,7 +771,7 @@ class MainActivity : AbstractBaseActivity(), ConnectionFactory.UpdateListener {
         launch {
             try {
                 item.icon = conn.httpClient.get(sitemap.icon.toUrl(this@MainActivity, !isDataSaverActive()))
-                    .asBitmap(defaultIcon!!.intrinsicWidth, true)
+                    .asBitmap(defaultIcon!!.intrinsicWidth, ImageConversionPolicy.ForceTargetSize)
                     .response
                     .toDrawable(resources)
             } catch (e: HttpClient.HttpException) {

mobile/src/main/java/org/openhab/habdroid/ui/WidgetAdapter.kt

@@ -635,7 +635,7 @@ class WidgetAdapter(
                 val bitmap = BitmapFactory.decodeByteArray(data, 0, data.size)
                 imageView.setImageBitmap(bitmap)
             } else if (widget.url != null) {
-                imageView.setImageUrl(connection, widget.url, parent.width, refreshDelayInMs = widget.refresh)
+                imageView.setImageUrl(connection, widget.url, refreshDelayInMs = widget.refresh)
             } else {
                 imageView.setImageDrawable(null)
             }
@@ -925,7 +925,7 @@ class WidgetAdapter(
             val chartUrl =
                 widget.toChartUrl(prefs, parent.width, chartTheme = chartTheme, density = density) ?: return
             Log.d(TAG, "Chart url = $chartUrl")
-            chart.setImageUrl(connection, chartUrl, parent.width, refreshDelayInMs = widget.refresh, forceLoad = true)
+            chart.setImageUrl(connection, chartUrl, refreshDelayInMs = widget.refresh, forceLoad = true)
         }
 
         override fun onStart() {
@@ -1390,8 +1390,7 @@ fun WidgetImageView.loadWidgetIcon(connection: Connection, widget: Widget, mappe
     }
     setImageUrl(
         connection,
-        widget.icon.toUrl(context, !context.isDataSaverActive()),
-        resources.getDimensionPixelSize(R.dimen.notificationlist_icon_size)
+        widget.icon.toUrl(context, !context.isDataSaverActive())
     )
     val color = mapper.mapColor(widget.iconColor)
     if (color != null) {

mobile/src/main/java/org/openhab/habdroid/ui/WidgetListFragment.kt

@@ -59,6 +59,7 @@ import org.openhab.habdroid.ui.widget.ContextMenuAwareRecyclerView
 import org.openhab.habdroid.ui.widget.RecyclerViewSwipeRefreshLayout
 import org.openhab.habdroid.util.CacheManager
 import org.openhab.habdroid.util.HttpClient
+import org.openhab.habdroid.util.ImageConversionPolicy
 import org.openhab.habdroid.util.PrefKeys
 import org.openhab.habdroid.util.SuggestedCommandsFactory
 import org.openhab.habdroid.util.ToastType
@@ -493,7 +494,7 @@ class WidgetListFragment : Fragment(), WidgetAdapter.ItemClickListener,
             try {
                 connection.httpClient
                     .get(linkedPage.icon.toUrl(context, true))
-                    .asBitmap(foregroundSize, true)
+                    .asBitmap(foregroundSize, ImageConversionPolicy.ForceTargetSize)
                     .response
             } catch (e: HttpClient.HttpException) {
                 null

mobile/src/main/java/org/openhab/habdroid/ui/homescreenwidget/ItemUpdateWidget.kt

@@ -44,6 +44,7 @@ import org.openhab.habdroid.ui.ItemUpdateWidgetItemPickerActivity
 import org.openhab.habdroid.ui.PreferencesActivity
 import org.openhab.habdroid.util.CacheManager
 import org.openhab.habdroid.util.HttpClient
+import org.openhab.habdroid.util.ImageConversionPolicy
 import org.openhab.habdroid.util.ToastType
 import org.openhab.habdroid.util.dpToPixel
 import org.openhab.habdroid.util.getStringOrEmpty
@@ -182,7 +183,7 @@ open class ItemUpdateWidget : AppWidgetProvider() {
                 val sizeInDp = min(height, width)
                 @Px val size = context.resources.dpToPixel(sizeInDp).toInt()
                 Log.d(TAG, "Icon size: $size")
-                iconData.svgToBitmap(size)
+                iconData.svgToBitmap(size, ImageConversionPolicy.PreferTargetSize)
             }
 
             val setIcon = { iconData: InputStream, isSvg: Boolean ->

mobile/src/main/java/org/openhab/habdroid/ui/widget/WidgetImageView.kt

@@ -31,11 +31,11 @@ import org.openhab.habdroid.R
 import org.openhab.habdroid.core.connection.Connection
 import org.openhab.habdroid.util.CacheManager
 import org.openhab.habdroid.util.HttpClient
+import org.openhab.habdroid.util.ImageConversionPolicy
 import kotlin.random.Random
 
 class WidgetImageView constructor(context: Context, attrs: AttributeSet?) : AppCompatImageView(context, attrs) {
     private var scope: CoroutineScope? = null
-    private val defaultSvgSize: Int
     private val fallback: Drawable?
     private val progressDrawable: Drawable?
 
@@ -49,6 +49,8 @@ class WidgetImageView constructor(context: Context, attrs: AttributeSet?) : AppC
     private var refreshInterval: Long = 0
     private var lastRefreshTimestamp: Long = 0
     private var refreshJob: Job? = null
+    private var pendingRequest: PendingRequest? = null
+    private var targetImageSize: Int = 0
 
     init {
         context.obtainStyledAttributes(attrs, R.styleable.WidgetImageView).apply {
@@ -58,19 +60,15 @@ class WidgetImageView constructor(context: Context, attrs: AttributeSet?) : AppC
             addRandomnessToUrl = getBoolean(R.styleable.WidgetImageView_addRandomnessToUrl, false)
             recycle()
         }
-
-        defaultSvgSize = context.resources.getDimensionPixelSize(R.dimen.svg_image_default_size)
     }
 
     fun setImageUrl(
         connection: Connection,
         url: String,
-        size: Int?,
         refreshDelayInMs: Int = 0,
         timeoutMillis: Long = HttpClient.DEFAULT_TIMEOUT_MS,
         forceLoad: Boolean = false
     ) {
-        val actualSize = size ?: defaultSvgSize
         val client = connection.httpClient
         val actualUrl = client.buildUrl(url)
 
@@ -81,25 +79,22 @@ class WidgetImageView constructor(context: Context, attrs: AttributeSet?) : AppC
                 // We're already in the process of loading this image, thus there's nothing to do
                 return
             }
-        } else {
+        } else if (pendingRequest == null) {
             lastRefreshTimestamp = 0
         }
 
-        cancelCurrentLoad()
-
-        val cached = CacheManager.getInstance(context).getCachedBitmap(actualUrl)
-        val request = HttpImageRequest(client, actualUrl, actualSize, timeoutMillis)
-
-        if (cached != null) {
-            setBitmapInternal(cached)
+        if (targetImageSize == 0) {
+            pendingRequest = PendingRequest(client, actualUrl, timeoutMillis, forceLoad)
         } else {
-            applyProgressDrawable()
+            doLoad(client, actualUrl, timeoutMillis, forceLoad)
         }
+    }
 
-        if (cached == null || forceLoad) {
-            request.execute(forceLoad)
-        }
-        lastRequest = request
+    override fun onLayout(changed: Boolean, left: Int, top: Int, right: Int, bottom: Int) {
+        super.onLayout(changed, left, top, right, bottom)
+        targetImageSize = right - left - paddingLeft - paddingRight
+        pendingRequest?.let { r -> doLoad(r.client, r.url, r.timeoutMillis, r.forceLoad) }
+        pendingRequest = null
     }
 
     override fun setImageResource(resId: Int) {
@@ -180,6 +175,24 @@ class WidgetImageView constructor(context: Context, attrs: AttributeSet?) : AppC
         removeProgressDrawable()
     }
 
+    private fun doLoad(client: HttpClient, url: HttpUrl, timeoutMillis: Long, forceLoad: Boolean) {
+        cancelCurrentLoad()
+
+        val cached = CacheManager.getInstance(context).getCachedBitmap(url)
+        val request = HttpImageRequest(client, url, targetImageSize, timeoutMillis)
+
+        if (cached != null) {
+            setBitmapInternal(cached)
+        } else {
+            applyProgressDrawable()
+        }
+
+        if (cached == null || forceLoad) {
+            request.execute(forceLoad)
+        }
+        lastRequest = request
+    }
+
     private fun setBitmapInternal(bitmap: Bitmap) {
         removeProgressDrawable()
         // Mark this call as being triggered by ourselves, as setImageBitmap()
@@ -222,10 +235,11 @@ class WidgetImageView constructor(context: Context, attrs: AttributeSet?) : AppC
 
     private fun removeProgressDrawable() {
         if (originalScaleType != null) {
-            super.setScaleType(originalScaleType)
             super.setAdjustViewBounds(originalAdjustViewBounds)
+            super.setScaleType(originalScaleType)
             originalScaleType = null
         }
+        super.setImageDrawable(null)
     }
 
     private inner class HttpImageRequest(
@@ -260,9 +274,14 @@ class WidgetImageView constructor(context: Context, attrs: AttributeSet?) : AppC
 
             job = scope?.launch(Dispatchers.Main) {
                 try {
+                    val conversionPolicy = when (originalScaleType ?: scaleType) {
+                        ScaleType.FIT_CENTER, ScaleType.FIT_START,
+                        ScaleType.FIT_END, ScaleType.FIT_XY -> ImageConversionPolicy.PreferTargetSize
+                        else -> ImageConversionPolicy.PreferSourceSize
+                    }
                     val bitmap = client.get(actualUrl.toString(),
                         timeoutMillis = timeoutMillis, caching = cachingMode)
-                        .asBitmap(size)
+                        .asBitmap(size, conversionPolicy)
                         .response
                     setBitmapInternal(bitmap)
                     CacheManager.getInstance(context).cacheBitmap(url, bitmap)
@@ -288,6 +307,8 @@ class WidgetImageView constructor(context: Context, attrs: AttributeSet?) : AppC
         }
     }
 
+    data class PendingRequest(val client: HttpClient, val url: HttpUrl, val timeoutMillis: Long, val forceLoad: Boolean)
+
     companion object {
         private val TAG = WidgetImageView::class.java.simpleName
     }