gpu: Render to a framebuffer object instead of an offscreen surface in gpu_perftests.

gpu: Render to a framebuffer object instead of an offscreen surface in gpu_perftests.

Offscreen surfaces are implemented in different ways on different devices.
Using a framebuffer object allows gpu_perftests to run where a drawable offscreen surface is not available (i.e: mac) and in general to have a more consistent behavior across different drivers.

BUG=423481
Committed: https://crrev.com/586c7d01c01db1f108f5c02f9ded90b2d1b0c7e3
Cr-Commit-Position: refs/heads/master@{#316909}

[Daniele Castagna, 2015-02-18 03:22:55]

dcastagna@chromium.org changed reviewers:
+ reveman@chromium.org

[Daniele Castagna, 2015-02-18 15:46:37]

PTAL.

As per offline discussion the class FramebufferObject has been removed.

[reveman, 2015-02-18 16:21:34]

lgtm with nits

https://codereview.chromium.org/939553002/diff/40001/gpu/perftests/texture_up...
File gpu/perftests/texture_upload_perftest.cc (right):

https://codereview.chromium.org/939553002/diff/40001/gpu/perftests/texture_up...
gpu/perftests/texture_upload_perftest.cc:174: if (framebuffer_object_ != 0) {
nit: I think just "if (framebuffer_object_)" is preferred but I can see "!= 0"
being used in this file already so you can keep this for consistency if you
like.

btw, do we really need all these "if" statements in TearDown? When is it OK for
framebuffer_object_ to be 0?

https://codereview.chromium.org/939553002/diff/40001/gpu/perftests/texture_up...
gpu/perftests/texture_upload_perftest.cc:178: if (color_texture_ != 0) {
nit: maybe remove "!= 0"

https://codereview.chromium.org/939553002/diff/40001/gpu/perftests/texture_up...
gpu/perftests/texture_upload_perftest.cc:194: DCHECK_GT(framebuffer_object_,
0u);
nit: DCHECK_NE as less than 0 is not possible

[Daniele Castagna, 2015-02-18 18:46:38]

New patchsets have been uploaded after l-g-t-m from reveman@chromium.org

[Daniele Castagna, 2015-02-18 18:47:07]

https://codereview.chromium.org/939553002/diff/40001/gpu/perftests/texture_up...
File gpu/perftests/texture_upload_perftest.cc (right):

https://codereview.chromium.org/939553002/diff/40001/gpu/perftests/texture_up...
gpu/perftests/texture_upload_perftest.cc:174: if (framebuffer_object_ != 0) {
On 2015/02/18 at 16:21:33, reveman wrote:
> nit: I think just "if (framebuffer_object_)" is preferred but I can see "!= 0"
being used in this file already so you can keep this for consistency if you
like.
> 
> btw, do we really need all these "if" statements in TearDown? When is it OK
for framebuffer_object_ to be 0?

We don't need them. I checked the documentation for all those functions and a
value of 0 is always ignored.

https://codereview.chromium.org/939553002/diff/40001/gpu/perftests/texture_up...
gpu/perftests/texture_upload_perftest.cc:178: if (color_texture_ != 0) {
On 2015/02/18 at 16:21:33, reveman wrote:
> nit: maybe remove "!= 0"

Removed all the if (blabla_ != 0)

https://codereview.chromium.org/939553002/diff/40001/gpu/perftests/texture_up...
gpu/perftests/texture_upload_perftest.cc:194: DCHECK_GT(framebuffer_object_,
0u);
On 2015/02/18 at 16:21:33, reveman wrote:
> nit: DCHECK_NE as less than 0 is not possible

Done.

[Daniele Castagna, 2015-02-18 18:52:19]

dcastagna@chromium.org changed reviewers:
+ piman@chromium.org

[piman, 2015-02-18 22:49:22]

lgtm

[Daniele Castagna, 2015-02-18 22:53:14]

The CQ bit was checked by dcastagna@chromium.org

[commit-bot: I haz the power, 2015-02-18 22:54:46]

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

[commit-bot: I haz the power, 2015-02-18 22:59:45]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-18 23:03:28]

Patchset 4 (id:??) landed as
https://crrev.com/586c7d01c01db1f108f5c02f9ded90b2d1b0c7e3
Cr-Commit-Position: refs/heads/master@{#316909}