gpu: Allow BufferData size=0 for pixel transfer buffer objects.

gpu: Allow BufferData size=0 for pixel transfer buffer objects.

This makes it possible to release all shared memory used by a
pixel transfer buffer without destroying the Id.

BUG=163482
TEST=manual


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

[greggman, 2012-11-30 09:13:15]

https://codereview.chromium.org/11415223/diff/1/gpu/command_buffer/client/gle...
File gpu/command_buffer/client/gles2_implementation.cc (left):

https://codereview.chromium.org/11415223/diff/1/gpu/command_buffer/client/gle...
gpu/command_buffer/client/gles2_implementation.cc:1200: if (size == 0) {
this removal I understand

https://codereview.chromium.org/11415223/diff/1/gpu/command_buffer/client/gle...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/11415223/diff/1/gpu/command_buffer/client/gle...
gpu/command_buffer/client/gles2_implementation.cc:1221: if (size) {
this addition I don't understand. It seems like you'd still want to call
CreateBuffer here and it will just create a zero sized buffer.

That way everything *should* just work.

   GLsizei someSizeThatHappensToBe0
   glBindBuffer(GL_PIXEL_UNPACK..., buf);
   glBufferData(GL_PIXEL_UNPACK..., 0, ptr, usage);
   // this should return a valid pointer just like
   // malloc(0) does
   glMapBufferCHROMIUM(GL_PIXEL_UNPACK,...);
   glTexImage2D(..., widthOf0, heightOf0, ....);
 
etc.

If you skip creating zero sized buffers then several other checks will fail if
called with a zero sized request. 

Right? No?

[reveman, 2012-11-30 09:30:33]

https://codereview.chromium.org/11415223/diff/1/gpu/command_buffer/client/gle...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/11415223/diff/1/gpu/command_buffer/client/gle...
gpu/command_buffer/client/gles2_implementation.cc:1221: if (size) {
On 2012/11/30 09:13:15, greggman wrote:
> this addition I don't understand. It seems like you'd still want to call
> CreateBuffer here and it will just create a zero sized buffer.
> 
> That way everything *should* just work.
> 
>    GLsizei someSizeThatHappensToBe0
>    glBindBuffer(GL_PIXEL_UNPACK..., buf);
>    glBufferData(GL_PIXEL_UNPACK..., 0, ptr, usage);
>    // this should return a valid pointer just like
>    // malloc(0) does
>    glMapBufferCHROMIUM(GL_PIXEL_UNPACK,...);
>    glTexImage2D(..., widthOf0, heightOf0, ....);
>  
> etc.
> 
> If you skip creating zero sized buffers then several other checks will fail if
> called with a zero sized request. 
> 
> Right? No?
> 

Yes, I think you're right. I might need to update the BufferTracker for this to
work. I'll look at what's needed.

[reveman, 2012-11-30 09:57:12]

Latest patch creates a Buffer with zero size. I also added a unit test. PTAL.

[greggman, 2012-12-03 04:29:07]

On 2012/11/30 09:57:12, David Reveman wrote:
> Latest patch creates a Buffer with zero size. I also added a unit test. PTAL.

you shouldn't need those checks for 0 in buffer_tracker.cc. That's my bug.
Here's a CL that fixes it

https://codereview.chromium.org/11419280/

[reveman, 2012-12-03 12:53:19]

On 2012/12/03 04:29:07, greggman wrote:
> On 2012/11/30 09:57:12, David Reveman wrote:
> > Latest patch creates a Buffer with zero size. I also added a unit test.
PTAL.
> 
> you shouldn't need those checks for 0 in buffer_tracker.cc. That's my bug.
> Here's a CL that fixes it
> 
> https://codereview.chromium.org/11419280/

Ok, do you think you can land that today? We urgently need this for impl-side
painting.

[greggman, 2012-12-03 13:56:18]

On 2012/12/03 12:53:19, David Reveman wrote:
> On 2012/12/03 04:29:07, greggman wrote:
> > On 2012/11/30 09:57:12, David Reveman wrote:
> > > Latest patch creates a Buffer with zero size. I also added a unit test.
> PTAL.
> > 
> > you shouldn't need those checks for 0 in buffer_tracker.cc. That's my bug.
> > Here's a CL that fixes it
> > 
> > https://codereview.chromium.org/11419280/
> 
> Ok, do you think you can land that today? We urgently need this for impl-side
> painting.

go ahead and land yours and I'll fix it when I check that other CL in

lgtm

[commit-bot: I haz the power, 2012-12-03 14:04:13]

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

[commit-bot: I haz the power, 2012-12-03 15:22:39]

Retried try job too often on mac_rel for step(s) ash_unittests

[commit-bot: I haz the power, 2012-12-03 16:22:37]

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

[commit-bot: I haz the power, 2012-12-03 16:56:14]

Retried try job too often on linux_rel for step(s) ash_unittests

[commit-bot: I haz the power, 2012-12-03 18:11:04]

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

[commit-bot: I haz the power, 2012-12-03 18:51:44]

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

[commit-bot: I haz the power, 2012-12-03 19:04:34]

Change committed as 170776