chromeos: Support turning displays off in extended mode.

chromeos/display/output_configurator.cc

Index: chromeos/display/output_configurator.cc
diff --git a/chromeos/display/output_configurator.cc b/chromeos/display/output_configurator.cc
index b41362ff9657278b8fdc1ffc2178ada9a6b79700..6e9332707901cfa966fe368cd1243c4927c890fa 100644
--- a/chromeos/display/output_configurator.cc
+++ b/chromeos/display/output_configurator.cc
@@ -1141,13 +1141,14 @@ bool OutputConfigurator::EnterState(
       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);
 
@@ -1203,15 +1204,26 @@ bool OutputConfigurator::EnterState(
                            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);