ui: rework primitive backing data into sync

Rework PrimitiveBackingData into SyncBackingData.

Google says that you should not use DiffUtil on the main thread, as it
is far too slow. In practice, the time it takes to diff is negligable,
even more so when costly diffs are filled in with a replace call
instead. Not only that, but switching to a synced list differ fixees
the horrible nightmare that is the queue adapter. Generally a win-win
for most use-cases in Auxio.
This commit is contained in:
OxygenCobalt 2022-06-07 20:00:10 -06:00
parent f85f366144
commit cc2561f20f
No known key found for this signature in database
GPG key ID: 37DBE3621FE9AD47
10 changed files with 164 additions and 135 deletions

View file

@ -28,8 +28,8 @@ import org.oxycblt.auxio.ui.DisplayMode
import org.oxycblt.auxio.ui.Item import org.oxycblt.auxio.ui.Item
import org.oxycblt.auxio.ui.MenuItemListener import org.oxycblt.auxio.ui.MenuItemListener
import org.oxycblt.auxio.ui.MonoAdapter import org.oxycblt.auxio.ui.MonoAdapter
import org.oxycblt.auxio.ui.PrimitiveBackingData
import org.oxycblt.auxio.ui.Sort import org.oxycblt.auxio.ui.Sort
import org.oxycblt.auxio.ui.SyncBackingData
import org.oxycblt.auxio.ui.newMenu import org.oxycblt.auxio.ui.newMenu
import org.oxycblt.auxio.util.formatDuration import org.oxycblt.auxio.util.formatDuration
import org.oxycblt.auxio.util.launch import org.oxycblt.auxio.util.launch
@ -50,7 +50,7 @@ class AlbumListFragment : HomeListFragment<Album>() {
adapter = homeAdapter adapter = homeAdapter
} }
launch { homeModel.albums.collect(homeAdapter.data::submitList) } launch { homeModel.albums.collect(homeAdapter.data::replaceList) }
} }
override fun getPopup(pos: Int): String? { override fun getPopup(pos: Int): String? {
@ -89,7 +89,7 @@ class AlbumListFragment : HomeListFragment<Album>() {
class AlbumAdapter(listener: MenuItemListener) : class AlbumAdapter(listener: MenuItemListener) :
MonoAdapter<Album, MenuItemListener, AlbumViewHolder>(listener) { MonoAdapter<Album, MenuItemListener, AlbumViewHolder>(listener) {
override val data = PrimitiveBackingData<Album>(this) override val data = SyncBackingData(this, AlbumViewHolder.DIFFER)
override val creator = AlbumViewHolder.CREATOR override val creator = AlbumViewHolder.CREATOR
} }
} }

View file

@ -28,8 +28,8 @@ import org.oxycblt.auxio.ui.DisplayMode
import org.oxycblt.auxio.ui.Item import org.oxycblt.auxio.ui.Item
import org.oxycblt.auxio.ui.MenuItemListener import org.oxycblt.auxio.ui.MenuItemListener
import org.oxycblt.auxio.ui.MonoAdapter import org.oxycblt.auxio.ui.MonoAdapter
import org.oxycblt.auxio.ui.PrimitiveBackingData
import org.oxycblt.auxio.ui.Sort import org.oxycblt.auxio.ui.Sort
import org.oxycblt.auxio.ui.SyncBackingData
import org.oxycblt.auxio.ui.newMenu import org.oxycblt.auxio.ui.newMenu
import org.oxycblt.auxio.util.formatDuration import org.oxycblt.auxio.util.formatDuration
import org.oxycblt.auxio.util.launch import org.oxycblt.auxio.util.launch
@ -50,7 +50,7 @@ class ArtistListFragment : HomeListFragment<Artist>() {
adapter = homeAdapter adapter = homeAdapter
} }
launch { homeModel.artists.collect(homeAdapter.data::submitList) } launch { homeModel.artists.collect(homeAdapter.data::replaceList) }
} }
override fun getPopup(pos: Int): String? { override fun getPopup(pos: Int): String? {
@ -83,7 +83,7 @@ class ArtistListFragment : HomeListFragment<Artist>() {
class ArtistAdapter(listener: MenuItemListener) : class ArtistAdapter(listener: MenuItemListener) :
MonoAdapter<Artist, MenuItemListener, ArtistViewHolder>(listener) { MonoAdapter<Artist, MenuItemListener, ArtistViewHolder>(listener) {
override val data = PrimitiveBackingData<Artist>(this) override val data = SyncBackingData(this, ArtistViewHolder.DIFFER)
override val creator = ArtistViewHolder.CREATOR override val creator = ArtistViewHolder.CREATOR
} }
} }

View file

@ -28,8 +28,8 @@ import org.oxycblt.auxio.ui.GenreViewHolder
import org.oxycblt.auxio.ui.Item import org.oxycblt.auxio.ui.Item
import org.oxycblt.auxio.ui.MenuItemListener import org.oxycblt.auxio.ui.MenuItemListener
import org.oxycblt.auxio.ui.MonoAdapter import org.oxycblt.auxio.ui.MonoAdapter
import org.oxycblt.auxio.ui.PrimitiveBackingData
import org.oxycblt.auxio.ui.Sort import org.oxycblt.auxio.ui.Sort
import org.oxycblt.auxio.ui.SyncBackingData
import org.oxycblt.auxio.ui.newMenu import org.oxycblt.auxio.ui.newMenu
import org.oxycblt.auxio.util.formatDuration import org.oxycblt.auxio.util.formatDuration
import org.oxycblt.auxio.util.launch import org.oxycblt.auxio.util.launch
@ -50,7 +50,7 @@ class GenreListFragment : HomeListFragment<Genre>() {
adapter = homeAdapter adapter = homeAdapter
} }
launch { homeModel.genres.collect(homeAdapter.data::submitList) } launch { homeModel.genres.collect(homeAdapter.data::replaceList) }
} }
override fun getPopup(pos: Int): String? { override fun getPopup(pos: Int): String? {
@ -83,7 +83,7 @@ class GenreListFragment : HomeListFragment<Genre>() {
class GenreAdapter(listener: MenuItemListener) : class GenreAdapter(listener: MenuItemListener) :
MonoAdapter<Genre, MenuItemListener, GenreViewHolder>(listener) { MonoAdapter<Genre, MenuItemListener, GenreViewHolder>(listener) {
override val data = PrimitiveBackingData<Genre>(this) override val data = SyncBackingData(this, GenreViewHolder.DIFFER)
override val creator = GenreViewHolder.CREATOR override val creator = GenreViewHolder.CREATOR
} }
} }

View file

@ -26,9 +26,9 @@ import org.oxycblt.auxio.ui.DisplayMode
import org.oxycblt.auxio.ui.Item import org.oxycblt.auxio.ui.Item
import org.oxycblt.auxio.ui.MenuItemListener import org.oxycblt.auxio.ui.MenuItemListener
import org.oxycblt.auxio.ui.MonoAdapter import org.oxycblt.auxio.ui.MonoAdapter
import org.oxycblt.auxio.ui.PrimitiveBackingData
import org.oxycblt.auxio.ui.SongViewHolder import org.oxycblt.auxio.ui.SongViewHolder
import org.oxycblt.auxio.ui.Sort import org.oxycblt.auxio.ui.Sort
import org.oxycblt.auxio.ui.SyncBackingData
import org.oxycblt.auxio.ui.newMenu import org.oxycblt.auxio.ui.newMenu
import org.oxycblt.auxio.util.formatDuration import org.oxycblt.auxio.util.formatDuration
import org.oxycblt.auxio.util.launch import org.oxycblt.auxio.util.launch
@ -49,7 +49,7 @@ class SongListFragment : HomeListFragment<Song>() {
adapter = homeAdapter adapter = homeAdapter
} }
launch { homeModel.songs.collect(homeAdapter.data::submitList) } launch { homeModel.songs.collect(homeAdapter.data::replaceList) }
} }
override fun getPopup(pos: Int): String? { override fun getPopup(pos: Int): String? {
@ -90,7 +90,7 @@ class SongListFragment : HomeListFragment<Song>() {
inner class SongsAdapter(listener: MenuItemListener) : inner class SongsAdapter(listener: MenuItemListener) :
MonoAdapter<Song, MenuItemListener, SongViewHolder>(listener) { MonoAdapter<Song, MenuItemListener, SongViewHolder>(listener) {
override val data = PrimitiveBackingData<Song>(this) override val data = SyncBackingData(this, SongViewHolder.DIFFER)
override val creator = SongViewHolder.CREATOR override val creator = SongViewHolder.CREATOR
} }
} }

View file

@ -19,9 +19,9 @@ package org.oxycblt.auxio.music.excluded
import android.content.Context import android.content.Context
import org.oxycblt.auxio.databinding.ItemExcludedDirBinding import org.oxycblt.auxio.databinding.ItemExcludedDirBinding
import org.oxycblt.auxio.ui.BackingData
import org.oxycblt.auxio.ui.BindingViewHolder import org.oxycblt.auxio.ui.BindingViewHolder
import org.oxycblt.auxio.ui.MonoAdapter import org.oxycblt.auxio.ui.MonoAdapter
import org.oxycblt.auxio.ui.PrimitiveBackingData
import org.oxycblt.auxio.util.inflater import org.oxycblt.auxio.util.inflater
import org.oxycblt.auxio.util.textSafe import org.oxycblt.auxio.util.textSafe
@ -31,12 +31,42 @@ import org.oxycblt.auxio.util.textSafe
*/ */
class ExcludedAdapter(listener: Listener) : class ExcludedAdapter(listener: Listener) :
MonoAdapter<ExcludedDirectory, ExcludedAdapter.Listener, ExcludedViewHolder>(listener) { MonoAdapter<ExcludedDirectory, ExcludedAdapter.Listener, ExcludedViewHolder>(listener) {
override val data = PrimitiveBackingData<ExcludedDirectory>(this) override val data = ExcludedBackingData(this)
override val creator = ExcludedViewHolder.CREATOR override val creator = ExcludedViewHolder.CREATOR
interface Listener { interface Listener {
fun onRemoveDirectory(dir: ExcludedDirectory) fun onRemoveDirectory(dir: ExcludedDirectory)
} }
class ExcludedBackingData(private val adapter: ExcludedAdapter) :
BackingData<ExcludedDirectory>() {
private val _currentList = mutableListOf<ExcludedDirectory>()
val currentList: List<ExcludedDirectory> = _currentList
override fun getItemCount(): Int = _currentList.size
override fun getItem(position: Int): ExcludedDirectory = _currentList[position]
fun add(dir: ExcludedDirectory) {
if (_currentList.contains(dir)) {
return
}
_currentList.add(dir)
adapter.notifyItemInserted(_currentList.lastIndex)
}
fun addAll(dirs: List<ExcludedDirectory>) {
val oldLastIndex = dirs.lastIndex
_currentList.addAll(dirs)
adapter.notifyItemRangeInserted(oldLastIndex, dirs.size)
}
fun remove(dir: ExcludedDirectory) {
val idx = _currentList.indexOf(dir)
_currentList.removeAt(idx)
adapter.notifyItemRemoved(idx)
}
}
} }
/** The viewholder for [ExcludedAdapter]. Not intended for use in other adapters. */ /** The viewholder for [ExcludedAdapter]. Not intended for use in other adapters. */

View file

@ -97,7 +97,7 @@ class ExcludedDialog :
?.mapNotNull(ExcludedDirectory::fromString) ?.mapNotNull(ExcludedDirectory::fromString)
?: settingsManager.excludedDirs ?: settingsManager.excludedDirs
excludedAdapter.data.submitList(dirs) excludedAdapter.data.addAll(dirs)
requireBinding().excludedEmpty.isVisible = dirs.isEmpty() requireBinding().excludedEmpty.isVisible = dirs.isEmpty()
} }

