Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix onboarding background - Canvas: trying to draw too large bitmap #4888

Closed
wants to merge 4 commits into from

Conversation

bemusementpark
Copy link
Contributor

@bemusementpark bemusementpark commented Aug 11, 2024

Task/Issue URL: #3914

Description

We were calling View#setBackground which is incorrect and inefficient because:

android:scaleType="centerCrop" applies to src of the ImageView, not the background of a View, which is set in WelcomePage

I could've changed this to setImageResource, but we can just set src in xml, and add xml resources for light and night

This should fix the OOM, because setting the background probably set an upscaled version of this bitmap, so even though the large version of the bitmap is about 10MB, this device perhaps thinks it is an xxhdpi device and does an x3 making this large bitmap require 100MB. That's probably what we see in the logs anyway. This PR will hopefully scale the bitmap properly so Canvas is not overwhelmed.

I've simplified the bitmap resolution by using the resource folders for each bitmap for each combination of light | dark & < 600dp | > 600dp. I just deleted the holder drawable files, because the only interesting thing they did was set clampMode, but that wasn't (intentionally) being used anyway, because the bitmap was usual too big, and if it was going to be used it probably wouldn't look very good depending on which edge is clamped. And there was a scaleMode set on the ImageView which was unused because no src was applied.

I also changed sceneBg to ImageView in one override to match the other xml file, so that the binding will not be "downcast" to a View, but will be accessible as ImageView, which should help devs find the ImageView methods they need, and remove any desire for casting.

If anyone doesn't know, <ImageView> will be inflated as AppCompatImageView if inflated in an AppCompatActivity.

Steps to test this PR

Open the app in a low spec tablet.
Observe app does not crash.

UI changes

Before After
!(Screenshot 2024-08-11 at 9 10 01 PM) (Screenshot 2024-08-11 at 9 14 15 PM)
!(Screenshot 2024-08-11 at 10 02 00 PM) (![Screenshot 2024-08-11 at 9 14 15 PM](Screenshot 2024-08-11 at 10 02 23 PM))

...and the proper center-cropping kinda looks better too 🎉

@bemusementpark bemusementpark changed the title Fix onboarding background Fix onboarding background - Canvas: trying to draw too large bitmap Aug 13, 2024
@malmstein
Copy link
Contributor

This PR has been open for too long. We won’t take this into consideration for now.

@malmstein malmstein closed this Oct 11, 2024
@bemusementpark
Copy link
Contributor Author

Hey @malmstein this PR is unlikely to have conflicts and is likely to address this issue: #3914 which I assume is caused by opening these specific Bitmaps too large because 1. the tablet is big 2 it has low resolution, and then it crashes because of low specs.

This PR is also a nice little app-size improvement, as extraneous files are removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants