Create BlurUtils.kt #5
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @BzCoder, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request introduces a new utility class, BlurUtils.kt, which provides methods for blurring images. It includes two blurring algorithms: rsBlur, which uses RenderScript for potentially faster blurring (especially on newer devices), and a standard blur method that implements a box blur algorithm in software. The utility is located in the me.bzcoder.easyglide.util package.
Highlights
- New Utility Class: Introduces
BlurUtils.ktfor blurring images. - RenderScript Blur: Implements
rsBlurusing RenderScript for hardware-accelerated blurring. - Box Blur Algorithm: Includes a standard
blurmethod implementing a box blur algorithm.
Changelog
- app/src/main/java/me/bzcoder/webview/utils/BlurUtils.kt
- Created
BlurUtils.ktwithrsBlurandblurmethods. - Added
rsBlurmethod to perform blurring using RenderScript. - Implemented
blurmethod with a box blur algorithm. - Placed the utility in the
me.bzcoder.easyglide.utilpackage.
- Created
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A blur, a haze,
Pixels blend in soft displays,
Focus gently lost.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The code introduces a utility class BlurUtils with two methods for blurring bitmaps: rsBlur using RenderScript and blur using a box blur algorithm. The implementation seems functional, but there are some areas for improvement in terms of efficiency, error handling, and code clarity.
Summary of Findings
- Potential NullPointerException in
rsBlur: ThersBlurmethod does not handle the case where the context is null, which could lead to a NullPointerException. A null check should be added to prevent this. - Performance of
blurmethod: Theblurmethod implements a box blur algorithm, which can be computationally expensive, especially for large radii. Consider using a more efficient blurring algorithm or leveraging hardware acceleration if available. - Clarity of variable names in
blurmethod: Theblurmethod uses many single-letter variable names (e.g.,w,h,x,y,i,p) and abbreviations (e.g.,wm,hm,wh,dv,rsum,gsum,bsum), which can make the code difficult to understand. More descriptive variable names would improve readability.
Merge Readiness
The code introduces a new utility class for blurring bitmaps. While the functionality appears to be implemented, there are potential issues with null context in rsBlur and performance concerns with the blur method. Addressing these issues would improve the quality and robustness of the code. I am unable to approve this pull request, and recommend that others review and approve this code before merging. I recommend that the pull request not be merged until the critical and high severity issues are addressed.
| fun rsBlur(context: Context?, toTransform: Bitmap, radius: Int): Bitmap { | ||
| val renderScript = RenderScript.create(context) | ||
| val input = Allocation.createFromBitmap(renderScript, toTransform) | ||
| val output = Allocation.createTyped(renderScript, input.type) | ||
| val scriptIntrinsicBlur = ScriptIntrinsicBlur.create(renderScript, Element.U8_4(renderScript)) | ||
| scriptIntrinsicBlur.setInput(input) | ||
| scriptIntrinsicBlur.setRadius(radius.toFloat()) | ||
| scriptIntrinsicBlur.forEach(output) | ||
| output.copyTo(toTransform) | ||
| renderScript.destroy() | ||
| return toTransform |
There was a problem hiding this comment.
The rsBlur method doesn't handle the case where context is null. This could lead to a NullPointerException. Add a null check for context and return the original bitmap if it's null.
if (context == null) {
return toTransform
}
val renderScript = RenderScript.create(context)
val input = Allocation.createFromBitmap(renderScript, toTransform)
val output = Allocation.createTyped(renderScript, input.type)
val scriptIntrinsicBlur = ScriptIntrinsicBlur.create(renderScript, Element.U8_4(renderScript))
scriptIntrinsicBlur.setInput(input)
scriptIntrinsicBlur.setRadius(radius.toFloat())
scriptIntrinsicBlur.forEach(output)
output.copyTo(toTransform)
renderScript.destroy()| fun blur(toTransform: Bitmap, radius: Int): Bitmap? { | ||
| if (radius < 1) { | ||
| return null | ||
| } | ||
| val w = toTransform.width | ||
| val h = toTransform.height | ||
| val pix = IntArray(w * h) | ||
| toTransform.getPixels(pix, 0, w, 0, 0, w, h) | ||
| val wm = w - 1 | ||
| val hm = h - 1 | ||
| val wh = w * h | ||
| val div = radius + radius + 1 | ||
| val r = IntArray(wh) | ||
| val g = IntArray(wh) | ||
| val b = IntArray(wh) | ||
| var rsum: Int | ||
| var gsum: Int | ||
| var bsum: Int | ||
| var x: Int | ||
| var y: Int | ||
| var i: Int | ||
| var p: Int | ||
| var yp: Int | ||
| var yi: Int | ||
| var yw: Int | ||
| val vmin = IntArray(Math.max(w, h)) | ||
| var divsum = div + 1 shr 1 | ||
| divsum *= divsum | ||
| val dv = IntArray(256 * divsum) | ||
| i = 0 | ||
| while (i < 256 * divsum) { | ||
| dv[i] = i / divsum | ||
| i++ | ||
| } | ||
| yi = 0 | ||
| yw = yi | ||
| val stack = Array(div) { IntArray(3) } | ||
| var stackpointer: Int | ||
| var stackstart: Int | ||
| var sir: IntArray | ||
| var rbs: Int | ||
| val r1 = radius + 1 | ||
| var routsum: Int | ||
| var goutsum: Int | ||
| var boutsum: Int | ||
| var rinsum: Int | ||
| var ginsum: Int | ||
| var binsum: Int | ||
| y = 0 | ||
| while (y < h) { | ||
| bsum = 0 | ||
| gsum = bsum | ||
| rsum = gsum | ||
| boutsum = rsum | ||
| goutsum = boutsum | ||
| routsum = goutsum | ||
| binsum = routsum | ||
| ginsum = binsum | ||
| rinsum = ginsum | ||
| i = -radius | ||
| while (i <= radius) { | ||
| p = pix[yi + Math.min(wm, Math.max(i, 0))] | ||
| sir = stack[i + radius] | ||
| sir[0] = p and 0xff0000 shr 16 | ||
| sir[1] = p and 0x00ff00 shr 8 | ||
| sir[2] = p and 0x0000ff | ||
| rbs = r1 - Math.abs(i) | ||
| rsum += sir[0] * rbs | ||
| gsum += sir[1] * rbs | ||
| bsum += sir[2] * rbs | ||
| if (i > 0) { | ||
| rinsum += sir[0] | ||
| ginsum += sir[1] | ||
| binsum += sir[2] | ||
| } else { | ||
| routsum += sir[0] | ||
| goutsum += sir[1] | ||
| boutsum += sir[2] | ||
| } | ||
| i++ | ||
| } | ||
| stackpointer = radius | ||
| x = 0 | ||
| while (x < w) { | ||
| r[yi] = dv[rsum] | ||
| g[yi] = dv[gsum] | ||
| b[yi] = dv[bsum] | ||
| rsum -= routsum | ||
| gsum -= goutsum | ||
| bsum -= boutsum | ||
| stackstart = stackpointer - radius + div | ||
| sir = stack[stackstart % div] | ||
| routsum -= sir[0] | ||
| goutsum -= sir[1] | ||
| boutsum -= sir[2] | ||
| if (y == 0) { | ||
| vmin[x] = Math.min(x + radius + 1, wm) | ||
| } | ||
| p = pix[yw + vmin[x]] | ||
| sir[0] = p and 0xff0000 shr 16 | ||
| sir[1] = p and 0x00ff00 shr 8 | ||
| sir[2] = p and 0x0000ff | ||
| rinsum += sir[0] | ||
| ginsum += sir[1] | ||
| binsum += sir[2] | ||
| rsum += rinsum | ||
| gsum += ginsum | ||
| bsum += binsum | ||
| stackpointer = (stackpointer + 1) % div | ||
| sir = stack[stackpointer % div] | ||
| routsum += sir[0] | ||
| goutsum += sir[1] | ||
| boutsum += sir[2] | ||
| rinsum -= sir[0] | ||
| ginsum -= sir[1] | ||
| binsum -= sir[2] | ||
| yi++ | ||
| x++ | ||
| } | ||
| yw += w | ||
| y++ | ||
| } | ||
| x = 0 | ||
| while (x < w) { | ||
| bsum = 0 | ||
| gsum = bsum | ||
| rsum = gsum | ||
| boutsum = rsum | ||
| goutsum = boutsum | ||
| routsum = goutsum | ||
| binsum = routsum | ||
| ginsum = binsum | ||
| rinsum = ginsum | ||
| yp = -radius * w | ||
| i = -radius | ||
| while (i <= radius) { | ||
| yi = Math.max(0, yp) + x | ||
| sir = stack[i + radius] | ||
| sir[0] = r[yi] | ||
| sir[1] = g[yi] | ||
| sir[2] = b[yi] | ||
| rbs = r1 - Math.abs(i) | ||
| rsum += r[yi] * rbs | ||
| gsum += g[yi] * rbs | ||
| bsum += b[yi] * rbs | ||
| if (i > 0) { | ||
| rinsum += sir[0] | ||
| ginsum += sir[1] | ||
| binsum += sir[2] | ||
| } else { | ||
| routsum += sir[0] | ||
| goutsum += sir[1] | ||
| boutsum += sir[2] | ||
| } | ||
| if (i < hm) { | ||
| yp += w | ||
| } | ||
| i++ | ||
| } | ||
| yi = x | ||
| stackpointer = radius | ||
| y = 0 | ||
| while (y < h) { | ||
| pix[yi] = -0x1000000 and pix[yi] or (dv[rsum] shl 16) or (dv[gsum] shl 8) or dv[bsum] | ||
| rsum -= routsum | ||
| gsum -= goutsum | ||
| bsum -= boutsum | ||
| stackstart = stackpointer - radius + div | ||
| sir = stack[stackstart % div] | ||
| routsum -= sir[0] | ||
| goutsum -= sir[1] | ||
| boutsum -= sir[2] | ||
| if (x == 0) { | ||
| vmin[y] = Math.min(y + r1, hm) * w | ||
| } | ||
| p = x + vmin[y] | ||
| sir[0] = r[p] | ||
| sir[1] = g[p] | ||
| sir[2] = b[p] | ||
| rinsum += sir[0] | ||
| ginsum += sir[1] | ||
| binsum += sir[2] | ||
| rsum += rinsum | ||
| gsum += ginsum | ||
| bsum += binsum | ||
| stackpointer = (stackpointer + 1) % div | ||
| sir = stack[stackpointer] | ||
| routsum += sir[0] | ||
| goutsum += sir[1] | ||
| boutsum += sir[2] | ||
| rinsum -= sir[0] | ||
| ginsum -= sir[1] | ||
| binsum -= sir[2] | ||
| yi += w | ||
| y++ | ||
| } | ||
| x++ | ||
| } | ||
| toTransform.setPixels(pix, 0, w, 0, 0, w, h) | ||
| return toTransform | ||
| } |
There was a problem hiding this comment.
The blur method implements a box blur algorithm, which can be slow for large radii. Consider using a more efficient algorithm like Gaussian blur, or leveraging hardware acceleration if possible. Profile the performance of this method to identify potential bottlenecks.
| fun blur(toTransform: Bitmap, radius: Int): Bitmap? { | ||
| if (radius < 1) { | ||
| return null | ||
| } |
There was a problem hiding this comment.
Consider throwing an IllegalArgumentException if the radius is invalid, rather than returning null. This provides more explicit feedback to the caller about the invalid argument.
if (radius < 1) {
throw IllegalArgumentException("Radius must be at least 1")
}| val w = toTransform.width | ||
| val h = toTransform.height | ||
| val pix = IntArray(w * h) | ||
| toTransform.getPixels(pix, 0, w, 0, 0, w, h) | ||
| val wm = w - 1 | ||
| val hm = h - 1 | ||
| val wh = w * h | ||
| val div = radius + radius + 1 | ||
| val r = IntArray(wh) | ||
| val g = IntArray(wh) | ||
| val b = IntArray(wh) | ||
| var rsum: Int | ||
| var gsum: Int | ||
| var bsum: Int | ||
| var x: Int | ||
| var y: Int | ||
| var i: Int | ||
| var p: Int | ||
| var yp: Int | ||
| var yi: Int | ||
| var yw: Int | ||
| val vmin = IntArray(Math.max(w, h)) | ||
| var divsum = div + 1 shr 1 | ||
| divsum *= divsum | ||
| val dv = IntArray(256 * divsum) | ||
| i = 0 | ||
| while (i < 256 * divsum) { | ||
| dv[i] = i / divsum | ||
| i++ | ||
| } | ||
| yi = 0 | ||
| yw = yi | ||
| val stack = Array(div) { IntArray(3) } | ||
| var stackpointer: Int | ||
| var stackstart: Int | ||
| var sir: IntArray | ||
| var rbs: Int | ||
| val r1 = radius + 1 | ||
| var routsum: Int | ||
| var goutsum: Int | ||
| var boutsum: Int | ||
| var rinsum: Int | ||
| var ginsum: Int | ||
| var binsum: Int |
There was a problem hiding this comment.
The variable names used in this method are not very descriptive. Consider using more meaningful names to improve readability. For example, w could be width, h could be height, rsum could be redSum, etc.
No description provided.