cc: Remove most methods from RendererClient.

cc: Remove most methods from RendererClient.

This class is being used to pass data to the Renderer, but it's more
clear if we just pass the data directly when we call DrawFrame.

There are also some interesting round trips where OutputSurface sets
data on the OutputSurfaceClient (LayerTreeHostImpl) which just stores
the data. Then the data is accessed through the RendererClient or
on the LTHI directly, but in places that could just query it from the
OutputSurface directly.

This patch unravels one of these loops with HasExternalStencilTest().

The remaining two getters on RendererClient are called in both DrawFrame
and in SwapBuffers, so I'm not sure if we want to pass them directly to
both functions (redundency is error-prone) or we have some other nice
idea to get rid of them.

R=enne
BUG=

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

[enne (OOO), 2013-09-07 00:37:42]

lgtm

[aelias_OOO_until_Jul13, 2013-09-07 00:39:00]

When you rebase you'll pick up r221321, and that patch removes all non-Renderer
callers of DeviceViewport, so you can include it in the FrameData structure as
nonflipped_window_viewport as we discussed.  The patch also introduces
DeviceClip which can be similarly renamed to nonflipped_window_clip.

[danakj, 2013-09-07 00:42:41]

On Fri, Sep 6, 2013 at 8:39 PM, <aelias@chromium.org> wrote:

> When you rebase you'll pick up r221321, and that patch removes all
> non-Renderer
> callers of DeviceViewport, so you can include it in the FrameData
> structure as
> nonflipped_window_viewport as we discussed.  The patch also introduces
> DeviceClip which can be similarly renamed to nonflipped_window_clip.
>

Ok thanks! I just rebased and found DeviceClip. I'll look at them in a
separate CL to save everyone effort.


>
>
https://codereview.chromium.**org/23961003/<https://codereview.chromium.org/2...
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2013-09-07 00:43:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/23961003/6001

[commit-bot: I haz the power, 2013-09-07 00:43:44]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

Thanks for your patience.

HTTP Error 404: Not Found

