fix issue #500 View.toBitmap should take scroll state into consideration#504
fix issue #500 View.toBitmap should take scroll state into consideration#504romainguy merged 5 commits intoandroid:masterfrom LanderlYoung:master
Conversation
|
GCLOUD_SERVICE_KEY is not set, can't run unit test. Maybe someone could fix that. #!/bin/bash -eo pipefail
ftl-tests/setup.sh
GCLOUD_SERVICE_KEY env variable is empty. Exiting.
Exited with code 1In my local environment (Android Studio 3.2 Preview + Emulator), testes passed. |
| textView.textSize = 10f | ||
| textView.setTextColor(Color.BLACK) | ||
| textView.text = (0..10).joinToString("\n") { | ||
| UUID.randomUUID().toString() |
There was a problem hiding this comment.
Can you use something that isn't random? Maybe just it.toString().repeat(20).
There was a problem hiding this comment.
sure, that's ok
There was a problem hiding this comment.
You could also speed up the test by just drawing colored rectangles and just doing a couple of getPixel checks. That's all you need to test scrolling.
There was a problem hiding this comment.
TextView drawText is reasonably fast. I don't think there is any need to do some optimization.
There was a problem hiding this comment.
I don't care about drawText(), it's the comparison of all the pixel using getPixel(), one by one, that bothers me. Please just make a simpler test and check only a couple of pixels. It's enough.
| } | ||
| return Bitmap.createBitmap(width, height, config).applyCanvas(::draw) | ||
| return Bitmap.createBitmap(width, height, config).applyCanvas { | ||
| withSave { |
There was a problem hiding this comment.
Since the canvas isn't re-used we don't need to bother with save/restoring.
There was a problem hiding this comment.
Yes, it's true. But with save/restore, we can keep code consistent with normal view draws. ^_^
There was a problem hiding this comment.
That consistency provides no value. You're doing work for no gain. Please remove.
| textView.textSize = 10f | ||
| textView.setTextColor(Color.BLACK) | ||
| textView.text = (0..10).joinToString("\n") { | ||
| UUID.randomUUID().toString() |
There was a problem hiding this comment.
You could also speed up the test by just drawing colored rectangles and just doing a couple of getPixel checks. That's all you need to test scrolling.
| val height = lhs.height | ||
| for (x in 0 until width) { | ||
| for (y in 0 until height) { | ||
| if (lhs.getPixel(x, y) != rhs.getPixel(x + offX, y + offY)) { |
There was a problem hiding this comment.
Yes, sort of, about 100ms to run toBitmapScrolls test.
BTW, just ask, since this is test code, does it really matters?
There was a problem hiding this comment.
Yes it does. There are many tests to run and if each one takes several 100s of ms, it adds up :) There's absolutely no benefit in checking all the pixels here.
simple bitmap compare
3 participants
related issue #500