-
Notifications
You must be signed in to change notification settings - Fork 76
PANA-5027: Support svgs with navigation3 #3027
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
base: develop
Are you sure you want to change the base?
Conversation
2c9ea68 to
d9ee674
Compare
|
🎯 Code Coverage 🔗 Commit SHA: 12c1163 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #3027 +/- ##
===========================================
- Coverage 71.19% 71.12% -0.07%
===========================================
Files 859 860 +1
Lines 31443 31492 +49
Branches 5302 5307 +5
===========================================
+ Hits 22383 22396 +13
- Misses 7560 7584 +24
- Partials 1500 1512 +12
🚀 New features to boost your workflow:
|
| private val logger: InternalLogger = | ||
| (Datadog.getInstance() as? FeatureSdkCore)?.internalLogger | ||
| ?: InternalLogger.UNBOUND |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is possible to have internalLogger from the particular SDK instance?
| // Fill with Black (Transparency -> Black pixel) | ||
| canvas.drawColor(android.graphics.Color.BLACK) | ||
| val paint = Paint() | ||
| // Draw content in White (Opacity -> White pixel) | ||
| // Intermediate alpha levels will blend to create shades of gray (Grayscale) | ||
| paint.color = android.graphics.Color.WHITE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite get this and comments. As I understand, drawing black and white for Bitmap.Config.ALPHA_8 somehow creates grayscale since this config contains only alpha information.
But this method doesn't check that we get Bitmap.Config.ALPHA_8 config for the input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the key is in this line:
canvas.drawBitmap(source, 0f, 0f, paint)
because source is the alpha8 mask. When the paint is applied to this mask the alpha channel will cause the shades. We only reach this method through createBitmapFromAlpha8 in semanticsUtil, so we know that the source will necessarily be an alpha8 mask. I'll change the name of the class or method to make it clear that it is only for alpha8 processing.
| throwable = e | ||
| ) | ||
| null | ||
| } catch (e: OutOfMemoryError) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we catch OOM exception specifically here?
| (Datadog.getInstance() as? FeatureSdkCore)?.internalLogger | ||
| ?: InternalLogger.UNBOUND | ||
| ) : BitmapCreationHelper { | ||
| override fun createBitmap(width: Int, height: Int, config: Bitmap.Config): Bitmap { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems this function only calls Bitmap.createBitmap and return it, and in the callsite it directly chains with drawBitmap, so why can't we have a single function create inline this one?
What does this PR do?
Navigation3 bumps the compose-ui dependency to 1.9. Whereas previously we could get argb8888 from vectorDrawables with version 1.5, with 1.9 we get alpha8. In this pr, we convert the alpha8 mask into a grayscale bitmap, using the alpha transparency to set the level of gray. This means we lose color information but we have a performant method to keep supporting svgs.