Make exo::Surface set device_scale_factor.

components/exo/surface.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/exo/surface.h"
 
 #include <utility>
 
 #include "base/callback_helpers.h"
 #include "base/logging.h"
@@ skipping 829 matching lines @@
   cc::CompositorFrame frame;
   // If we commit while we don't have an active BeginFrame, we acknowledge a
   // manual one.
   if (current_begin_frame_ack_.sequence_number ==
       cc::BeginFrameArgs::kInvalidFrameNumber) {
     current_begin_frame_ack_ = cc::BeginFrameAck::CreateManualAckWithDamage();
   } else {
     current_begin_frame_ack_.has_damage = true;
   }
   frame.metadata.begin_frame_ack = current_begin_frame_ack_;
+  frame.metadata.device_scale_factor = state_.buffer_scale;

[reveman, 2017/04/25 20:56:17]

I don't think this is correct as setting the viewport is an alternative way to
describe the resolution of the buffer. buffer_scale just describes the size of
the buffer compared to the DSF sent to the client here:
https://cs.chromium.org/chromium/src/components/exo/wayland/server.cc?l=1141

I think DSF used here should always match the value we send to the client. How
is this working today by not setting this value? Can we unit test this?

[kylechar, 2017/04/26 13:56:52]

Okay, that mostly makes sense to me and I misunderstood what what |buffer_scale|
was being used for.

Are you saying that this is the device_scale_factor that is sent to all clients?

https://cs.chromium.org/chromium/src/components/exo/wayland/server.cc?l=1141

What about clients who's windows are not on the primary display? The
device_scale_factor could be different between displays?
CompositorFrameMetadata::device_scale_factor could be used by anything after
this step in cc::Surface/cc::Display pipeline. I think the only thing that is
guaranteed is that when SurfaceManager::SurfaceCreated() runs it sends out that
device_scale_factor as part of the SurfaceInfo for any cc::SurfaceObservers to
see.

I think it's safe to say that exo isn't using the
CompositorFrameMetadata::device_scale_factor and doesn't have any
cc::SurfaceObservers that use SurfaceInfo::device_scale_factor right now. Since
it's unused it doesn't cause problems.

We were using SurfaceInfo::device_scale_factor for something in mus and added a
DCHECK that is's not zero after tracking down a bug related to that (not exo
related). That DCHECK also ended up catching that it's not set here.

[reveman, 2017/04/26 17:48:42]

We always send the primary scale right now. Real multi display support for
wayland clients is still missing.
Thanks for explaining. That makes sense. Probably best to just send the DSF of
the display that the surface is currently associated with for now.

[kylechar, 2017/04/27 18:24:56]

Done.

 
   if (current_resource_.id) {
     // Texture quad is only needed if buffer is not fully transparent.
     if (state_.alpha) {
       cc::TextureDrawQuad* texture_quad =
           render_pass->CreateAndAppendDrawQuad<cc::TextureDrawQuad>();
       float vertex_opacity[4] = {1.0, 1.0, 1.0, 1.0};
       gfx::Rect opaque_rect;
       if (!current_resource_has_alpha_ ||
           state_.blend_mode == SkBlendMode::kSrc ||
@@ skipping 16 matching lines @@
         render_pass->CreateAndAppendDrawQuad<cc::SolidColorDrawQuad>();
     solid_quad->SetNew(quad_state, quad_rect, quad_rect, SK_ColorBLACK, false);
   }
 
   frame.render_pass_list.push_back(std::move(render_pass));
   compositor_frame_sink_holder_->GetCompositorFrameSink()
       ->SubmitCompositorFrame(local_surface_id_, std::move(frame));
 }
 
 }  // namespace exo