Add a setting for users to lower the threshold for HW bitmaps #1486
Conversation
| 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.
There was a problem hiding this comment.
I'd say use a slider with limited values, like 4k, 8k, 16k, etc.
There was a problem hiding this comment.
That's a neat approach, but most webtoons are already above 8k so just one step would mean filtering out pages that would normally work in the device. It seems rather overkill.
In my video, the issue disappeared after changing the default 16k to just 15500, so if the slider starts from maxTextureSize as the highest point and goes down in 4 or 6 steps of 500 px, it should be enough to handle the issue without being too drastic. I doubt the breakpoint for a device can be more than 3k pixels below maxTextureSize.
Edit: A quick test using max = maxTextureSize, min = (maxTextureSize/2), steps = 10 as arguments for the Slider preference seems to work fine. Should give enough flexibility and uses only device related values. 5 steps is also fine
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
Range is maxTextureSize/2 .. maxTextureSize, set to 5 steps in SettingsAdvancedScreen
Raikuha
force-pushed
the
custom_max_texture
branch
from
83e6162
to
e5cd22a
Compare
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