Part 1/3 (compositor) of adding with device scale factor to transport surfaces

Part 1/3 (compositor) of adding with device scale factor to transport surfaces

Add a DeviceScaleFactor method to RendererClient (and all
its instances) to get the scale factor from the layer tree to the
output surface.

Add a scale factor argument to OutputSurface::Reshape

Replace OutputSurface::Reshape's use of WebGraphicsContext3D's
reshape with reshapeWithScaleFactor.

Implement reshapeWithScaleFactor for command buffer
based WebGraphicsContext3Ds, but stop propagating the
scale factor argument there.

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

[ccameron, 2013-05-22 08:37:28]

Adding jamesr for cc/ changes
Adding kbr for content/ and webkit/ changes.

This is part 1 of a 3-part series which sends the correct
scale factor to CompositingIOSurfaceMac, to avoid scaled
flickering when moving across screens.

The subsequent parts are
https://codereview.chromium.org/15685003/ (GL)
https://codereview.chromium.org/15689003/ (RWH/IOSurface)

https://codereview.chromium.org/15688002/diff/1/content/common/gpu/client/web...
File content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc
(right):

https://codereview.chromium.org/15688002/diff/1/content/common/gpu/client/web...
content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc:578:
gl_->ResizeCHROMIUM(width, height);
This is where we drop the scale factor on the floor and pick it up in the next
change, https://codereview.chromium.org/15685003/

[danakj, 2013-05-22 14:08:42]

https://codereview.chromium.org/15688002/diff/1/cc/output/gl_renderer_unittes...
File cc/output/gl_renderer_unittest.cc (right):

https://codereview.chromium.org/15688002/diff/1/cc/output/gl_renderer_unittes...
cc/output/gl_renderer_unittest.cc:170: return 1.f;
Can we have a test where this is != 1?

https://codereview.chromium.org/15688002/diff/1/cc/output/software_renderer_u...
File cc/output/software_renderer_unittest.cc (right):

https://codereview.chromium.org/15688002/diff/1/cc/output/software_renderer_u...
cc/output/software_renderer_unittest.cc:56: return 1.f;
And here?

[ccameron, 2013-05-22 18:26:11]

Added two more unit tests (one for LTHI -> WGC3D and another for
renderer->outputsurface).

https://codereview.chromium.org/15688002/diff/1/cc/output/gl_renderer_unittes...
File cc/output/gl_renderer_unittest.cc (right):

https://codereview.chromium.org/15688002/diff/1/cc/output/gl_renderer_unittes...
cc/output/gl_renderer_unittest.cc:170: return 1.f;
On 2013/05/22 14:08:42, danakj wrote:
> Can we have a test where this is != 1?

The test wasn't verifying any of the Reshape arguments before (viewport size is
always 1x1) ... I've added a test that verifies the size and the scale factor.

https://codereview.chromium.org/15688002/diff/1/cc/output/software_renderer_u...
File cc/output/software_renderer_unittest.cc (right):

https://codereview.chromium.org/15688002/diff/1/cc/output/software_renderer_u...
cc/output/software_renderer_unittest.cc:56: return 1.f;
On 2013/05/22 14:08:42, danakj wrote:
> And here?

Software renderer doesn't use this value yet -- the output being tested (actual
pixel values rendered) is independent of this value.

[danakj, 2013-05-22 18:28:15]

https://codereview.chromium.org/15688002/diff/8001/cc/output/gl_renderer_unit...
File cc/output/gl_renderer_unittest.cc (right):

https://codereview.chromium.org/15688002/diff/8001/cc/output/gl_renderer_unit...
cc/output/gl_renderer_unittest.cc:173: return 1.f;
This should return scale_factor_ right?

[ccameron, 2013-05-22 18:34:55]

https://codereview.chromium.org/15688002/diff/8001/cc/output/gl_renderer_unit...
File cc/output/gl_renderer_unittest.cc (right):

https://codereview.chromium.org/15688002/diff/8001/cc/output/gl_renderer_unit...
cc/output/gl_renderer_unittest.cc:173: return 1.f;
On 2013/05/22 18:28:16, danakj wrote:
> This should return scale_factor_ right?