View file

@ -192,28 +192,20 @@ class PlaybackViewModel : ViewModel(), PlaybackStateManager.Callback, MusicStore
playbackManager.prev() playbackManager.prev()
} }
/** /** Remove a queue item using it's recyclerview adapter index. */
* Remove a queue item using it's recyclerview adapter index. If the indices are valid, [apply] fun removeQueueDataItem(adapterIndex: Int) {
* is called just before the change is committed so that the adapter can be updated.
*/
fun removeQueueDataItem(adapterIndex: Int, apply: () -> Unit) {
val index = val index =
adapterIndex + (playbackManager.queue.size - unlikelyToBeNull(_nextUp.value).size) adapterIndex + (playbackManager.queue.size - unlikelyToBeNull(_nextUp.value).size)
if (index in playbackManager.queue.indices) { if (index in playbackManager.queue.indices) {
apply()
playbackManager.removeQueueItem(index) playbackManager.removeQueueItem(index)
} }
} }
/** /** Move queue items using their recyclerview adapter indices. */
* Move queue items using their recyclerview adapter indices. If the indices are valid, [apply] fun moveQueueDataItems(adapterFrom: Int, adapterTo: Int): Boolean {
* is called just before the change is committed so that the adapter can be updated.
*/
fun moveQueueDataItems(adapterFrom: Int, adapterTo: Int, apply: () -> Unit): Boolean {
val delta = (playbackManager.queue.size - unlikelyToBeNull(_nextUp.value).size) val delta = (playbackManager.queue.size - unlikelyToBeNull(_nextUp.value).size)
val from = adapterFrom + delta val from = adapterFrom + delta
val to = adapterTo + delta val to = adapterTo + delta
if (from in playbackManager.queue.indices && to in playbackManager.queue.indices) { if (from in playbackManager.queue.indices && to in playbackManager.queue.indices) {
apply()
playbackManager.moveQueueItem(from, to) playbackManager.moveQueueItem(from, to)
return true return true
} }

View file

@ -23,17 +23,15 @@ import android.graphics.drawable.ColorDrawable
import android.view.MotionEvent import android.view.MotionEvent
import android.view.View import android.view.View
import androidx.core.view.isInvisible import androidx.core.view.isInvisible
import androidx.recyclerview.widget.AsyncListDiffer
import androidx.recyclerview.widget.DiffUtil
import androidx.recyclerview.widget.RecyclerView import androidx.recyclerview.widget.RecyclerView
import com.google.android.material.shape.MaterialShapeDrawable import com.google.android.material.shape.MaterialShapeDrawable
import org.oxycblt.auxio.IntegerTable import org.oxycblt.auxio.IntegerTable
import org.oxycblt.auxio.databinding.ItemQueueSongBinding import org.oxycblt.auxio.databinding.ItemQueueSongBinding
import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.music.Song
import org.oxycblt.auxio.ui.BackingData
import org.oxycblt.auxio.ui.BindingViewHolder import org.oxycblt.auxio.ui.BindingViewHolder
import org.oxycblt.auxio.ui.MonoAdapter import org.oxycblt.auxio.ui.MonoAdapter
import org.oxycblt.auxio.ui.SongViewHolder import org.oxycblt.auxio.ui.SongViewHolder
import org.oxycblt.auxio.ui.SyncBackingData
import org.oxycblt.auxio.util.context import org.oxycblt.auxio.util.context
import org.oxycblt.auxio.util.disableDropShadowCompat import org.oxycblt.auxio.util.disableDropShadowCompat
import org.oxycblt.auxio.util.inflater import org.oxycblt.auxio.util.inflater
@ -42,7 +40,7 @@ import org.oxycblt.auxio.util.textSafe
class QueueAdapter(listener: QueueItemListener) : class QueueAdapter(listener: QueueItemListener) :
MonoAdapter<Song, QueueItemListener, QueueSongViewHolder>(listener) { MonoAdapter<Song, QueueItemListener, QueueSongViewHolder>(listener) {
override val data = HybridBackingData(this, QueueSongViewHolder.DIFFER) override val data = SyncBackingData(this, QueueSongViewHolder.DIFFER)
override val creator = QueueSongViewHolder.CREATOR override val creator = QueueSongViewHolder.CREATOR
} }
@ -107,67 +105,3 @@ private constructor(
val DIFFER = SongViewHolder.DIFFER val DIFFER = SongViewHolder.DIFFER
} }
} }
/**
* A list-backed [BackingData] that can be modified with both adapter primitives and
* [AsyncListDiffer]. This is incredibly dangerous and can probably crash the app if you look at it
* the wrong way, so please don't use it outside of the queue module.
*/
class HybridBackingData<T>(
private val adapter: RecyclerView.Adapter<*>,
diffCallback: DiffUtil.ItemCallback<T>
) : BackingData<T>() {
private var _currentList = mutableListOf<T>()
val currentList: List<T>
get() = _currentList
private val differ = AsyncListDiffer(adapter, diffCallback)
override fun getItem(position: Int): T = _currentList[position]
override fun getItemCount(): Int = _currentList.size
fun submitList(newData: List<T>, onDone: () -> Unit = {}) {
if (newData != _currentList) {
_currentList = newData.toMutableList()
differ.submitList(newData, onDone)
}
}
fun moveItems(from: Int, to: Int) {
_currentList.add(to, _currentList.removeAt(from))
differ.rewriteListUnsafe(_currentList)
adapter.notifyItemMoved(from, to)
}
fun removeItem(at: Int) {
_currentList.removeAt(at)
differ.rewriteListUnsafe(_currentList)
adapter.notifyItemRemoved(at)
}
/**
* Rewrites the AsyncListDiffer's internal list, cancelling any diffs that are currently in
* progress. I cannot describe in words how dangerous this is, but it's also the only thing I
* can do to marry the adapter primitives with DiffUtil.
*/
private fun <T> AsyncListDiffer<T>.rewriteListUnsafe(newList: List<T>) {
differMaxGenerationsField.set(this, (differMaxGenerationsField.get(this) as Int) + 1)
differListField.set(this, newList.toMutableList())
differImmutableListField.set(this, newList)
}
companion object {
private val differListField =
AsyncListDiffer::class.java.getDeclaredField("mList").apply { isAccessible = true }
private val differImmutableListField =
AsyncListDiffer::class.java.getDeclaredField("mReadOnlyList").apply {
isAccessible = true
}
private val differMaxGenerationsField =
AsyncListDiffer::class.java.getDeclaredField("mMaxScheduledGeneration").apply {
isAccessible = true
}
}
}

