Put background filters on the makeWithFilter() fast-path

Put background filters on the makeWithFilter() fast-path

Implements GLRenderer::ApplyBackgroundFilters using ApplyImageFilter API.

BUG=613233
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

[Anton Obzhirov, 2016-07-12 10:22:16]

Description was changed from

==========
Put background filters on the makeWithFilter() fast-path

Implements GLRenderer::ApplyBackgroundFilters using ApplyImageFilter API.

BUG=613233
==========

to

==========
Put background filters on the makeWithFilter() fast-path

Implements GLRenderer::ApplyBackgroundFilters using ApplyImageFilter API.

BUG=613233
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Anton Obzhirov, 2016-07-12 10:23:23]

Description was changed from

==========
Put background filters on the makeWithFilter() fast-path

Implements GLRenderer::ApplyBackgroundFilters using ApplyImageFilter API.

BUG=613233
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Put background filters on the makeWithFilter() fast-path

Implements GLRenderer::ApplyBackgroundFilters using ApplyImageFilter API.

BUG=613233
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Anton Obzhirov, 2016-07-12 10:23:23]

a.obzhirov@samsung.com changed reviewers:
+ senorblanco@google.com

[Anton Obzhirov, 2016-07-12 10:28:00]

On 2016/07/12 10:23:23, Anton Obzhirov wrote:
> mailto:a.obzhirov@samsung.com changed reviewers:
> + mailto:senorblanco@google.com

Hi, this is my first attempt, not sure if it is the right approach so share it
with you to check. 
I didn't add background_tex_transform uniform since there is a way to reuse
locations.backdrop_rect.
But may be I are missing some cases.

[Stephen White, 2016-07-12 16:37:46]

The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run

[Stephen White, 2016-07-12 16:37:52]

https://codereview.chromium.org/2136003003/diff/1/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/2136003003/diff/1/cc/output/gl_renderer.cc#ne...
cc/output/gl_renderer.cc:1327:
background_texture_size.set_height(background_image->height());
It looks like the backdrop_rect is also used when doing shader-based blending,
and setting an empty backdrop_rect may cause problems for that case.

Perhaps we should simply modify background_rect just below the call to
ApplyBackgroundFilters() above to reflect the background filters case, and leave
it unmodified otherwise, so that shader-based blending continues to work.

We should also test the combination of the two. I'm actually not certain what
the correct background_rect is in that case; I suppose it depends on the order
of operations.

[commit-bot: I haz the power, 2016-07-12 16:38:53]

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

[commit-bot: I haz the power, 2016-07-12 17:24:41]

The CQ bit was unchecked by commit-bot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-12 17:24:42]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Anton Obzhirov, 2016-07-14 12:32:09]

https://codereview.chromium.org/2136003003/diff/1/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/2136003003/diff/1/cc/output/gl_renderer.cc#ne...
cc/output/gl_renderer.cc:1327:
background_texture_size.set_height(background_image->height());
On 2016/07/12 16:37:52, Stephen White wrote:
> It looks like the backdrop_rect is also used when doing shader-based blending,
> and setting an empty backdrop_rect may cause problems for that case.
> 
> Perhaps we should simply modify background_rect just below the call to
> ApplyBackgroundFilters() above to reflect the background filters case, and
leave
> it unmodified otherwise, so that shader-based blending continues to work.
> 
> We should also test the combination of the two. I'm actually not certain what
> the correct background_rect is in that case; I suppose it depends on the order
> of operations.
Yes, you are right, I'll move it up as you suggested. I also couldn't find a
test with blending and background filter, may be new one should be added.