[commit-bot: I haz the power, 2013-09-07 00:45:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/23961003/10001

[danakj, 2013-09-07 00:46:27]

On 2013/09/07 00:39:00, aelias wrote:
> When you rebase you'll pick up r221321, and that patch removes all
non-Renderer
> callers of DeviceViewport, so you can include it in the FrameData structure as
> nonflipped_window_viewport as we discussed.  The patch also introduces
> DeviceClip which can be similarly renamed to nonflipped_window_clip.

DeviceViewport() is still used in GLRenderer::SwapBuffers and I'm not sure the
best way to go about removing it from RendererClient in that scenario yet..

[slavi, 2013-09-07 00:51:08]

On 2013/09/07 00:46:27, danakj wrote:
> On 2013/09/07 00:39:00, aelias wrote:
> > When you rebase you'll pick up r221321, and that patch removes all
> non-Renderer
> > callers of DeviceViewport, so you can include it in the FrameData structure
as
> > nonflipped_window_viewport as we discussed.  The patch also introduces
> > DeviceClip which can be similarly renamed to nonflipped_window_clip.
> 
> DeviceViewport() is still used in GLRenderer::SwapBuffers and I'm not sure the
> best way to go about removing it from RendererClient in that scenario yet..

Pass as argument to DirectRenderer::DrawFrame and cache in
DirectRenderer::last_viewport_ for use after DrawFrame?

[aelias_OOO_until_Jul13, 2013-09-07 00:52:07]

On 2013/09/07 00:46:27, danakj wrote:
> On 2013/09/07 00:39:00, aelias wrote:
> > When you rebase you'll pick up r221321, and that patch removes all
> non-Renderer
> > callers of DeviceViewport, so you can include it in the FrameData structure
as
> > nonflipped_window_viewport as we discussed.  The patch also introduces
> > DeviceClip which can be similarly renamed to nonflipped_window_clip.
> 
> DeviceViewport() is still used in GLRenderer::SwapBuffers and I'm not sure the
> best way to go about removing it from RendererClient in that scenario yet..

I think it might be more correct to use
gfx::Rect(output_surface_->SurfaceSize()) there anyway... DeviceViewport should
have same value as that on all the platforms where partial-swap is shipping.

[commit-bot: I haz the power, 2013-09-07 07:16:44]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-09-07 14:42:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/23961003/10001

[commit-bot: I haz the power, 2013-09-07 14:42:57]

Failed to apply patch for cc/output/gl_renderer.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/output/gl_renderer.cc
  Hunk #6 FAILED at 1160.
  Hunk #7 succeeded at 2100 (offset -1 lines).
  Hunk #8 succeeded at 2513 (offset -1 lines).
  Hunk #9 succeeded at 3161 (offset -1 lines).
  1 out of 9 hunks FAILED -- saving rejects to file cc/output/gl_renderer.cc.rej

Patch:       cc/output/gl_renderer.cc
Index: cc/output/gl_renderer.cc
diff --git a/cc/output/gl_renderer.cc b/cc/output/gl_renderer.cc
index
f7e3a238a3a7260fc4d20d20abe559e5b3c14770..f6727d861f9d59405a87814c3aeaa12b3a258aea
100644
--- a/cc/output/gl_renderer.cc
+++ b/cc/output/gl_renderer.cc
@@ -127,12 +127,16 @@ struct GLRenderer::PendingAsyncReadPixels {
 };
 
 scoped_ptr<GLRenderer> GLRenderer::Create(RendererClient* client,
+                                          const LayerTreeSettings* settings,
                                           OutputSurface* output_surface,
                                           ResourceProvider* resource_provider,
                                           int highp_threshold_min,
                                           bool use_skia_gpu_backend) {
-  scoped_ptr<GLRenderer> renderer(new GLRenderer(
-      client, output_surface, resource_provider, highp_threshold_min));
+  scoped_ptr<GLRenderer> renderer(new GLRenderer(client,
+                                                 settings,
+                                                 output_surface,
+                                                 resource_provider,
+                                                 highp_threshold_min));
   if (!renderer->Initialize())
     return scoped_ptr<GLRenderer>();
   if (use_skia_gpu_backend) {
@@ -145,10 +149,11 @@ scoped_ptr<GLRenderer> GLRenderer::Create(RendererClient*
client,
 }
 
 GLRenderer::GLRenderer(RendererClient* client,
+                       const LayerTreeSettings* settings,
                        OutputSurface* output_surface,
                        ResourceProvider* resource_provider,
                        int highp_threshold_min)
-    : DirectRenderer(client, output_surface, resource_provider),
+    : DirectRenderer(client, settings, output_surface, resource_provider),
       offscreen_framebuffer_id_(0),
       shared_geometry_quad_(gfx::RectF(-0.5f, -0.5f, 1.0f, 1.0f)),
       context_(output_surface->context_provider()->Context3d()),
@@ -170,18 +175,15 @@ bool GLRenderer::Initialize() {
   if (!context_->makeContextCurrent())
     return false;
 
-  std::string unique_context_name = base::StringPrintf(
-      "%s-%p",
-      Settings().compositor_name.c_str(),
-      context_);
+  std::string unique_context_name =
+      base::StringPrintf("%s-%p", settings_->compositor_name.c_str(),
context_);
   context_->pushGroupMarkerEXT(unique_context_name.c_str());
 
   ContextProvider::Capabilities context_caps =
     output_surface_->context_provider()->ContextCapabilities();
 
   capabilities_.using_partial_swap =
-      Settings().partial_swap_enabled &&
-      context_caps.post_sub_buffer;
+      settings_->partial_swap_enabled && context_caps.post_sub_buffer;
 
   capabilities_.using_set_visibility = context_caps.set_visibility;
 
@@ -202,7 +204,7 @@ bool GLRenderer::Initialize() {
   capabilities_.using_offscreen_context3d = true;
 
   capabilities_.using_map_image =
-      Settings().use_map_image && context_caps.map_image;
+      settings_->use_map_image && context_caps.map_image;
 
   capabilities_.using_discard_framebuffer =
       context_caps.discard_framebuffer;
@@ -290,7 +292,7 @@ void GLRenderer::ViewportChanged() {
 void GLRenderer::ClearFramebuffer(DrawingFrame* frame) {
   // It's unsafe to clear when we have a stencil test because glClear ignores
   // stencil.
-  if (client_->ExternalStencilTestEnabled() &&
+  if (output_surface_->HasExternalStencilTest() &&
       frame->current_render_pass == frame->root_render_pass) {
     DCHECK(!frame->current_render_pass->has_transparent_background);
     return;
@@ -1158,10 +1160,9 @@ bool GLRenderer::SetupQuadForAntialiasing(
       gfx::IsNearestRectWithinDistance(device_layer_quad.BoundingBox(),
                                        kAntiAliasingEpsilon);
 
-  bool use_aa = Settings().allow_antialiasing &&
+  bool use_aa = settings_->allow_antialiasing &&
                 !clipped &&  // code can't handle clipped quads
-                !is_nearest_rect_within_epsilon &&
-                quad->IsEdge();
+                !is_nearest_rect_within_epsilon && quad->IsEdge();
   if (!use_aa)
     return false;
 
@@ -2099,7 +2100,7 @@ void GLRenderer::SwapBuffers() {
   compositor_frame.metadata = client_->MakeCompositorFrameMetadata();
   compositor_frame.gl_frame_data = make_scoped_ptr(new GLFrameData);
   compositor_frame.gl_frame_data->size = output_surface_->SurfaceSize();
-  if (capabilities_.using_partial_swap && client_->AllowPartialSwap()) {
+  if (capabilities_.using_partial_swap) {
     // If supported, we can save significant bandwidth by only swapping the
     // damaged/scissored region (clamped to the viewport)
     swap_buffer_rect_.Intersect(client_->DeviceViewport());
@@ -2512,7 +2513,7 @@ void
GLRenderer::BindFramebufferToOutputSurface(DrawingFrame* frame) {
   current_framebuffer_lock_.reset();
   output_surface_->BindFramebuffer();
 
-  if (client_->ExternalStencilTestEnabled()) {
+  if (output_surface_->HasExternalStencilTest()) {
     SetStencilEnabled(true);
     GLC(context_, context_->stencilFunc(GL_EQUAL, 1, 1));
   } else {
@@ -3160,11 +3161,9 @@ void GLRenderer::LazyLabelOffscreenContext(
     return;
   offscreen_context_labelled_ = true;
   std::string unique_context_name = base::StringPrintf(
-      "%s-Offscreen-%p",
-      Settings().compositor_name.c_str(),
-      context_);
-  offscreen_context_provider->Context3d()->pushGroupMarkerEXT(
-      unique_context_name.c_str());
+      "%s-Offscreen-%p", settings_->compositor_name.c_str(), context_);
+  offscreen_context_provider->Context3d()
+      ->pushGroupMarkerEXT(unique_context_name.c_str());
 }

[commit-bot: I haz the power, 2013-09-09 14:39:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/23961003/38001

[commit-bot: I haz the power, 2013-09-09 17:24:34]

Change committed as 222050