View file

@ -148,17 +148,12 @@ class QueueDragCallback(
recyclerView: RecyclerView, recyclerView: RecyclerView,
viewHolder: RecyclerView.ViewHolder, viewHolder: RecyclerView.ViewHolder,
target: RecyclerView.ViewHolder target: RecyclerView.ViewHolder
): Boolean { ): Boolean =
val from = viewHolder.bindingAdapterPosition playbackModel.moveQueueDataItems(
val to = target.bindingAdapterPosition viewHolder.bindingAdapterPosition, target.bindingAdapterPosition)
return playbackModel.moveQueueDataItems(from, to) { queueAdapter.data.moveItems(from, to) }
}
override fun onSwiped(viewHolder: RecyclerView.ViewHolder, direction: Int) { override fun onSwiped(viewHolder: RecyclerView.ViewHolder, direction: Int) {
playbackModel.removeQueueDataItem(viewHolder.bindingAdapterPosition) { playbackModel.removeQueueDataItem(viewHolder.bindingAdapterPosition)
queueAdapter.data.removeItem(viewHolder.bindingAdapterPosition)
}
} }
override fun isLongPressDragEnabled(): Boolean = false override fun isLongPressDragEnabled(): Boolean = false

View file

@ -20,8 +20,8 @@ package org.oxycblt.auxio.ui
import android.content.Context import android.content.Context
import android.view.View import android.view.View
import android.view.ViewGroup import android.view.ViewGroup
import android.widget.Adapter
import androidx.annotation.StringRes import androidx.annotation.StringRes
import androidx.recyclerview.widget.AdapterListUpdateCallback
import androidx.recyclerview.widget.AsyncListDiffer import androidx.recyclerview.widget.AsyncListDiffer
import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.DiffUtil
import androidx.recyclerview.widget.RecyclerView import androidx.recyclerview.widget.RecyclerView
@ -183,50 +183,128 @@ abstract class BackingData<T> {
} }
/** /**
* A list-backed [BackingData] that is modified using adapter primitives. Useful in cases where * A list-backed [BackingData] that is modified synchronously. This is generally the recommended
* [AsyncBackingData] is not preferable due to bugs involving diffing. * option for most adapters.
* @author OxygenCobalt * @author OxygenCobalt
*/ */
class PrimitiveBackingData<T>(private val adapter: RecyclerView.Adapter<*>) : BackingData<T>() { class SyncBackingData<T>(adapter: RecyclerView.Adapter<*>, diffCallback: DiffUtil.ItemCallback<T>) :
private var _currentList = mutableListOf<T>() BackingData<T>() {
private var differ = SyncListDiffer(adapter, diffCallback)
/** The current list backing this adapter. */ /** The current list backing this adapter. */
val currentList: List<T> val currentList: List<T>
get() = _currentList get() = differ.currentList
override fun getItem(position: Int): T = _currentList[position] override fun getItem(position: Int): T = differ.currentList[position]
override fun getItemCount(): Int = _currentList.size override fun getItemCount(): Int = differ.currentList.size
/** Submit a list normally, doing a diff synchronously. */
fun submitList(newList: List<T>) {
differ.currentList = newList
}
/** /**
* Update the list with a [newList]. This does a basic animation that removes all items and * Replace this list with a new list. This is useful for very large list diffs that would
* replaces them with the new ones. * generally be too chaotic and slow to provide a good UX.
*/ */
@Suppress("NotifyDatasetChanged") fun replaceList(newList: List<T>) {
fun submitList(newList: List<T>) { if (newList == differ.currentList) {
if (_currentList.isNotEmpty()) { return
val oldListSize = _currentList.size
_currentList.clear()
adapter.notifyItemRangeRemoved(0, oldListSize)
} }
_currentList = newList.toMutableList() differ.currentList = emptyList()
adapter.notifyItemRangeInserted(0, _currentList.size) differ.currentList = newList
} }
}
/** Add an item to the list. */ /**
fun add(item: T) { * Like [AsyncListDiffer], but synchronous. This may seem like it would be inefficient, but in
_currentList.add(item) * practice Auxio's lists tend to be small enough to the point where this does not matter,
adapter.notifyItemInserted(_currentList.lastIndex) * and situations that would be inefficient are ruled out with [SyncBackingData.replaceList].
} */
private class SyncListDiffer<T>(
adapter: RecyclerView.Adapter<*>,
private val diffCallback: DiffUtil.ItemCallback<T>
) {
private val updateCallback = AdapterListUpdateCallback(adapter)
/** Remove an item from the list. */ private var _currentList: List<T> = emptyList()
fun remove(item: T) { var currentList: List<T>
val idx = _currentList.indexOf(item) get() = _currentList
if (idx != -1) { set(newList) {
_currentList.removeAt(idx) if (newList === _currentList || newList.isEmpty() && _currentList.isEmpty()) {
adapter.notifyItemRemoved(idx) return
}
if (newList.isEmpty()) {
val oldListSize = _currentList.size
_currentList = emptyList()
updateCallback.onRemoved(0, oldListSize)
return
}
if (_currentList.isEmpty()) {
_currentList = newList
updateCallback.onInserted(0, newList.size)
return
}
val oldList = _currentList
val result =
DiffUtil.calculateDiff(
object : DiffUtil.Callback() {
override fun getOldListSize(): Int {
return oldList.size
}
override fun getNewListSize(): Int {
return newList.size
}
override fun areItemsTheSame(
oldItemPosition: Int,
newItemPosition: Int
): Boolean {
val oldItem: T? = oldList[oldItemPosition]
val newItem: T? = newList[newItemPosition]
return if (oldItem != null && newItem != null) {
diffCallback.areItemsTheSame(oldItem, newItem)
} else {
oldItem == null && newItem == null
}
}
override fun areContentsTheSame(
oldItemPosition: Int,
newItemPosition: Int
): Boolean {
val oldItem: T? = oldList[oldItemPosition]
val newItem: T? = newList[newItemPosition]
return if (oldItem != null && newItem != null) {
diffCallback.areContentsTheSame(oldItem, newItem)
} else if (oldItem == null && newItem == null) {
true
} else {
throw AssertionError()
}
}
override fun getChangePayload(
oldItemPosition: Int,
newItemPosition: Int
): Any? {
val oldItem: T? = oldList[oldItemPosition]
val newItem: T? = newList[newItemPosition]
return if (oldItem != null && newItem != null) {
diffCallback.getChangePayload(oldItem, newItem)
} else {
throw AssertionError()
}
}
})
_currentList = newList
result.dispatchUpdatesTo(updateCallback)
} }
}
} }
/** /**