When resizing-to-fit, prefer to match desktop rotation if possible.

remoting/host/desktop_resizer_win.cc

 // Copyright (c) 2012 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 "remoting/host/desktop_resizer.h"
 
 #include <map>
 
 #include "base/logging.h"
 
@@ skipping 15 matching lines @@
   virtual SkISize GetCurrentSize() OVERRIDE;
   virtual std::list<SkISize> GetSupportedSizes(
       const SkISize& preferred) OVERRIDE;
   virtual void SetSize(const SkISize& size) OVERRIDE;
   virtual void RestoreSize(const SkISize& original) OVERRIDE;
 
  private:
   static bool IsResizeSupported();
 
   // Calls EnumDisplaySettingsEx() for the primary monitor.
-  // Returns a DEVMODE with no fields if |mode_number| does not exist.
+  // Returns false if |mode_number| does not exist.
   static bool GetPrimaryDisplayMode(
       DWORD mode_number, DWORD flags, DEVMODE* mode);
 
   // Returns true if the mode has width, height, bits-per-pixel, frequency
   // and orientation fields.
   static bool IsModeValid(const DEVMODE& mode);
 
   // Returns the width & height of |mode|, or 0x0 if they are missing.
   static SkISize GetModeSize(const DEVMODE& mode);
 
@@ skipping 37 matching lines @@
       break;
 
     // Ignore modes missing the fields that we expect.
     if (!IsModeValid(candidate_mode))
       continue;
 
     // Ignore modes with differing bits-per-pixel.
     if (candidate_mode.dmBitsPerPel != current_mode.dmBitsPerPel)
       continue;
 
-    // If there is an existing mode of the same dimensions, prefer the
-    // one with the higher frequency.
+    // If there is an existing mode of the same dimensions, choose the
+    // closest match to the current mode.
     SkISize candidate_size = GetModeSize(candidate_mode);
     if (modes_.count(candidate_size) != 0) {
       DEVMODE existing_mode = modes_[candidate_size];

[Jamie, 2013/02/13 18:21:01]

Would best_mode_for_size be a better name for this variable? I spent a few
minutes trying to work out what it was for.

[Wez, 2013/02/13 22:44:55]

You mean modes_ -> best_mode_for_size_?

[Jamie, 2013/02/14 00:10:34]

I meant best_mode_for_size instead of existing_mode, but actually I like your
suggestion better.

-      if (existing_mode.dmDisplayFrequency >
-          candidate_mode.dmDisplayFrequency)
+
+      // Skip this mode if its rotation is a worse match for the current mode.
+      if ((candidate_mode.dmDisplayOrientation !=
+           current_mode.dmDisplayOrientation) &&
+          (existing_mode.dmDisplayOrientation ==
+           current_mode.dmDisplayOrientation)) {

[Jamie, 2013/02/13 18:21:01]

This will never pick a portrait mode if the physical display is landscape,
correct? I'm not sure that's the right choice for Me2Me, especially if the
display is curtained. I think it deserves a comment at least, explaining that
this is a design choice.

[Wez, 2013/02/13 22:44:55]

No.  This logic applies iff the mode we're enumerating, and the previous
best-match mode have the same dimensions.  If the new mode's orientation doesn't
match the current mode, but the best-match mode's does, then we prefer the
best-match mode still - i.e. we prefer matching the orientation over matching
the refresh rate.

I'll rephrase this as a single comment explaining that prioritisation across
these blocks.

[Jamie, 2013/02/14 00:10:34]

Oh, I see. I had it in mind that the API returned the unrotated width and
height, but that wouldn't be very useful... In that case, this is fine.

         continue;
+      }
+
+      // Skip this mode if its frequency does not match, and the existing
+      // choice has a higher frequency.
+      if ((candidate_mode.dmDisplayFrequency !=
+           current_mode.dmDisplayFrequency) &&
+          (existing_mode.dmDisplayFrequency >=
+           candidate_mode.dmDisplayFrequency)) {
+        continue;
+      }
     } else {
       // If we haven't seen this size before, add it to those we return.
       sizes.push_back(candidate_size);
     }
 
     modes_[candidate_size] = candidate_mode;
   }
 
   return sizes;
 }
@@ skipping 43 matching lines @@
 SkISize DesktopResizerWin::GetModeSize(const DEVMODE& mode) {
   DCHECK(IsModeValid(mode));
   return SkISize::Make(mode.dmPelsWidth, mode.dmPelsHeight);
 }
 
 scoped_ptr<DesktopResizer> DesktopResizer::Create() {
   return scoped_ptr<DesktopResizer>(new DesktopResizerWin);
 }
 
 }  // namespace remoting