Skip to content

Commit 319c915

Browse files
lukstbitdavid-allison
authored andcommitted
Improve background loading in DeckPicker
Offload the drawable creation on a background thread.
1 parent 59a4c8d commit 319c915

File tree

3 files changed

+83
-28
lines changed

3 files changed

+83
-28
lines changed

AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt

Lines changed: 55 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ import com.ichi2.anki.utils.ext.dismissAllDialogFragments
172172
import com.ichi2.anki.utils.ext.getSizeOfBitmapFromCollection
173173
import com.ichi2.anki.utils.ext.setFragmentResultListener
174174
import com.ichi2.anki.utils.ext.showDialogFragment
175+
import com.ichi2.anki.utils.runWithOOMCheck
175176
import com.ichi2.anki.widgets.DeckAdapter
176177
import com.ichi2.anki.worker.SyncMediaWorker
177178
import com.ichi2.anki.worker.SyncWorker
@@ -198,6 +199,7 @@ import com.ichi2.utils.positiveButton
198199
import com.ichi2.utils.show
199200
import com.ichi2.utils.title
200201
import com.ichi2.widget.WidgetStatus
202+
import kotlinx.coroutines.CancellationException
201203
import kotlinx.coroutines.Dispatchers
202204
import kotlinx.coroutines.Job
203205
import kotlinx.coroutines.flow.collectLatest
@@ -557,22 +559,9 @@ open class DeckPicker :
557559
// Add background to Deckpicker activity
558560
val view = binding.deckpickerXlView ?: binding.rootLayout
559561

