android: Workaround libhwui N bug

android: Workaround libhwui N bug

Bug is that if functor is the only operation in a canvas, then that draw
will not swap. Workaround it by always painting a 1x1 pixel.

BUG=704212
Review-Url: https://codereview.chromium.org/2779773002
Cr-Commit-Position: refs/heads/master@{#459906}
Committed: https://chromium.googlesource.com/chromium/src/+/340c180766d2857dab1a5a8a15df5c8e0c054f43

[boliu, 2017-03-27 18:34:00]

boliu@chromium.org changed reviewers:
+ tobiasjs@chromium.org

[boliu, 2017-03-27 18:34:01]

ptal, checked that it actually works

[Chris Craik, 2017-03-27 19:47:10]

ccraik@google.com changed reviewers:
+ ccraik@google.com

[Chris Craik, 2017-03-27 19:47:11]

This won't work if the background is 0x00000000 transparent.

Also, assuming that the functor is drawing the background, I expect this will
cause the background to double-blend if it's partially-transparent.

[boliu, 2017-03-27 20:11:24]

Description was changed from

==========
android: Workaround libhwui N bug

Bug is that if functor is the only operation in a canvas, then that draw
will not swap. Workaround it by always paiting the background color on N
before inserting functor.

BUG=704212
==========

to

==========
android: Workaround libhwui N bug

Bug is that if functor is the only operation in a canvas, then that draw
will not swap. Workaround it by painting a 1x1 almost transparent pixel.

BUG=704212
==========

[boliu, 2017-03-27 20:16:57]

On 2017/03/27 19:47:11, Chris Craik wrote:
> This won't work if the background is 0x00000000 transparent.

Ahh, if transparent won't work, then I guess there is not a "no-op" workaround
then.

> 
> Also, assuming that the functor is drawing the background, I expect this will
> cause the background to double-blend if it's partially-transparent.

Yes.. although I thought wrong blending is better than a random pixel being the
wrong color, but I guess can't get around it.

[Chris Craik, 2017-03-27 20:29:35]

On 2017/03/27 at 20:16:57, boliu wrote:
> On 2017/03/27 19:47:11, Chris Craik wrote:
> > This won't work if the background is 0x00000000 transparent.
> 
> Ahh, if transparent won't work, then I guess there is not a "no-op" workaround
then.
> 
> > 
> > Also, assuming that the functor is drawing the background, I expect this
will
> > cause the background to double-blend if it's partially-transparent.
> 
> Yes.. although I thought wrong blending is better than a random pixel being
the wrong color, but I guess can't get around it.

Alternate suggestion is to do a drawColor(Color.TRANSPARENT,
PorterDuff.Mode.ADD), which shouldn't actually affect the render output. 

Drawing 0x01000000 over a single pixel (or even the entire background) isn't
likely to be noticable, but you could make it truly invisible by drawing it with
a ColorMatrixColorFilter that effectively noops the color by scaling its alpha
channel down by a factor of 10.

All the draw operation needs to do is get past the canvas-level rejection logic,
which is essentially just
https://android.googlesource.com/platform/frameworks/base/+/master/libs/hwui/...
and then have tho op not be clipped out.


In any case, you should be able to draw a single pixel, to avoid fill cost of
touching the entire screen (which we do pay even with those noop draws).

[Tobias Sargeant, 2017-03-27 20:33:36]

On 2017/03/27 20:16:57, boliu wrote:
> On 2017/03/27 19:47:11, Chris Craik wrote:
> > This won't work if the background is 0x00000000 transparent.
> 
> Ahh, if transparent won't work, then I guess there is not a "no-op" workaround
> then.
> 
> > 
> > Also, assuming that the functor is drawing the background, I expect this
will
> > cause the background to double-blend if it's partially-transparent.
> 
> Yes.. although I thought wrong blending is better than a random pixel being
the
> wrong color, but I guess can't get around it.

LGTM. It looks like Chris has already suggested ways of making the visible
effect smaller, which was all I was going to comment on.

[boliu, 2017-03-27 20:59:24]

Description was changed from

==========
android: Workaround libhwui N bug

Bug is that if functor is the only operation in a canvas, then that draw
will not swap. Workaround it by painting a 1x1 almost transparent pixel.

BUG=704212
==========

to

==========
android: Workaround libhwui N bug

Bug is that if functor is the only operation in a canvas, then that draw
will not swap. Workaround it by always painting a 1x1 pixel.

BUG=704212
==========

[boliu, 2017-03-27 21:06:45]

On 2017/03/27 20:29:35, Chris Craik wrote:
> On 2017/03/27 at 20:16:57, boliu wrote:
> > On 2017/03/27 19:47:11, Chris Craik wrote:
> > > This won't work if the background is 0x00000000 transparent.
> > 
> > Ahh, if transparent won't work, then I guess there is not a "no-op"
workaround
> then.
> > 
> > > 
> > > Also, assuming that the functor is drawing the background, I expect this
> will
> > > cause the background to double-blend if it's partially-transparent.
> > 
> > Yes.. although I thought wrong blending is better than a random pixel being
> the wrong color, but I guess can't get around it.
> 
> Alternate suggestion is to do a drawColor(Color.TRANSPARENT,
> PorterDuff.Mode.ADD), which shouldn't actually affect the render output. 
> 
> Drawing 0x01000000 over a single pixel (or even the entire background) isn't
> likely to be noticable, but you could make it truly invisible by drawing it
with
> a ColorMatrixColorFilter that effectively noops the color by scaling its alpha
> channel down by a factor of 10.

Ok, did the color filter trick.

> 
> All the draw operation needs to do is get past the canvas-level rejection
logic,
> which is essentially just
>
https://android.googlesource.com/platform/frameworks/base/+/master/libs/hwui/...
> and then have tho op not be clipped out.
> 
> 
> In any case, you should be able to draw a single pixel, to avoid fill cost of
> touching the entire screen (which we do pay even with those noop draws).

[boliu, 2017-03-27 21:07:19]

The CQ bit was checked by boliu@chromium.org

[boliu, 2017-03-27 21:07:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
tobiasjs@chromium.org
Link to the patchset: https://codereview.chromium.org/2779773002/#ps40001
(title: "color filter")

[commit-bot: I haz the power, 2017-03-27 21:07:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Chris Craik, 2017-03-27 21:23:23]

lgtm

[commit-bot: I haz the power, 2017-03-27 22:22:27]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490648839476240,
"parent_rev": "8ea74ef3873df9a1b97277a1ddc92414137ff1cd", "commit_rev":
"340c180766d2857dab1a5a8a15df5c8e0c054f43"}

[commit-bot: I haz the power, 2017-03-27 22:23:11]

Description was changed from

==========
android: Workaround libhwui N bug

Bug is that if functor is the only operation in a canvas, then that draw
will not swap. Workaround it by always painting a 1x1 pixel.

BUG=704212
==========

to

==========
android: Workaround libhwui N bug

Bug is that if functor is the only operation in a canvas, then that draw
will not swap. Workaround it by always painting a 1x1 pixel.

BUG=704212

Review-Url: https://codereview.chromium.org/2779773002
Cr-Commit-Position: refs/heads/master@{#459906}
Committed:
https://chromium.googlesource.com/chromium/src/+/340c180766d2857dab1a5a8a15df...
==========

[commit-bot: I haz the power, 2017-03-27 22:23:12]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/340c180766d2857dab1a5a8a15df...