chromeos: Support turning displays off in extended mode.

chromeos/display/output_configurator.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 "chromeos/display/output_configurator.h"
 
 #include <cmath>
 
 #include <X11/Xatom.h>
 #include <X11/Xlib.h>
@@ skipping 1123 matching lines @@
         ConfigureCTM(display, outputs[0].touch_device_id, ctm);
       }
       break;
     }
     case 2: {
       RRCrtc primary_crtc =
           GetNextCrtcAfter(display, screen, outputs[0].output, None);
       RRCrtc secondary_crtc =
           GetNextCrtcAfter(display, screen, outputs[1].output, primary_crtc);
 
-      // Workaround for crbug.com/148365: leave internal display on for
-      // internal-off, external-on so user can move cursor (and hence
-      // windows) onto internal display even when it's off.

[oshima, 2013/03/25 22:20:40]

I'm probably missing something but it sounds to me that this comment
contradicts the code below. That is, primary_power_on is always false when
power_state is DISPLAY_POWER_INTERNAL_OFF_EXTERNAL_ON?

I know you're changing this code, but I'd like to understand how this CL changes
exiting behavior correctly.

[Daniel Erat, 2013/03/26 03:19:15]

Yes, I was confused by this too.  The old code seems wrong but I think it had
the desired effect -- on laptops, the internal panel is always the primary
display, right?  If so, then the primary display would be turned off for the
other two states, INTERNAL_ON_EXTERNAL_OFF and ALL_OFF, as expected.

secondary_power_on seems more problematic, since it looks like an external
secondary display would be turned off for INTERNAL_OFF_EXTERNAL_ON instead of
being kept on.

       bool primary_power_on = power_state == DISPLAY_POWER_ALL_ON ||
+          (power_state == DISPLAY_POWER_INTERNAL_OFF_EXTERNAL_ON &&
+           !outputs[0].is_internal) ||
           (power_state == DISPLAY_POWER_INTERNAL_ON_EXTERNAL_OFF &&
            outputs[0].is_internal);
       bool secondary_power_on = power_state == DISPLAY_POWER_ALL_ON ||
+          (power_state == DISPLAY_POWER_INTERNAL_OFF_EXTERNAL_ON &&
+           !outputs[1].is_internal) ||
           (power_state == DISPLAY_POWER_INTERNAL_ON_EXTERNAL_OFF &&
            outputs[1].is_internal);
 
       if (output_state == STATE_DUAL_MIRROR) {
         XRRModeInfo* mode_info = ModeInfoForID(screen, outputs[0].mirror_mode);
         if (mode_info == NULL) {
           UMA_HISTOGRAM_COUNTS("Display.EnterState.mirror_failures", 1);
           return false;
         }
 
@@ skipping 35 matching lines @@
 
         int primary_height = primary_mode_info->height;
         int secondary_height = secondary_mode_info->height;
         CrtcConfig config1(primary_crtc, 0, 0,
                            primary_power_on ? outputs[0].native_mode : None,
                            outputs[0].output);
         CrtcConfig config2(secondary_crtc, 0, 0,
                            secondary_power_on ? outputs[1].native_mode : None,
                            outputs[1].output);
 
-        if (output_state == STATE_DUAL_EXTENDED)
-          config2.y = primary_height + kVerticalGap;
-        else
-          config1.y = secondary_height + kVerticalGap;
+        if (output_state == STATE_DUAL_EXTENDED) {

[oshima, 2013/03/25 22:20:40]

If we have two displays and not mirrored, it must be extended. Probably legacy
code that we no longer need?

[Daniel Erat, 2013/03/26 03:19:15]

Yeah, I think the only other state is one representing unknown values.  I'm
happy to kill the else clause here if it's unneeded.

+          if (primary_power_on)
+            config2.y = primary_height + kVerticalGap;
+        } else {
+          if (secondary_power_on)
+            config1.y = secondary_height + kVerticalGap;
+        }
 
         int width = std::max<int>(
             primary_mode_info->width, secondary_mode_info->width);
         int height = primary_height + secondary_height + kVerticalGap;
 
+        if (primary_power_on && !secondary_power_on) {
+          width = primary_mode_info->width;
+          height = primary_height;
+        } else if (!primary_power_on && secondary_power_on) {
+          width = secondary_mode_info->width;
+          height = secondary_height;
+        }
+
         CreateFrameBuffer(display, screen, window, width, height, &config1,
                           &config2);
 
         ConfigureCrtc(display, screen, &config1);
         ConfigureCrtc(display, screen, &config2);
 
         if (outputs[0].touch_device_id != None) {
           CoordinateTransformation ctm;
           ctm.x_scale = static_cast<float>(primary_mode_info->width) / width;
           ctm.x_offset = static_cast<float>(config1.x) / width;
@@ skipping 107 matching lines @@
 // static
 RRMode OutputConfigurator::GetOutputNativeMode(
     const XRROutputInfo* output_info) {
   if (output_info->nmode <= 0)
     return None;
 
   return output_info->modes[0];
 }
 
 }  // namespace chromeos