560-
var hasDeckPickerBackground = false
561-
try {
562-
hasDeckPickerBackground = applyDeckPickerBackground()
563-
} catch (e: OutOfMemoryError) {
564-
// 6608 - OOM should be catchable here.
565-
Timber.w(e, "Failed to apply background - OOM")
566-
showThemedToast(this, getString(R.string.background_image_too_large), false)
567-
} catch (e: Exception) {
568-
Timber.w(e, "Failed to apply background")
569-
showThemedToast(this, getString(R.string.failed_to_apply_background_image, e.localizedMessage), false)
570-
}
571-
572562
deckListAdapter =
573563
DeckAdapter(
574564
this,
575-
activityHasBackground = hasDeckPickerBackground,
576565
onDeckSelected = { onDeckClick(it, DeckSelectionType.DEFAULT) },
577566
onDeckCountsSelected = { onDeckClick(it, DeckSelectionType.SHOW_STUDY_OPTIONS) },
578567
onDeckChildrenToggled = { deckId ->
@@ -587,6 +576,8 @@ open class DeckPicker :
587576
)
588577
deckPickerBinding.decks.adapter = deckListAdapter
589578

579+
lifecycleScope.launch { applyDeckPickerBackground() }
580+
590581
pullToSyncWrapper =
591582
deckPickerBinding.pullToSyncWrapper.apply {
592583
setDistanceToTriggerSync(SWIPE_TO_SYNC_TRIGGER_DISTANCE)
@@ -1065,43 +1056,82 @@ open class DeckPicker :
10651056
showDatabaseErrorDialog(DatabaseErrorDialogType.DIALOG_DISK_FULL)
10661057
}
10671058

1068-
// throws doesn't seem to be checked by the compiler - consider it to be documentation
1069-
@Throws(OutOfMemoryError::class)
1070-
private fun applyDeckPickerBackground(): Boolean {
1059+
// Note: when changing this method consider OutOfMemoryErrors
1060+
private suspend fun applyDeckPickerBackground() {
10711061
// Allow the user to clear data and get back to a good state if they provide an invalid background.
10721062
if (!Prefs.isBackgroundEnabled) {
10731063
Timber.d("No DeckPicker background preference")
10741064
deckPickerBinding.background.setBackgroundResource(0)
1075-
return false
1065+
deckListAdapter.activityHasBackground = false
1066+
return
10761067
}
10771068
val currentAnkiDroidDirectory = CollectionHelper.getCurrentAnkiDroidDirectory(this)
10781069
val imgFile = File(currentAnkiDroidDirectory, BackgroundImage.FILENAME)
10791070
if (!imgFile.exists()) {
10801071
Timber.d("No DeckPicker background image")
10811072
deckPickerBinding.background.setBackgroundResource(0)
1082-
return false
1073+
deckListAdapter.activityHasBackground = false
1074+
return
10831075
}
10841076

10851077
// TODO: Temporary fix to stop a crash on startup [15450], it can be removed either:
10861078
// * by moving this check to an upgrade path
10871079
// * once enough time has passed
10881080
// null shouldn't happen as we check for the file being present above this call
1089-
val (bitmapWidth, bitmapHeight) = getSizeOfBitmapFromCollection(BackgroundImage.FILENAME) ?: return false
1081+
val (bitmapWidth, bitmapHeight) = getSizeOfBitmapFromCollection(BackgroundImage.FILENAME) ?: return
10901082
if (bitmapWidth <= 0 || bitmapHeight <= 0) {
10911083
Timber.w("Decoding background image for dimensions info failed")
10921084
deckPickerBinding.background.setBackgroundResource(0)
1093-
return false
1085+
deckListAdapter.activityHasBackground = false
1086+
return
10941087
}
10951088
if (bitmapWidth * bitmapHeight * BITMAP_BYTES_PER_PIXEL > BackgroundImage.MAX_BITMAP_SIZE) {
10961089
Timber.w("DeckPicker background image dimensions too large")
10971090
deckPickerBinding.background.setBackgroundResource(0)
1098-
return false
1091+
deckListAdapter.activityHasBackground = false
1092+
return
1093+
}
1094+
1095+
fun onOOMError(error: OutOfMemoryError) {
1096+
Timber.w(error, "Failed to apply background - OOM")
1097+
showThemedToast(
1098+
this@DeckPicker,
1099+
getString(R.string.background_image_too_large),
1100+
false,
1101+
)
1102+
deckListAdapter.activityHasBackground = false
10991103
}
11001104

1101-
Timber.i("Applying background")
1102-
val drawable = Drawable.createFromPath(imgFile.absolutePath)
1103-
deckPickerBinding.background.setImageDrawable(drawable)
1104-
return true
1105+
try {
1106+
Timber.i("Applying background image selected by user")
1107+
val drawable =
1108+
withContext(Dispatchers.IO) {
1109+
// 6608 - OOM should be catchable here.
1110+
runWithOOMCheck(
1111+
{ Drawable.createFromPath(imgFile.absolutePath) },
1112+
::onOOMError,
1113+
)
1114+
}
1115+
runWithOOMCheck(
1116+
{
1117+
deckPickerBinding.background.setImageDrawable(drawable)
1118+
deckListAdapter.activityHasBackground = drawable != null
1119+
},
1120+
onError = ::onOOMError,
1121+
)
1122+
} catch (e: Exception) {
1123+
if (e is CancellationException) {
1124+
throw e
1125+
} else {
1126+
Timber.w(e, "Failed to apply background")
1127+
showThemedToast(
1128+
this,
1129+
getString(R.string.failed_to_apply_background_image, e.localizedMessage),
1130+
false,
1131+
)
1132+
deckListAdapter.activityHasBackground = false
1133+
}
1134+
}
11051135
}
11061136

11071137
override fun onCreateOptionsMenu(menu: Menu): Boolean {

AnkiDroid/src/main/java/com/ichi2/anki/utils/Exceptions.kt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,19 @@ fun Context.getUserFriendlyErrorText(e: Exception): String =
3636
?: e::class.simpleName?.ifBlank { null }
3737
?: getString(R.string.error__etc__unknown_error)
3838
}
39+
40+
/**
41+
* Runs [action] and guards against [OutOfMemoryError] using a try-catch block.
42+
* @param action the code to run
43+
* @param onError optional listener to be notified when a [OutOfMemoryError] occurred
44+
* @return the result of successfully executing [action] or null if an [OutOfMemoryError] occurred
45+
*/
46+
fun <T> runWithOOMCheck(
47+
action: () -> T,
48+
onError: ((OutOfMemoryError) -> Unit)? = null,
49+
) = try {
50+
action()
51+
} catch (e: OutOfMemoryError) {
52+
onError?.invoke(e)
53+
null
54+
}

AnkiDroid/src/main/java/com/ichi2/anki/widgets/DeckAdapter.kt

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,6 @@ import net.ankiweb.rsdroid.RustCleanup
4141
/**
4242
* A [RecyclerView.Adapter] used to show the list of decks inside [com.ichi2.anki.DeckPicker].
4343
*
44-
* @param activityHasBackground true if [com.ichi2.anki.DeckPicker] has a background set, false
45-
* otherwise. If true the adapter will make the rows transparent so the background can be seen.
4644
* @param onDeckSelected callback triggered when the user selects a deck
4745
* @param onDeckCountsSelected callback triggered when the user selects the counts of a deck
4846
* @param onDeckChildrenToggled callback triggered when the user toggles the visibility of its
@@ -54,7 +52,6 @@ import net.ankiweb.rsdroid.RustCleanup
5452
@RustCleanup("Differs from legacy backend: Create deck 'One', create deck 'One::two'. 'One::two' was not expanded")
5553
class DeckAdapter(
5654
context: Context,
57-
private val activityHasBackground: Boolean,
5855
private val onDeckSelected: (DeckId) -> Unit,
5956
private val onDeckCountsSelected: (DeckId) -> Unit,
6057
private val onDeckChildrenToggled: (DeckId) -> Unit,
@@ -82,6 +79,18 @@ class DeckAdapter(
8279
// Flags
8380
private var hasSubdecks = false
8481

82+
/**
83+
* Flag to indicate if the activity has a background set. If true the adapter will make the rows
84+
* transparent so the background can be seen.
85+
*/
86+
var activityHasBackground: Boolean = false
87+
set(value) {
88+
if (field != value) {
89+
field = value
90+
notifyDataSetChanged()
91+
}
92+
}
93+
8594
class ViewHolder(
8695
v: View,
8796
) : RecyclerView.ViewHolder(v) {

0 commit comments

Comments
 (0)