gpu: Refactor and cleanup CHROMIUM_copy_texture implementation.

gpu: Refactor and cleanup CHROMIUM_copy_texture implementation.

This removes duplicate shader code, makes it easier to support
additional texture targets and implements on demand initialization
of shaders.

BUG=366710
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266351

[reveman, 2014-04-25 03:03:05]

FYI, I will be adding TEXTURE_RECTANGLE_ARB support in a follow up patch.

[piman, 2014-04-25 03:30:40]

I left some comments. I think we can do even more cleanup, dunno if you want to
do it with this CL or leave it for later.

https://codereview.chromium.org/255763002/diff/20001/gpu/command_buffer/servi...
File gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc (right):

https://codereview.chromium.org/255763002/diff/20001/gpu/command_buffer/servi...
gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc:32: -1.0f,  1.0f,
0.0f, 1.0f };
This should be 2 components per vertex instead of 4... 0, 1 is implied for z, w
if we have vec2's.

https://codereview.chromium.org/255763002/diff/20001/gpu/command_buffer/servi...
gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc:36:
SHADER_COPY_TEXTURE_FLIP_Y,
nit: it'd be nice to be consistent between FLIP_Y and FLIPY.

https://codereview.chromium.org/255763002/diff/20001/gpu/command_buffer/servi...
gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc:74: gl_FragColor =
TextureLookup(u_texSampler, vec2(v_uv.s, 1.0 - v_uv.t));
Maybe for later... those shaders would be more efficient if we did the uv math
per vertex instead of per pixel (and we do uv math there anyway).

Actually, we could make it a uniform too, to halve the number of shaders.

https://codereview.chromium.org/255763002/diff/20001/gpu/command_buffer/servi...
gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc:135:
SHADER_COPY_TEXTURE,                            // F P U
Shouldn't this last one be SHADER_COPY_TEXTURE_FLIP_Y?

https://codereview.chromium.org/255763002/diff/20001/gpu/command_buffer/servi...
File gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.h (right):

https://codereview.chromium.org/255763002/diff/20001/gpu/command_buffer/servi...
gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.h:61: typedef
std::map<ProgramMapKey, ProgramInfo> ProgramMap;
nit: hash_map, or even SmallMap.

[reveman, 2014-04-25 20:27:09]

PTAL. I prefer to keep changes that modify the shaders in a separate patch and
limit this to a refactor. I'll make sure to include all suggestions in the
follow up patch.

https://codereview.chromium.org/255763002/diff/20001/gpu/command_buffer/servi...
File gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc (right):

https://codereview.chromium.org/255763002/diff/20001/gpu/command_buffer/servi...
gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc:32: -1.0f,  1.0f,
0.0f, 1.0f };
On 2014/04/25 03:30:40, piman wrote:
> This should be 2 components per vertex instead of 4... 0, 1 is implied for z,
w
> if we have vec2's.

I'll fix this in a follow up.

https://codereview.chromium.org/255763002/diff/20001/gpu/command_buffer/servi...
gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc:36:
SHADER_COPY_TEXTURE_FLIP_Y,
On 2014/04/25 03:30:40, piman wrote:
> nit: it'd be nice to be consistent between FLIP_Y and FLIPY.

Done.

https://codereview.chromium.org/255763002/diff/20001/gpu/command_buffer/servi...
gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc:74: gl_FragColor =
TextureLookup(u_texSampler, vec2(v_uv.s, 1.0 - v_uv.t));
On 2014/04/25 03:30:40, piman wrote:
> Maybe for later... those shaders would be more efficient if we did the uv math
> per vertex instead of per pixel (and we do uv math there anyway).
> 
> Actually, we could make it a uniform too, to halve the number of shaders.

I'll do this in a follow up. I need to change this to support TEXTURE_RECTANGLE
anyhow and I'll make sure to remove all this per-fragment math while at it.

https://codereview.chromium.org/255763002/diff/20001/gpu/command_buffer/servi...
gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc:135:
SHADER_COPY_TEXTURE,                            // F P U
On 2014/04/25 03:30:40, piman wrote:
> Shouldn't this last one be SHADER_COPY_TEXTURE_FLIP_Y?

Sure, I assume these last two are invalid configurations that we never hit but
having the premult and unpremult cancel each other out makes some sense..

https://codereview.chromium.org/255763002/diff/20001/gpu/command_buffer/servi...
File gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.h (right):

https://codereview.chromium.org/255763002/diff/20001/gpu/command_buffer/servi...
gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.h:61: typedef
std::map<ProgramMapKey, ProgramInfo> ProgramMap;
On 2014/04/25 03:30:40, piman wrote:
> nit: hash_map, or even SmallMap.

Done.

[piman, 2014-04-25 20:54:17]

lgtm

[reveman, 2014-04-25 20:54:44]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-04-25 22:03:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/255763002/40001

[commit-bot: I haz the power, 2014-04-26 10:14:25]

Change committed as 266351