Fix glTexSubImage2D for non-32bpp formats.

Fix glTexSubImage2D for non-32bpp formats.

The non-mapped upload path for glTexSubImage2D is assuming 32bpp
blindly, for all formats.

TEST=local build, run on ARM (forcing non-mapped path)
BUG=None

Change-Id: Ib8ac46646b27faeeb8c19ebb519d321e25c1a8cb


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=163477

[sheu, 2012-10-22 20:28:44]

PTAL

[enne (OOO), 2012-10-22 20:36:14]

http://codereview.chromium.org/11229040/diff/3001/cc/texture_uploader.cc
File cc/texture_uploader.cc (right):

http://codereview.chromium.org/11229040/diff/3001/cc/texture_uploader.cc#newc...
cc/texture_uploader.cc:28: // synchronize access.
Please do not update style along with behavior changes in the same patch.

http://codereview.chromium.org/11229040/diff/3001/cc/texture_uploader.cc#newc...
cc/texture_uploader.cc:224: switch (format) {
Can you not duplicate this switch statement from below? Maybe this can go into a
static helper function?

It looks like texture.cc also makes this same mistake.

[sheu, 2012-10-22 21:31:59]

New version uploaded.

http://codereview.chromium.org/11229040/diff/3001/cc/texture_uploader.cc
File cc/texture_uploader.cc (right):

http://codereview.chromium.org/11229040/diff/3001/cc/texture_uploader.cc#newc...
cc/texture_uploader.cc:28: // synchronize access.
I thought this would be small enough to slip in.  I guess not :-)

http://codereview.chromium.org/11229040/diff/3001/cc/texture_uploader.cc#newc...
cc/texture_uploader.cc:224: switch (format) {
On 2012/10/22 20:36:14, enne wrote:
> Can you not duplicate this switch statement from below? Maybe this can go into
a
> static helper function?
> 
> It looks like texture.cc also makes this same mistake.

Done.

[jamesr, 2012-10-22 21:47:59]

lgtm

http://codereview.chromium.org/11229040/diff/9001/cc/texture_uploader.cc
File cc/texture_uploader.cc (right):

http://codereview.chromium.org/11229040/diff/9001/cc/texture_uploader.cc#newc...
cc/texture_uploader.cc:222: source_rect.width() * source_rect.height() *
bytes_per_pixel;
don't line wrap this, just let it go long. we'll rewrap all of these when we
change indentation, wrapping preemptively is just creating extra unnecessary
work for us

[sheu, 2012-10-22 21:56:47]

http://codereview.chromium.org/11229040/diff/9001/cc/texture_uploader.cc
File cc/texture_uploader.cc (right):

http://codereview.chromium.org/11229040/diff/9001/cc/texture_uploader.cc#newc...
cc/texture_uploader.cc:222: source_rect.width() * source_rect.height() *
bytes_per_pixel;
On 2012/10/22 21:47:59, jamesr wrote:
> don't line wrap this, just let it go long. we'll rewrap all of these when we
> change indentation, wrapping preemptively is just creating extra unnecessary
> work for us

Done.

[commit-bot: I haz the power, 2012-10-22 21:57:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sheu@chromium.org/11229040/11002

[commit-bot: I haz the power, 2012-10-22 23:45:58]

Retried try job too often for step(s) browser_tests

[sheu, 2012-10-22 23:58:21]

Somebody chumped in a change ahead of me.  Rebasing.

[commit-bot: I haz the power, 2012-10-22 23:59:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sheu@chromium.org/11229040/12001

[commit-bot: I haz the power, 2012-10-23 02:03:35]

Change committed as 163477