Fix PepperView handling of 1:1 high-DPI.

The special-case handling for displaying the host desktop at 1:1
in high-DPI added in r174845 incorrectly handles mapping of the
clip rect from DIPs to client plugin output coordinates.

This CL re-names the various size and scale members to make a
clearer distinction between DIP, device and (client plugin) view
coordinates, and corrects the clip rect logic.

BUG=163228,168440
TEST=Connect high-DPI client to Chromoting host, switch to Original Size, and configure host to be larger than the client. Scroll around the host desktop and verify that all contents update correctly, e.g. while a video is played in a maximized window.

Review URL: https://chromiumcodereview.appspot.com/11782016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@175988 0039d316-1c4b-4281-b951-d872f2087c98

remoting/client/plugin/chromoting_instance.cc

@@ -381,7 +381,7 @@ bool ChromotingInstance::HandleInputEvent(const pp::InputEvent& event) {
 
   // TODO(wez): When we have a good hook into Host dimensions changes, move
   // this there.
-  mouse_input_filter_.set_output_size(view_->get_screen_size());
+  mouse_input_filter_.set_output_size(view_->get_source_size());
 
   return input_handler_.HandleInputEvent(event);
 }

remoting/client/plugin/pepper_view.cc

@@ -43,12 +43,13 @@ PepperView::PepperView(ChromotingInstance* instance,
     producer_(producer),
     merge_buffer_(NULL),
     merge_clip_area_(SkIRect::MakeEmpty()),
+    dips_size_(SkISize::Make(0, 0)),
+    dips_to_device_scale_(1.0f),
     view_size_(SkISize::Make(0, 0)),
+    dips_to_view_scale_(1.0f),
     clip_area_(SkIRect::MakeEmpty()),
     source_size_(SkISize::Make(0, 0)),
     source_dpi_(SkIPoint::Make(0, 0)),
-    view_size_dips_(SkISize::Make(0, 0)),
-    view_scale_(1.0f),
     flush_pending_(false),
     is_initialized_(false),
     frame_received_(false) {
@@ -74,37 +75,40 @@ void PepperView::SetView(const pp::View& view) {
   bool view_changed = false;
 
   pp::Rect pp_size = view.GetRect();
-  SkISize new_size_dips = SkISize::Make(pp_size.width(), pp_size.height());
+  SkISize new_dips_size = SkISize::Make(pp_size.width(), pp_size.height());
   pp::ViewDev view_dev(view);
-  float new_scale = view_dev.GetDeviceScale();
+  float new_dips_to_device_scale = view_dev.GetDeviceScale();
 
-  if (view_size_dips_ != new_size_dips || view_scale_ != new_scale) {
+  if (dips_size_ != new_dips_size ||
+      dips_to_device_scale_ != new_dips_to_device_scale) {
     view_changed = true;
-    view_scale_ = new_scale;
-    view_size_dips_ = new_size_dips;
+    dips_to_device_scale_ = new_dips_to_device_scale;
+    dips_size_ = new_dips_size;
 
-    // If |view_scale_| is > 1.0 then the device is high-DPI, and there are
-    // actually |view_scale_| physical pixels for every one Density Independent
-    // Pixel (DIP).  If we specify a scale of 1.0 to Graphics2D then we can
-    // render at DIP resolution and let PPAPI up-scale for high-DPI devices.
-    // Note that |view_scale_| is DIPS->pixels, |scale| is pixels->DIPS.
-    float scale = 1.0f;
-    view_size_ = view_size_dips_;
+    // If |dips_to_device_scale_| is > 1.0 then the device is high-DPI, and
+    // there are actually |view_device_scale_| physical pixels for every one
+    // Density Independent Pixel (DIP).  If we specify a scale of 1.0 to
+    // Graphics2D then we can render at DIP resolution and let PPAPI up-scale
+    // for high-DPI devices.
+    dips_to_view_scale_ = 1.0f;
+    view_size_ = dips_size_;
 
     // If the view's DIP dimensions don't match the source then let the frame
     // producer do the scaling, and render at device resolution.
-    if (view_size_dips_ != source_size_) {
-      scale = 1.0f / view_scale_;
-      view_size_ = SkISize::Make(ceilf(view_size_dips_.width() * view_scale_),
-                                 ceilf(view_size_dips_.height() * view_scale_));
+    if (dips_size_ != source_size_) {
+      dips_to_view_scale_ = dips_to_device_scale_;
+      view_size_ = SkISize::Make(
+          ceilf(dips_size_.width() * dips_to_view_scale_),
+          ceilf(dips_size_.height() * dips_to_view_scale_));
     }
 
     // Create a 2D rendering context at the chosen frame dimensions.
     pp::Size pp_size = pp::Size(view_size_.width(), view_size_.height());
     graphics2d_ = pp::Graphics2D(instance_, pp_size, true);
 
+    // Specify the scale from our coordinates to DIPs.
     pp::Graphics2D_Dev graphics2d_dev(graphics2d_);
-    graphics2d_dev.SetScale(scale);
+    graphics2d_dev.SetScale(1.0f / dips_to_view_scale_);
 
     bool result = instance_->BindGraphics(graphics2d_);
 
@@ -113,10 +117,11 @@ void PepperView::SetView(const pp::View& view) {
   }
 
   pp::Rect pp_clip = view.GetClipRect();
-  SkIRect new_clip = SkIRect::MakeLTRB(floorf(pp_clip.x() * view_scale_),
-                                       floorf(pp_clip.y() * view_scale_),
-                                       ceilf(pp_clip.right() * view_scale_),
-                                       ceilf(pp_clip.bottom() * view_scale_));
+  SkIRect new_clip = SkIRect::MakeLTRB(
+      floorf(pp_clip.x() * dips_to_view_scale_),
+      floorf(pp_clip.y() * dips_to_view_scale_),
+      ceilf(pp_clip.right() * dips_to_view_scale_),
+      ceilf(pp_clip.bottom() * dips_to_view_scale_));
   if (clip_area_ != new_clip) {
     view_changed = true;
 
@@ -142,8 +147,8 @@ void PepperView::ApplyBuffer(const SkISize& view_size,
     instance_->OnFirstFrameReceived();
     frame_received_ = true;
   }
-  // Currently we cannot use the data in the buffer is scale factor has changed
-  // already.
+  // We cannot use the data in the buffer if its dimensions don't match the
+  // current view size.
   // TODO(alexeypa): We could rescale and draw it (or even draw it without
   // rescaling) to reduce the perceived lag while we are waiting for
   // the properly scaled data.

remoting/client/plugin/pepper_view.h

@@ -45,42 +45,43 @@ class PepperView : public FrameConsumer,
   virtual void SetSourceSize(const SkISize& source_size,
                              const SkIPoint& dpi) OVERRIDE;
 
-  // Updates the PepperView's size, clipping area and scale factor.
+  // Updates the PepperView's size & clipping area, taking into account the
+  // DIP-to-device scale factor.
   void SetView(const pp::View& view);
 
-  // Return the dimensions of the view and source in device pixels.
-  const SkISize& get_view_size() const {
-    return view_size_;
-  }
-  const SkISize& get_screen_size() const {
+  // Returns the dimensions of the most recently displayed frame, in pixels.
+  const SkISize& get_source_size() const {
     return source_size_;
   }
 
   // Return the dimensions of the view in Density Independent Pixels (DIPs).
-  // On high-DPI devices this will be smaller than the size in device pixels.
+  // Note that there may be multiple device pixels per DIP.
   const SkISize& get_view_size_dips() const {
-    return view_size_dips_;
+    return dips_size_;
   }
 
  private:
-  // This routine allocates an image buffer.
+  // Allocates a new frame buffer to supply to the FrameProducer to render into.
+  // Returns NULL if the maximum number of buffers has already been allocated.
   pp::ImageData* AllocateBuffer();
 
-  // This routine frees an image buffer allocated by AllocateBuffer().
+  // Frees a frame buffer previously allocated by AllocateBuffer.
   void FreeBuffer(pp::ImageData* buffer);
 
-  // This routine makes sure that enough image buffers are in flight to keep
-  // the decoding pipeline busy.
+  // Allocates buffers and passes them to the FrameProducer to render into until
+  // the maximum number of buffers are in-flight.
   void InitiateDrawing();
 
-  // This routine applies the given image buffer to the screen taking into
-  // account |clip_area| of the buffer and |region| describing valid parts
-  // of the buffer.
+  // Renders the parts of |buffer| identified by |region| to the view.  If the
+  // clip area of the view has changed since the buffer was generated then
+  // FrameProducer is supplied the missed parts of |region|.  The FrameProducer
+  // will be supplied a new buffer when FlushBuffer() completes.
   void FlushBuffer(const SkIRect& clip_area,
                    pp::ImageData* buffer,
                    const SkRegion& region);
 
-  // This is a completion callback for FlushGraphics().
+  // Handles completion of FlushBuffer(), triggering a new buffer to be
+  // returned to FrameProducer for rendering.
   void OnFlushDone(base::Time paint_start, pp::ImageData* buffer, int result);
 
   // Reference to the creating plugin instance. Needed for interacting with
@@ -103,20 +104,29 @@ class PepperView : public FrameConsumer,
   SkIRect merge_clip_area_;
   SkRegion merge_region_;
 
-  // View size, clip area and host dimensions, in device pixels.
+  // View size in Density Independent Pixels (DIPs).
+  SkISize dips_size_;
+
+  // Scale factor from DIPs to device pixels.
+  float dips_to_device_scale_;
+
+  // View size in output pixels. This is the size at which FrameProducer must
+  // render frames. It usually matches the DIPs size of the view, but may match
+  // the size in device pixels when scaling is in effect, to reduce artefacts.
   SkISize view_size_;
+
+  // Scale factor from output pixels to device pixels.
+  float dips_to_view_scale_;
+
+  // Visible area of the view, in output pixels.
   SkIRect clip_area_;
+
+  // Size of the most recent source frame in pixels.
   SkISize source_size_;
 
-  // The DPI of the host screen.
+  // Resolution of the most recent source frame dots-per-inch.
   SkIPoint source_dpi_;
 
-  // View size in Density-Independent pixels.
-  SkISize view_size_dips_;
-
-  // DIP-to-device pixel scale factor.
-  float view_scale_;
-
   // True if there is already a Flush() pending on the Graphics2D context.
   bool flush_pending_;