Yes -- fixed. The test verifies that DeviceScaleFactor() is found on both sides,
so this worked (just didn't test the new value). I was considering adding
arguments to verify to DrawFrame to be more emphatic (lmk if that seems more
reasonable).

[danakj, 2013-05-22 18:39:16]

Thanks for the test, it looks good.

/me passes back to jamesr for the WGC3D changes.

https://codereview.chromium.org/15688002/diff/9019/cc/output/gl_renderer_unit...
File cc/output/gl_renderer_unittest.cc (right):

https://codereview.chromium.org/15688002/diff/9019/cc/output/gl_renderer_unit...
cc/output/gl_renderer_unittest.cc:1514: clear_viewport_changed();
nit: 4space indent

[Ken Russell (switch to Gerrit), 2013-05-22 18:50:54]

lgtm

https://codereview.chromium.org/15688002/diff/9019/cc/output/gl_renderer_unit...
File cc/output/gl_renderer_unittest.cc (right):

https://codereview.chromium.org/15688002/diff/9019/cc/output/gl_renderer_unit...
cc/output/gl_renderer_unittest.cc:1548: renderer_.ViewportChanged();
Is there any way to check that these result in a particular call or calls?

https://codereview.chromium.org/15688002/diff/9019/content/common/gpu/client/...
File content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc
(right):

https://codereview.chromium.org/15688002/diff/9019/content/common/gpu/client/...
content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc:574: int
width, int height, float scale_factor) {
Presumably the scale factor will be passed down further in subsequent CLs?

https://codereview.chromium.org/15688002/diff/16001/webkit/gpu/webgraphicscon...
File webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.h (right):

https://codereview.chromium.org/15688002/diff/16001/webkit/gpu/webgraphicscon...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.h:81: virtual
void reshapeWithScaleFactor(int width, int height, float scaleFactor);
It looks like webgraphicscontext3d_in_process_impl.cc is unfortunately still in
the tree. Does it need to be updated too?

[jamesr, 2013-05-22 19:04:34]

lgtm. remember to update wgc3d_in_process_impl

[ccameron, 2013-05-22 19:24:43]

Thanks!!

https://codereview.chromium.org/15688002/diff/9019/cc/output/gl_renderer_unit...
File cc/output/gl_renderer_unittest.cc (right):

https://codereview.chromium.org/15688002/diff/9019/cc/output/gl_renderer_unit...
cc/output/gl_renderer_unittest.cc:1514: clear_viewport_changed();
On 2013/05/22 18:39:16, danakj wrote:
> nit: 4space indent

Done.

https://codereview.chromium.org/15688002/diff/9019/cc/output/gl_renderer_unit...
cc/output/gl_renderer_unittest.cc:1548: renderer_.ViewportChanged();
On 2013/05/22 18:50:54, kbr wrote:
> Is there any way to check that these result in a particular call or calls?

ViewportChanged() indicates that the RendererClient's values have changed and
need to be re-examined  -- if the call is removed then the EXPECT_CALL(Reshape)
fails.

From an architectural perspective, this approach is probably less robust than
having the renderer always check the values and elide redundant calls itself.

https://codereview.chromium.org/15688002/diff/9019/content/common/gpu/client/...
File content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc
(right):

https://codereview.chromium.org/15688002/diff/9019/content/common/gpu/client/...
content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc:574: int
width, int height, float scale_factor) {
On 2013/05/22 18:50:54, kbr wrote:
> Presumably the scale factor will be passed down further in subsequent CLs?

Exactly -- part 2 picks up here and send it to the image transport surface.

https://codereview.chromium.org/15688002/diff/16001/webkit/gpu/webgraphicscon...
File webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.h (right):

https://codereview.chromium.org/15688002/diff/16001/webkit/gpu/webgraphicscon...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.h:81: virtual
void reshapeWithScaleFactor(int width, int height, float scaleFactor);
On 2013/05/22 18:50:55, kbr wrote:
> It looks like webgraphicscontext3d_in_process_impl.cc is unfortunately still
in
> the tree. Does it need to be updated too?

Totally missed that. Fixed.

[commit-bot: I haz the power, 2013-05-23 23:37:02]

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

[commit-bot: I haz the power, 2013-05-24 00:04:54]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-05-24 09:36:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ccameron@chromium.org/15688002/33001

[commit-bot: I haz the power, 2013-05-24 10:09:14]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-05-24 18:35:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ccameron@chromium.org/15688002/54001

[commit-bot: I haz the power, 2013-05-24 22:01:53]

Change committed as 202189