Add a setting for users to lower the threshold for HW bitmaps #1486
Conversation
Debugging shows images may have bigger dimensions than expected. To avoid exceeding maxTextureSize, a second optional check is added, controlled by a toggle under Advanced Settings.
Some pages present flickering due to having dimensions that surpass those calculated in SubsamplingScaleImageView.
Rather than a toggle and test-based guesses, now the fix relies on user input to handle the images that don't work normally
| Preference.PreferenceItem.EditTextPreference( | ||
| pref = maxBitmapSize, | ||
| title = stringResource(MR.strings.pref_max_bitmap_size), | ||
| subtitle = stringResource(MR.strings.pref_max_bitmap_size_summary), | ||
| onValueChanged = { | ||
| if (it.isDigitsOnly() && it <= maxBitmapSize.defaultValue()) { | ||
| context.toast(MR.strings.pref_max_bitmap_size_success) | ||
| } else { | ||
| context.toast(MR.strings.pref_max_bitmap_size_error) | ||
| return@EditTextPreference false | ||
| } | ||
| true | ||
| }, | ||
| ), | ||
| Preference.PreferenceItem.TextPreference( | ||
| title = stringResource(MR.strings.pref_max_bitmap_size_reset), | ||
| enabled = remember(maxBitmap) { maxBitmap != maxBitmapSize.defaultValue() }, | ||
| onClick = { | ||
| maxBitmapSize.delete() | ||
| context.toast(MR.strings.pref_max_bitmap_size_success) | ||
| }, | ||
| ), |
There was a problem hiding this comment.
I'm not sure how i feel about an arbitrary value
There was a problem hiding this comment.
@AntsyLich Which arbitrary value are you referring to? The default value is set as GLUtil.maxTextureSize when declaring the function in BasePreferences, to mantain the way it currently works for non-affected users (which are the majority of the userbase).
I took this approach precisely because I don't want to be hardcoding a value that might not cover all cases or that might cover more than needed. This would only allow the user to lower the threshold to "reject" a hardware bitmap in the event that maxTextureSize turns out to be too permissive in practice, so unless someone changes this value, the decoder will work just as it does currently. It doesn't scale or affect the images in the slightest either, simply bypasses the hwBitmap creation if the image is taller than the device can handle (as defined by the user).
Edit: As for the user-defined value, it shouldn't be arbitrary either, but rather based on the pages that fail for them. They should check the height of the images and adjust the setting to "filter" anything equal or higher than that. If they overdo it and performance takes a hit because most pages are filtered, then that's on them.
Though I'm honestly not sure how much of a performance hit there would be since the decoding is already over by the time a hardware bitmap is created or not.
There was a problem hiding this comment.
I meant letting the user set an arbitrary value
There was a problem hiding this comment.
It should be fine since it's only a value that limits which images are shown as hardware bitmaps or left as they were. Same results as having a lower SDK from the start, but limited to certain pages or chapters rather than all of them. The setting also checks to ensure the value can't be higher than maxTextureSize and that it can only be a numerical value.
The user still has other options like downloading the pages while splitting them, or simply changing sources. This PR is just a more user-friendly method with less steps. A cleaner alternative like splitting the vertical splits on the fly before displaying each half in the reader seemed like too much work just to deal with a few edge cases so I gave up on that.
Anyway, since this PR worked for my needs, I brought it up, but in the end it is just a suggestion so don't feel pressured if it doesn't convince you.
Allow users to configure a new value that will override maxTextureSize when deciding if a software bitmap should be replaced by a hardware one after being decoded, to prevent textures from failing on older devices. Default is maxTextureSize since that works for most people.
Fix #1436 with a better approach than #1440 since it's all up to the user. It references the way the user-agent setting is set, so if there's anything that should be changed or streamlined in the code I'm all ears.
The issue of blanks and flickering:
Screen_recording_20241115_094748.mp4
What the setting looks like and how it works:
Screen_recording_20241115_095003.mp4