Added a simple smoke test for Graphics3D. It attempts to exercise the entire graphics-3d pipeline...

Added a simple smoke test for Graphics3D. It attempts to exercise the entire graphics-3d pipeline modulo compositor.
BUG=93809
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=99387

[brettw, 2011-08-31 20:13:39]

LGTM

[piman, 2011-08-31 20:18:54]

http://codereview.chromium.org/7818004/diff/1/ppapi/tests/test_graphics_3d.cc
File ppapi/tests/test_graphics_3d.cc (right):

http://codereview.chromium.org/7818004/diff/1/ppapi/tests/test_graphics_3d.cc...
ppapi/tests/test_graphics_3d.cc:54:
graphics_3d_->Create(instance_->pp_instance(), 0, attribs);
use pp::Graphics3D to avoid manual refcounting? Saves a few lines too.

http://codereview.chromium.org/7818004/diff/1/ppapi/tests/test_graphics_3d.cc...
ppapi/tests/test_graphics_3d.cc:55: ASSERT_NE(context, NULL);
nit: s/NULL/0/ (PP_Resource is not a pointer).

http://codereview.chromium.org/7818004/diff/1/ppapi/tests/test_graphics_3d.cc...
ppapi/tests/test_graphics_3d.cc:65: std::string error = TestPixel(context,
width/2, height/2, red_color);
Here you rely on the swap buffers being non-destructive, while true today, it
will not be true in the long term. It would be better to move this to be before
the swap.

http://codereview.chromium.org/7818004/diff/1/ppapi/tests/test_graphics_3d.cc...
ppapi/tests/test_graphics_3d.cc:79:
PP_MakeCompletionCallback(SwapBuffersCallback, &swap_buffers));
You should be able to use the TestCompletionCallback instead of re-implementing
this all.

[alokp, 2011-08-31 20:48:28]

http://codereview.chromium.org/7818004/diff/1/ppapi/tests/test_graphics_3d.cc
File ppapi/tests/test_graphics_3d.cc (right):

http://codereview.chromium.org/7818004/diff/1/ppapi/tests/test_graphics_3d.cc...
ppapi/tests/test_graphics_3d.cc:54:
graphics_3d_->Create(instance_->pp_instance(), 0, attribs);
On 2011/08/31 20:18:54, piman wrote:
> use pp::Graphics3D to avoid manual refcounting? Saves a few lines too.

Done.

http://codereview.chromium.org/7818004/diff/1/ppapi/tests/test_graphics_3d.cc...
ppapi/tests/test_graphics_3d.cc:55: ASSERT_NE(context, NULL);
On 2011/08/31 20:18:54, piman wrote:
> nit: s/NULL/0/ (PP_Resource is not a pointer).

Done.

http://codereview.chromium.org/7818004/diff/1/ppapi/tests/test_graphics_3d.cc...
ppapi/tests/test_graphics_3d.cc:65: std::string error = TestPixel(context,
width/2, height/2, red_color);
On 2011/08/31 20:18:54, piman wrote:
> Here you rely on the swap buffers being non-destructive, while true today, it
> will not be true in the long term. It would be better to move this to be
before
> the swap.

Done.

http://codereview.chromium.org/7818004/diff/1/ppapi/tests/test_graphics_3d.cc...
ppapi/tests/test_graphics_3d.cc:79:
PP_MakeCompletionCallback(SwapBuffersCallback, &swap_buffers));
Nice utility. Thanks for pointing it out. Done.

[piman, 2011-08-31 21:02:38]

LGTM + 1 thing

http://codereview.chromium.org/7818004/diff/6002/ppapi/tests/test_graphics_3d.cc
File ppapi/tests/test_graphics_3d.cc (right):

http://codereview.chromium.org/7818004/diff/6002/ppapi/tests/test_graphics_3d...
ppapi/tests/test_graphics_3d.cc:20:
pp::Module::Get()->GetBrowserInterface(PPB_GRAPHICS_3D_DEV_INTERFACE));
Don't need this anymore?

[alokp, 2011-09-01 21:05:16]

http://codereview.chromium.org/7818004/diff/6002/ppapi/tests/test_graphics_3d.cc
File ppapi/tests/test_graphics_3d.cc (right):

http://codereview.chromium.org/7818004/diff/6002/ppapi/tests/test_graphics_3d...
ppapi/tests/test_graphics_3d.cc:20:
pp::Module::Get()->GetBrowserInterface(PPB_GRAPHICS_3D_DEV_INTERFACE));
On 2011/08/31 21:02:38, piman wrote:
> Don't need this anymore?

Done.

[piman, 2011-09-02 00:30:04]

LGTM

[commit-bot: I haz the power, 2011-09-02 16:31:42]

Change committed as 99387