|
|
Created:
6 years, 8 months ago by reveman Modified:
6 years, 7 months ago Reviewers:
piman CC:
chromium-reviews, piman+watch_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
Descriptiongpu: Optimize and cleanup shader code used for CHROMIUM_copy_texture.
Removes all dependent texture lookups and makes sure we're not using
unnecessarily high precision for texture coordinates.
BUG=269808
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266801
Patch Set 1 #
Total comments: 2
Patch Set 2 : remove unnecessary blanklines and use 0u when initializing GLuint #Patch Set 3 : fix DCHECK_LTs #
Messages
Total messages: 26 (0 generated)
lgtm https://codereview.chromium.org/257803011/diff/1/gpu/command_buffer/service/g... File gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc (right): https://codereview.chromium.org/257803011/diff/1/gpu/command_buffer/service/g... gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc:180: fragment_shaders_(NUM_FRAGMENT_SHADERS, 0), nit: if we're going to preallocate and it's fixed size, should we just use an array rather than a vector?
https://codereview.chromium.org/257803011/diff/1/gpu/command_buffer/service/g... File gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc (right): https://codereview.chromium.org/257803011/diff/1/gpu/command_buffer/service/g... gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc:180: fragment_shaders_(NUM_FRAGMENT_SHADERS, 0), On 2014/04/28 17:28:15, piman wrote: > nit: if we're going to preallocate and it's fixed size, should we just use an > array rather than a vector? I think it's worth using separate heap allocations here so we don't have to expose NUM_VERTEX/FRAGMENT_SHADERS in the header and can keep all these details in the anonymous namespace. We could use scoped_ptr<GLuint[]> but I prefer to use vectors as it makes initializing all values to 0 and deleting shaders when ::Destroy() is called nice and pretty.
The CQ bit was checked by reveman@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/reveman@chromium.org/257803011/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: tryserver.chromium on chromium_presubmit
The CQ bit was checked by reveman@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/reveman@chromium.org/257803011/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: tryserver.chromium on win_chromium_rel
The CQ bit was checked by reveman@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/reveman@chromium.org/257803011/30001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: tryserver.chromium on linux_chromium_chromeos_rel
The CQ bit was checked by reveman@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/reveman@chromium.org/257803011/30001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: tryserver.chromium on chromium_presubmit
The CQ bit was checked by reveman@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/reveman@chromium.org/257803011/30001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: tryserver.chromium on linux_chromium_chromeos_rel
The CQ bit was checked by reveman@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/reveman@chromium.org/257803011/30001
Message was sent while issue was closed.
Change committed as 266801 |