diff --git a/app/src/main/java/org/oxycblt/auxio/home/list/AlbumListFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/list/AlbumListFragment.kt index 2249c1bee..bc2893784 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/list/AlbumListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/list/AlbumListFragment.kt @@ -28,8 +28,8 @@ import org.oxycblt.auxio.ui.DisplayMode import org.oxycblt.auxio.ui.Item import org.oxycblt.auxio.ui.MenuItemListener import org.oxycblt.auxio.ui.MonoAdapter -import org.oxycblt.auxio.ui.PrimitiveBackingData import org.oxycblt.auxio.ui.Sort +import org.oxycblt.auxio.ui.SyncBackingData import org.oxycblt.auxio.ui.newMenu import org.oxycblt.auxio.util.formatDuration import org.oxycblt.auxio.util.launch @@ -50,7 +50,7 @@ class AlbumListFragment : HomeListFragment() { adapter = homeAdapter } - launch { homeModel.albums.collect(homeAdapter.data::submitList) } + launch { homeModel.albums.collect(homeAdapter.data::replaceList) } } override fun getPopup(pos: Int): String? { @@ -89,7 +89,7 @@ class AlbumListFragment : HomeListFragment() { class AlbumAdapter(listener: MenuItemListener) : MonoAdapter(listener) { - override val data = PrimitiveBackingData(this) + override val data = SyncBackingData(this, AlbumViewHolder.DIFFER) override val creator = AlbumViewHolder.CREATOR } } diff --git a/app/src/main/java/org/oxycblt/auxio/home/list/ArtistListFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/list/ArtistListFragment.kt index d306b64a3..034bea7ff 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/list/ArtistListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/list/ArtistListFragment.kt @@ -28,8 +28,8 @@ import org.oxycblt.auxio.ui.DisplayMode import org.oxycblt.auxio.ui.Item import org.oxycblt.auxio.ui.MenuItemListener import org.oxycblt.auxio.ui.MonoAdapter -import org.oxycblt.auxio.ui.PrimitiveBackingData import org.oxycblt.auxio.ui.Sort +import org.oxycblt.auxio.ui.SyncBackingData import org.oxycblt.auxio.ui.newMenu import org.oxycblt.auxio.util.formatDuration import org.oxycblt.auxio.util.launch @@ -50,7 +50,7 @@ class ArtistListFragment : HomeListFragment() { adapter = homeAdapter } - launch { homeModel.artists.collect(homeAdapter.data::submitList) } + launch { homeModel.artists.collect(homeAdapter.data::replaceList) } } override fun getPopup(pos: Int): String? { @@ -83,7 +83,7 @@ class ArtistListFragment : HomeListFragment() { class ArtistAdapter(listener: MenuItemListener) : MonoAdapter(listener) { - override val data = PrimitiveBackingData(this) + override val data = SyncBackingData(this, ArtistViewHolder.DIFFER) override val creator = ArtistViewHolder.CREATOR } } diff --git a/app/src/main/java/org/oxycblt/auxio/home/list/GenreListFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/list/GenreListFragment.kt index 12e439e3a..37f280e3a 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/list/GenreListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/list/GenreListFragment.kt @@ -28,8 +28,8 @@ import org.oxycblt.auxio.ui.GenreViewHolder import org.oxycblt.auxio.ui.Item import org.oxycblt.auxio.ui.MenuItemListener import org.oxycblt.auxio.ui.MonoAdapter -import org.oxycblt.auxio.ui.PrimitiveBackingData import org.oxycblt.auxio.ui.Sort +import org.oxycblt.auxio.ui.SyncBackingData import org.oxycblt.auxio.ui.newMenu import org.oxycblt.auxio.util.formatDuration import org.oxycblt.auxio.util.launch @@ -50,7 +50,7 @@ class GenreListFragment : HomeListFragment() { adapter = homeAdapter } - launch { homeModel.genres.collect(homeAdapter.data::submitList) } + launch { homeModel.genres.collect(homeAdapter.data::replaceList) } } override fun getPopup(pos: Int): String? { @@ -83,7 +83,7 @@ class GenreListFragment : HomeListFragment() { class GenreAdapter(listener: MenuItemListener) : MonoAdapter(listener) { - override val data = PrimitiveBackingData(this) + override val data = SyncBackingData(this, GenreViewHolder.DIFFER) override val creator = GenreViewHolder.CREATOR } } diff --git a/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt index 2d358446c..089a71030 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt @@ -26,9 +26,9 @@ import org.oxycblt.auxio.ui.DisplayMode import org.oxycblt.auxio.ui.Item import org.oxycblt.auxio.ui.MenuItemListener import org.oxycblt.auxio.ui.MonoAdapter -import org.oxycblt.auxio.ui.PrimitiveBackingData import org.oxycblt.auxio.ui.SongViewHolder import org.oxycblt.auxio.ui.Sort +import org.oxycblt.auxio.ui.SyncBackingData import org.oxycblt.auxio.ui.newMenu import org.oxycblt.auxio.util.formatDuration import org.oxycblt.auxio.util.launch @@ -49,7 +49,7 @@ class SongListFragment : HomeListFragment() { adapter = homeAdapter } - launch { homeModel.songs.collect(homeAdapter.data::submitList) } + launch { homeModel.songs.collect(homeAdapter.data::replaceList) } } override fun getPopup(pos: Int): String? { @@ -90,7 +90,7 @@ class SongListFragment : HomeListFragment() { inner class SongsAdapter(listener: MenuItemListener) : MonoAdapter(listener) { - override val data = PrimitiveBackingData(this) + override val data = SyncBackingData(this, SongViewHolder.DIFFER) override val creator = SongViewHolder.CREATOR } } diff --git a/app/src/main/java/org/oxycblt/auxio/music/excluded/ExcludedAdapter.kt b/app/src/main/java/org/oxycblt/auxio/music/excluded/ExcludedAdapter.kt index 9b9580ed5..9e84d14e8 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/excluded/ExcludedAdapter.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/excluded/ExcludedAdapter.kt @@ -19,9 +19,9 @@ package org.oxycblt.auxio.music.excluded import android.content.Context import org.oxycblt.auxio.databinding.ItemExcludedDirBinding +import org.oxycblt.auxio.ui.BackingData import org.oxycblt.auxio.ui.BindingViewHolder import org.oxycblt.auxio.ui.MonoAdapter -import org.oxycblt.auxio.ui.PrimitiveBackingData import org.oxycblt.auxio.util.inflater import org.oxycblt.auxio.util.textSafe @@ -31,12 +31,42 @@ import org.oxycblt.auxio.util.textSafe */ class ExcludedAdapter(listener: Listener) : MonoAdapter(listener) { - override val data = PrimitiveBackingData(this) + override val data = ExcludedBackingData(this) override val creator = ExcludedViewHolder.CREATOR interface Listener { fun onRemoveDirectory(dir: ExcludedDirectory) } + + class ExcludedBackingData(private val adapter: ExcludedAdapter) : + BackingData() { + private val _currentList = mutableListOf() + val currentList: List = _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) { + 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. */ diff --git a/app/src/main/java/org/oxycblt/auxio/music/excluded/ExcludedDialog.kt b/app/src/main/java/org/oxycblt/auxio/music/excluded/ExcludedDialog.kt index c9a543230..111945623 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/excluded/ExcludedDialog.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/excluded/ExcludedDialog.kt @@ -97,7 +97,7 @@ class ExcludedDialog : ?.mapNotNull(ExcludedDirectory::fromString) ?: settingsManager.excludedDirs - excludedAdapter.data.submitList(dirs) + excludedAdapter.data.addAll(dirs) requireBinding().excludedEmpty.isVisible = dirs.isEmpty() } diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt index 3cb83aaa7..b2a3e8d70 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt @@ -192,28 +192,20 @@ class PlaybackViewModel : ViewModel(), PlaybackStateManager.Callback, MusicStore playbackManager.prev() } - /** - * Remove a queue item using it's recyclerview adapter index. If the indices are valid, [apply] - * is called just before the change is committed so that the adapter can be updated. - */ - fun removeQueueDataItem(adapterIndex: Int, apply: () -> Unit) { + /** Remove a queue item using it's recyclerview adapter index. */ + fun removeQueueDataItem(adapterIndex: Int) { val index = adapterIndex + (playbackManager.queue.size - unlikelyToBeNull(_nextUp.value).size) if (index in playbackManager.queue.indices) { - apply() playbackManager.removeQueueItem(index) } } - /** - * Move queue items using their recyclerview adapter indices. If the indices are valid, [apply] - * is called just before the change is committed so that the adapter can be updated. - */ - fun moveQueueDataItems(adapterFrom: Int, adapterTo: Int, apply: () -> Unit): Boolean { + /** Move queue items using their recyclerview adapter indices. */ + fun moveQueueDataItems(adapterFrom: Int, adapterTo: Int): Boolean { val delta = (playbackManager.queue.size - unlikelyToBeNull(_nextUp.value).size) val from = adapterFrom + delta val to = adapterTo + delta if (from in playbackManager.queue.indices && to in playbackManager.queue.indices) { - apply() playbackManager.moveQueueItem(from, to) return true } diff --git a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueAdapter.kt b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueAdapter.kt index 3f1d8738d..bd0705dc0 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueAdapter.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueAdapter.kt @@ -23,17 +23,15 @@ import android.graphics.drawable.ColorDrawable import android.view.MotionEvent import android.view.View import androidx.core.view.isInvisible -import androidx.recyclerview.widget.AsyncListDiffer -import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.RecyclerView import com.google.android.material.shape.MaterialShapeDrawable import org.oxycblt.auxio.IntegerTable import org.oxycblt.auxio.databinding.ItemQueueSongBinding import org.oxycblt.auxio.music.Song -import org.oxycblt.auxio.ui.BackingData import org.oxycblt.auxio.ui.BindingViewHolder import org.oxycblt.auxio.ui.MonoAdapter import org.oxycblt.auxio.ui.SongViewHolder +import org.oxycblt.auxio.ui.SyncBackingData import org.oxycblt.auxio.util.context import org.oxycblt.auxio.util.disableDropShadowCompat import org.oxycblt.auxio.util.inflater @@ -42,7 +40,7 @@ import org.oxycblt.auxio.util.textSafe class QueueAdapter(listener: QueueItemListener) : MonoAdapter(listener) { - override val data = HybridBackingData(this, QueueSongViewHolder.DIFFER) + override val data = SyncBackingData(this, QueueSongViewHolder.DIFFER) override val creator = QueueSongViewHolder.CREATOR } @@ -107,67 +105,3 @@ private constructor( 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( - private val adapter: RecyclerView.Adapter<*>, - diffCallback: DiffUtil.ItemCallback -) : BackingData() { - private var _currentList = mutableListOf() - val currentList: List - 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, 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 AsyncListDiffer.rewriteListUnsafe(newList: List) { - 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 - } - } -} diff --git a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueDragCallback.kt b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueDragCallback.kt index 4267ed25c..aeb9f6bef 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueDragCallback.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueDragCallback.kt @@ -148,17 +148,12 @@ class QueueDragCallback( recyclerView: RecyclerView, viewHolder: RecyclerView.ViewHolder, target: RecyclerView.ViewHolder - ): Boolean { - val from = viewHolder.bindingAdapterPosition - val to = target.bindingAdapterPosition - - return playbackModel.moveQueueDataItems(from, to) { queueAdapter.data.moveItems(from, to) } - } + ): Boolean = + playbackModel.moveQueueDataItems( + viewHolder.bindingAdapterPosition, target.bindingAdapterPosition) override fun onSwiped(viewHolder: RecyclerView.ViewHolder, direction: Int) { - playbackModel.removeQueueDataItem(viewHolder.bindingAdapterPosition) { - queueAdapter.data.removeItem(viewHolder.bindingAdapterPosition) - } + playbackModel.removeQueueDataItem(viewHolder.bindingAdapterPosition) } override fun isLongPressDragEnabled(): Boolean = false diff --git a/app/src/main/java/org/oxycblt/auxio/ui/RecyclerFramework.kt b/app/src/main/java/org/oxycblt/auxio/ui/RecyclerFramework.kt index fbfe17c4c..5f1e4a43f 100644 --- a/app/src/main/java/org/oxycblt/auxio/ui/RecyclerFramework.kt +++ b/app/src/main/java/org/oxycblt/auxio/ui/RecyclerFramework.kt @@ -20,8 +20,8 @@ package org.oxycblt.auxio.ui import android.content.Context import android.view.View import android.view.ViewGroup -import android.widget.Adapter import androidx.annotation.StringRes +import androidx.recyclerview.widget.AdapterListUpdateCallback import androidx.recyclerview.widget.AsyncListDiffer import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.RecyclerView @@ -183,50 +183,128 @@ abstract class BackingData { } /** - * A list-backed [BackingData] that is modified using adapter primitives. Useful in cases where - * [AsyncBackingData] is not preferable due to bugs involving diffing. + * A list-backed [BackingData] that is modified synchronously. This is generally the recommended + * option for most adapters. * @author OxygenCobalt */ -class PrimitiveBackingData(private val adapter: RecyclerView.Adapter<*>) : BackingData() { - private var _currentList = mutableListOf() - +class SyncBackingData(adapter: RecyclerView.Adapter<*>, diffCallback: DiffUtil.ItemCallback) : + BackingData() { + private var differ = SyncListDiffer(adapter, diffCallback) /** The current list backing this adapter. */ val currentList: List - get() = _currentList + get() = differ.currentList - override fun getItem(position: Int): T = _currentList[position] - override fun getItemCount(): Int = _currentList.size + override fun getItem(position: Int): T = differ.currentList[position] + override fun getItemCount(): Int = differ.currentList.size + + /** Submit a list normally, doing a diff synchronously. */ + fun submitList(newList: List) { + differ.currentList = newList + } /** - * Update the list with a [newList]. This does a basic animation that removes all items and - * replaces them with the new ones. + * Replace this list with a new list. This is useful for very large list diffs that would + * generally be too chaotic and slow to provide a good UX. */ - @Suppress("NotifyDatasetChanged") - fun submitList(newList: List) { - if (_currentList.isNotEmpty()) { - val oldListSize = _currentList.size - _currentList.clear() - adapter.notifyItemRangeRemoved(0, oldListSize) + fun replaceList(newList: List) { + if (newList == differ.currentList) { + return } - _currentList = newList.toMutableList() - adapter.notifyItemRangeInserted(0, _currentList.size) + differ.currentList = emptyList() + differ.currentList = newList } +} - /** Add an item to the list. */ - fun add(item: T) { - _currentList.add(item) - adapter.notifyItemInserted(_currentList.lastIndex) - } +/** + * Like [AsyncListDiffer], but synchronous. This may seem like it would be inefficient, but in + * practice Auxio's lists tend to be small enough to the point where this does not matter, + * and situations that would be inefficient are ruled out with [SyncBackingData.replaceList]. + */ +private class SyncListDiffer( + adapter: RecyclerView.Adapter<*>, + private val diffCallback: DiffUtil.ItemCallback +) { + private val updateCallback = AdapterListUpdateCallback(adapter) - /** Remove an item from the list. */ - fun remove(item: T) { - val idx = _currentList.indexOf(item) - if (idx != -1) { - _currentList.removeAt(idx) - adapter.notifyItemRemoved(idx) + private var _currentList: List = emptyList() + var currentList: List + get() = _currentList + set(newList) { + if (newList === _currentList || newList.isEmpty() && _currentList.isEmpty()) { + 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) } - } } /**