cc: Make one-copy tile initialization mechanism use immutable textures.

cc: Make one-copy tile
initialization mechanism use immutable textures.

Currently, one-copy doesn't use immutable texture while the compositor intends
to use immutable texture. It's because one-copy tries to copy source texture to
unallocated destination texture. In the case, GPU process allocates the
destination texture as normal texture.

In theory, it speeds up drawing speed, because immutable texture skips texture
validation on drawing call. However, I didn't see actual performance difference
in Linux and Android.

BUG=406404
Committed: https://crrev.com/eebda5986284b9fce42c2ab8de5d2686b079e52e
Cr-Commit-Position: refs/heads/master@{#305077}

[dshwang, 2014-11-20 16:54:28]

dongseong.hwang@intel.com changed reviewers:
+ reveman@chromium.org

[dshwang, 2014-11-20 16:58:40]

Hi, could you review?

https://codereview.chromium.org/737373004/diff/1/cc/resources/resource_provid...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/737373004/diff/1/cc/resources/resource_provid...
cc/resources/resource_provider.cc:2080: LazyAllocate(dest_resource);
It doesn't regress perf because CopyTextureChromium calls glTexImage2D if the
compositor doesn't allocate the dest texture.
See
https://code.google.com/p/chromium/codesearch#chromium/src/gpu/command_buffer...

https://codereview.chromium.org/737373004/diff/1/ui/gl/gl_image_memory.cc
File ui/gl/gl_image_memory.cc (right):

https://codereview.chromium.org/737373004/diff/1/ui/gl/gl_image_memory.cc#new...
ui/gl/gl_image_memory.cc:184: memory_);
glTexImage2D isn't allowed after calling glTexStorage2D.
In addition we can use glTexSubImage2D safely because
GLES2DecoderImpl::DoCopyTextureCHROMIUM makes sure the bound texture is already
allocated.
https://code.google.com/p/chromium/codesearch#chromium/src/gpu/command_buffer...

[reveman, 2014-11-20 17:44:36]

Looks good. Does this have a performance impact? In that case, please explain
why in the description.

Can you also update the title. It's not clear if it states a problem, the
current condition or a change. Should it be "cc: Make one-copy tile
initialization mechanism use immutable textures." ?

[dshwang, 2014-11-20 17:48:53]

On 2014/11/20 17:44:36, reveman wrote:
> Looks good. Does this have a performance impact? In that case, please explain
> why in the description.

In theory, it speeds up drawing speed, because it uses immutable texture, which
skips texture validation on drawing call.
I cannot see real perf difference in Linux. Let me check Android.

> Can you also update the title. It's not clear if it states a problem, the
> current condition or a change. Should it be "cc: Make one-copy tile
> initialization mechanism use immutable textures." ?

Thx for good title. Done.

[reveman, 2014-11-20 17:55:18]

On 2014/11/20 17:48:53, dshwang wrote:
> On 2014/11/20 17:44:36, reveman wrote:
> > Looks good. Does this have a performance impact? In that case, please
explain
> > why in the description.
> 
> In theory, it speeds up drawing speed, because it uses immutable texture,
which
> skips texture validation on drawing call.
> I cannot see real perf difference in Linux. Let me check Android.

Makes sense. Would be nice if you could mention this in the description.

> 
> > Can you also update the title. It's not clear if it states a problem, the
> > current condition or a change. Should it be "cc: Make one-copy tile
> > initialization mechanism use immutable textures." ?
> 
> Thx for good title. Done.

[dshwang, 2014-11-20 18:14:43]

On 2014/11/20 17:55:18, reveman wrote:
> On 2014/11/20 17:48:53, dshwang wrote:
> > On 2014/11/20 17:44:36, reveman wrote:
> > > Looks good. Does this have a performance impact? In that case, please
> explain
> > > why in the description.
> > 
> > In theory, it speeds up drawing speed, because it uses immutable texture,
> which
> > skips texture validation on drawing call.
> > I cannot see real perf difference in Linux. Let me check Android.
> 
> Makes sense. Would be nice if you could mention this in the description.

Yes, Done. I cannot see perf difference on Android also. I tested it using
http://media.24ways.org/2009/15/space.html and blind_perf.animation

[reveman, 2014-11-20 18:57:15]

lgtm with nit

https://codereview.chromium.org/737373004/diff/20001/ui/gl/gl_image_memory.cc
File ui/gl/gl_image_memory.cc (right):

https://codereview.chromium.org/737373004/diff/20001/ui/gl/gl_image_memory.cc...
ui/gl/gl_image_memory.cc:184: memory_);
nit: Please make the style consistent with TexSubImage2D call in
::DoBindTexImage for now ("//" style comments and one argument per line) I'll cl
format this whole file in a follow up.

[dshwang, 2014-11-20 19:15:25]

On 2014/11/20 18:57:15, reveman wrote:
> lgtm with nit

Thank you for review!

> 
> https://codereview.chromium.org/737373004/diff/20001/ui/gl/gl_image_memory.cc
> File ui/gl/gl_image_memory.cc (right):
> 
>
https://codereview.chromium.org/737373004/diff/20001/ui/gl/gl_image_memory.cc...
> ui/gl/gl_image_memory.cc:184: memory_);
> nit: Please make the style consistent with TexSubImage2D call in
> ::DoBindTexImage for now ("//" style comments and one argument per line) I'll
cl
> format this whole file in a follow up.

yes, Done.

[dshwang, 2014-11-20 19:15:41]

The CQ bit was checked by dongseong.hwang@intel.com

[commit-bot: I haz the power, 2014-11-20 19:16:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/737373004/40001

[commit-bot: I haz the power, 2014-11-20 20:55:38]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-11-20 20:56:13]

Patchset 3 (id:??) landed as
https://crrev.com/eebda5986284b9fce42c2ab8de5d2686b079e52e
Cr-Commit-Position: refs/heads/master@{#305077}