chromeos: Include mode details in OutputSnapshot.

chromeos/display/real_output_configurator_delegate.cc

 // Copyright (c) 2013 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/real_output_configurator_delegate.h"
 
 #include <X11/Xatom.h>
 #include <X11/Xlib.h>
 #include <X11/extensions/dpms.h>
 #include <X11/extensions/XInput.h>
 #include <X11/extensions/XInput2.h>
 #include <X11/extensions/Xrandr.h>
 
 #include <cmath>
+#include <utility>
 
 #include "base/logging.h"
 #include "base/message_loop/message_pump_aurax11.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/power_manager_client.h"
 #include "chromeos/display/output_util.h"
 
 namespace chromeos {
 
 namespace {
@@ skipping 83 matching lines @@
 RealOutputConfiguratorDelegate::GetOutputs(
     const OutputConfigurator::StateController* state_controller) {
   CHECK(screen_) << "Server not grabbed";
 
   std::vector<OutputConfigurator::OutputSnapshot> outputs;
   XRROutputInfo* one_info = NULL;
   XRROutputInfo* two_info = NULL;
   RRCrtc last_used_crtc = None;
 
   for (int i = 0; i < screen_->noutput && outputs.size() < 2; ++i) {
-    RROutput this_id = screen_->outputs[i];
-    XRROutputInfo* output_info = XRRGetOutputInfo(display_, screen_, this_id);
+    RROutput output_id = screen_->outputs[i];
+    XRROutputInfo* output_info = XRRGetOutputInfo(display_, screen_, output_id);
     bool is_connected = (output_info->connection == RR_Connected);
 
-    if (is_connected) {
-      OutputConfigurator::OutputSnapshot to_populate;
-      to_populate.output = this_id;
-      to_populate.has_display_id =
-          GetDisplayId(this_id, i, &to_populate.display_id);
-      to_populate.is_internal = IsInternalOutput(output_info);
-      // Use the index as a valid display id even if the internal
-      // display doesn't have valid EDID because the index
-      // will never change.
-      if (!to_populate.has_display_id && to_populate.is_internal)
-        to_populate.has_display_id = true;
+    if (!is_connected) {
+      XRRFreeOutputInfo(output_info);
+      continue;
+    }
 
-      (outputs.empty() ? one_info : two_info) = output_info;
+    OutputConfigurator::OutputSnapshot output;

[oshima, 2013/08/15 19:03:24]

Consider moving the creation to separate method as this method is
relatively big.

[Daniel Erat, 2013/08/15 19:26:23]

I'm worried that that would make the code more complicated (the method would
take a long list of parameters; when a new field gets added both the method
signature and all of the callers need to be updated; the initialization of the
struct would be distributed across that method and this one; etc.).

[oshima, 2013/08/15 19:37:26]

It looked to me that you can create from three parameters,
output_id, i, and output_info. If that's not the case, then please disregard.

[Daniel Erat, 2013/08/15 21:20:33]

It takes few more than that (to initialize all of the fields, apart from the
ones that depend on other outputs), but I've made this change.

+    output.output = output_id;
+    output.width_mm = output_info->mm_width;
+    output.height_mm = output_info->mm_height;
+    output.has_display_id = GetDisplayId(output_id, i, &output.display_id);
+    output.is_internal = IsInternalOutput(output_info);
+    // Use the index as a valid display id even if the internal
+    // display doesn't have valid EDID because the index
+    // will never change.
+    if (!output.has_display_id && output.is_internal)
+      output.has_display_id = true;
 
-      // Now, look up the current CRTC and any related info.
-      if (output_info->crtc) {
-        XRRCrtcInfo* crtc_info = XRRGetCrtcInfo(
-            display_, screen_, output_info->crtc);
-        to_populate.current_mode = crtc_info->mode;
-        to_populate.x = crtc_info->x;
-        to_populate.y = crtc_info->y;
-        XRRFreeCrtcInfo(crtc_info);
+    (outputs.empty() ? one_info : two_info) = output_info;
+
+    // Now, look up the current CRTC and any related info.
+    if (output_info->crtc) {
+      XRRCrtcInfo* crtc_info = XRRGetCrtcInfo(
+          display_, screen_, output_info->crtc);
+      output.current_mode = crtc_info->mode;
+      output.x = crtc_info->x;
+      output.y = crtc_info->y;
+      XRRFreeCrtcInfo(crtc_info);
+    }
+
+    // Assign a CRTC that isn't already in use.
+    for (int j = 0; j < output_info->ncrtc; ++j) {
+      if (output_info->crtcs[j] != last_used_crtc) {
+        output.crtc = output_info->crtcs[j];
+        last_used_crtc = output.crtc;
+        break;
       }
+    }
 
-      // Assign a CRTC that isn't already in use.
-      for (int j = 0; j < output_info->ncrtc; ++j) {
-        if (output_info->crtcs[j] != last_used_crtc) {
-          to_populate.crtc = output_info->crtcs[j];
-          last_used_crtc = to_populate.crtc;
-          break;
-        }
+    output.native_mode = GetOutputNativeMode(output_info);
+
+    if (output.has_display_id) {
+      int width = 0, height = 0;
+      if (state_controller &&
+          state_controller->GetResolutionForDisplayId(
+              output.display_id, &width, &height)) {
+        output.selected_mode =
+            FindOutputModeMatchingSize(screen_, output_info, width, height);
       }
-      to_populate.native_mode = GetOutputNativeMode(output_info);
-      if (to_populate.has_display_id) {
-        int width = 0, height = 0;
-        if (state_controller &&
-            state_controller->GetResolutionForDisplayId(
-                to_populate.display_id, &width, &height)) {
-          to_populate.selected_mode =
-              FindOutputModeMatchingSize(screen_, output_info, width, height);
-        }
-      }
-      // Fallback to native mode.
-      if (to_populate.selected_mode == None)
-        to_populate.selected_mode = to_populate.native_mode;
+    }
+    // Fall back to native mode.
+    if (output.selected_mode == None)
+      output.selected_mode = output.native_mode;
 
-      to_populate.is_aspect_preserving_scaling =
-          IsOutputAspectPreservingScaling(this_id);
-      to_populate.touch_device_id = None;
+    output.is_aspect_preserving_scaling =
+        IsOutputAspectPreservingScaling(output_id);
+    output.touch_device_id = None;
 
-      VLOG(2) << "Found display " << outputs.size() << ":"
-              << " output=" << to_populate.output
-              << " crtc=" << to_populate.crtc
-              << " current_mode=" << to_populate.current_mode;
-      outputs.push_back(to_populate);
-    } else {
-      XRRFreeOutputInfo(output_info);
+    for (int mode_index = 0; mode_index < output_info->nmode; ++mode_index) {
+      const RRMode mode = output_info->modes[mode_index];
+      OutputConfigurator::ModeInfo info;
+      if (GetModeDetails(mode, &info.width, &info.height, &info.interlaced))
+        output.mode_infos.insert(std::make_pair(mode, info));
+      else
+        LOG(WARNING) << "Unable to find XRRModeInfo for mode " << mode;
     }
+
+    VLOG(2) << "Found display " << outputs.size() << ":"
+            << " output=" << output.output
+            << " crtc=" << output.crtc
+            << " current_mode=" << output.current_mode;
+    outputs.push_back(output);
   }
 
   if (outputs.size() == 2) {
     bool one_is_internal = IsInternalOutput(one_info);
     bool two_is_internal = IsInternalOutput(two_info);
     int internal_outputs = (one_is_internal ? 1 : 0) +
         (two_is_internal ? 1 : 0);
     DCHECK_LT(internal_outputs, 2);
     LOG_IF(WARNING, internal_outputs == 2)
         << "Two internal outputs detected.";
@@ skipping 31 matching lines @@
       }
     }
   }
 
   GetTouchscreens(&outputs);
   XRRFreeOutputInfo(one_info);
   XRRFreeOutputInfo(two_info);
   return outputs;
 }
 
-bool RealOutputConfiguratorDelegate::GetModeDetails(RRMode mode,
-                                                    int* width,
-                                                    int* height,
-                                                    bool* interlaced) {
-  CHECK(screen_) << "Server not grabbed";
-  // TODO: Determine if we need to organize modes in a way which provides
-  // better than O(n) lookup time.  In many call sites, for example, the
-  // "next" mode is typically what we are looking for so using this
-  // helper might be too expensive.
-  for (int i = 0; i < screen_->nmode; ++i) {
-    if (mode == screen_->modes[i].id) {
-      const XRRModeInfo& info = screen_->modes[i];
-      if (width)
-        *width = info.width;
-      if (height)
-        *height = info.height;
-      if (interlaced)
-        *interlaced = info.modeFlags & RR_Interlace;
-      return true;
-    }
-  }
-  return false;
-}
-
 bool RealOutputConfiguratorDelegate::ConfigureCrtc(
     RRCrtc crtc,
     RRMode mode,
     RROutput output,
     int x,
     int y) {
   CHECK(screen_) << "Server not grabbed";
   VLOG(1) << "ConfigureCrtc: crtc=" << crtc
           << " mode=" << mode
           << " output=" << output
@@ skipping 71 matching lines @@
   }
   XIFreeDeviceInfo(info);
 }
 
 void RealOutputConfiguratorDelegate::SendProjectingStateToPowerManager(
     bool projecting) {
   chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->
       SetIsProjecting(projecting);
 }
 
+bool RealOutputConfiguratorDelegate::GetModeDetails(RRMode mode,
+                                                    int* width,
+                                                    int* height,
+                                                    bool* interlaced) {
+  CHECK(screen_) << "Server not grabbed";
+  // TODO: Determine if we need to organize modes in a way which provides
+  // better than O(n) lookup time.  In many call sites, for example, the
+  // "next" mode is typically what we are looking for so using this
+  // helper might be too expensive.
+  for (int i = 0; i < screen_->nmode; ++i) {
+    if (mode == screen_->modes[i].id) {
+      const XRRModeInfo& info = screen_->modes[i];
+      if (width)
+        *width = info.width;
+      if (height)
+        *height = info.height;
+      if (interlaced)
+        *interlaced = info.modeFlags & RR_Interlace;
+      return true;
+    }
+  }
+  return false;
+}
+
 void RealOutputConfiguratorDelegate::DestroyUnusedCrtcs(
     const std::vector<OutputConfigurator::OutputSnapshot>& outputs) {
   CHECK(screen_) << "Server not grabbed";
   // Setting the screen size will fail if any CRTC doesn't fit afterwards.
   // At the same time, turning CRTCs off and back on uses up a lot of time.
   // This function tries to be smart to avoid too many off/on cycles:
   // - We disable all the CRTCs we won't need after the FB resize.
   // - We set the new modes on CRTCs, if they fit in both the old and new
   //   FBs, and park them at (0,0)
   // - We disable the CRTCs we will need but don't fit in the old FB. Those
   //   will be reenabled after the resize.
   // We don't worry about the cached state of the outputs here since we are
   // not interested in the state we are setting - we just try to get the CRTCs
   // out of the way so we can rebuild the frame buffer.
   for (int i = 0; i < screen_->ncrtc; ++i) {
     // Default config is to disable the crtcs.
     RRCrtc crtc = screen_->crtcs[i];
     RRMode mode = None;
     RROutput output = None;
+    const OutputConfigurator::ModeInfo* mode_info = NULL;
     for (std::vector<OutputConfigurator::OutputSnapshot>::const_iterator it =
          outputs.begin(); it != outputs.end(); ++it) {
       if (crtc == it->crtc) {
         mode = it->current_mode;
         output = it->output;
+        mode_info = OutputConfigurator::GetModeInfo(*it, mode);
         break;
       }
     }
 
-    if (mode != None) {
+    if (mode_info) {
       // In case our CRTC doesn't fit in our current framebuffer, disable it.
       // It'll get reenabled after we resize the framebuffer.
-      int mode_width = 0, mode_height = 0;
-      CHECK(GetModeDetails(mode, &mode_width, &mode_height, NULL));
       int current_width = DisplayWidth(display_, DefaultScreen(display_));
       int current_height = DisplayHeight(display_, DefaultScreen(display_));
-      if (mode_width > current_width || mode_height > current_height) {
+      if (mode_info->width > current_width ||
+          mode_info->height > current_height) {
         mode = None;
         output = None;
+        mode_info = NULL;
       }
     }
 
     ConfigureCrtc(crtc, mode, output, 0, 0);
   }
 }
 
 bool RealOutputConfiguratorDelegate::IsOutputAspectPreservingScaling(
     RROutput id) {
   bool ret = false;
@@ skipping 153 matching lines @@
         is_direct_touch = touch_info->mode == XIDirectTouch;
       }
 #endif
     }
 
     // Touchscreens should have absolute X and Y axes,
     // and be direct touch devices.
     if (width > 0.0 && height > 0.0 && is_direct_touch) {
       size_t k = 0;
       for (; k < outputs->size(); k++) {
-        if ((*outputs)[k].native_mode == None ||
-            (*outputs)[k].touch_device_id != None)
+        OutputConfigurator::OutputSnapshot* output = &(*outputs)[k];
+        if (output->native_mode == None || output->touch_device_id != None)
           continue;
-        int native_mode_width = 0, native_mode_height = 0;
-        if (!GetModeDetails((*outputs)[k].native_mode, &native_mode_width,
-                            &native_mode_height, NULL))
+
+        const OutputConfigurator::ModeInfo* mode_info =
+            OutputConfigurator::GetModeInfo(*output, output->native_mode);
+        if (!mode_info)
           continue;
 
         // Allow 1 pixel difference between screen and touchscreen
         // resolutions.  Because in some cases for monitor resolution
         // 1024x768 touchscreen's resolution would be 1024x768, but for
         // some 1023x767.  It really depends on touchscreen's firmware
         // configuration.
-        if (std::abs(native_mode_width - width) <= 1.0 &&
-            std::abs(native_mode_height - height) <= 1.0) {
-          (*outputs)[k].touch_device_id = info[i].deviceid;
+        if (std::abs(mode_info->width - width) <= 1.0 &&
+            std::abs(mode_info->height - height) <= 1.0) {
+          output->touch_device_id = info[i].deviceid;
 
           VLOG(2) << "Found touchscreen for output #" << k
-                  << " id " << (*outputs)[k].touch_device_id
+                  << " id " << output->touch_device_id
                   << " width " << width
                   << " height " << height;
           break;
         }
       }
 
       VLOG_IF(2, k == outputs->size())
           << "No matching output - ignoring touchscreen"
           << " id " << info[i].deviceid
           << " width " << width
           << " height " << height;
     }
   }
 
   XIFreeDeviceInfo(info);
 }
 
 }  // namespace chromeos