monitor_reconfig: Handle external monitors and add tests.
This splits the resolution-choosing logic out into its own
class to make it easier to test. It also adds some
probably-unreliable heuristics to try to guess when the user
would rather that we just use the external output's maximum
resolution instead of trying to find a resolution that'll
also work on the built-in output.
BUG=chromium-os:2933
TEST=added a bunch
Review URL: http://codereview.chromium.org/3304011
diff --git a/Makefile b/Makefile
index d62f0f6..3ef81f1 100644
--- a/Makefile
+++ b/Makefile
@@ -7,21 +7,28 @@
PKG_CONFIG ?= pkg-config
LIBS = -lbase -lpthread -lrt
+TEST_LIBS = -lgtest -lgflags
INCLUDE_DIRS = -I.. $(shell $(PKG_CONFIG) --cflags x11 xrandr)
LIB_DIRS = $(shell $(PKG_CONFIG) --libs x11 xrandr)
BIN=monitor_reconfigure
-OBJECTS=monitor_reconfigure_main.o
+OBJECTS=monitor_reconfigure_main.o resolution_selector.o
.cc.o:
$(CXX) $(CXXFLAGS) $(INCLUDE_DIRS) -c $< -o $@
$(BIN): $(OBJECTS)
$(CXX) $(CXXFLAGS) $(INCLUDE_DIRS) $(LIB_DIRS) $^ $(LIBS) $(LDFLAGS) \
- -o $@
+ -o $@
all: $(BIN)
+test: resolution_selector_test
+
+resolution_selector_test: resolution_selector_test.cc resolution_selector.o
+ $(CXX) $(CXXFLAGS) $(INCLUDE_DIRS) $(LIB_DIRS) $^ \
+ $(LIBS) $(TEST_LIBS) $(LDFLAGS) -o $@
+
clean:
- @rm -f $(BIN) $(OBJECTS)
+ @rm -f $(BIN) $(OBJECTS) resolution_selector_test
diff --git a/monitor_reconfigure_main.cc b/monitor_reconfigure_main.cc
index 4a8b011..7d96cfb 100644
--- a/monitor_reconfigure_main.cc
+++ b/monitor_reconfigure_main.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -18,6 +18,8 @@
namespace monitor_reconfig {
+static const char kLcdOutputName[] = "LVDS1";
+
MonitorReconfigureMain::MonitorReconfigureMain(Display* display,
XRRScreenResources* screen_info)
: display_(display),
@@ -31,13 +33,38 @@
DetermineOutputs();
}
+void MonitorReconfigureMain::Run() {
+ vector<ResolutionSelector::Mode> lcd_modes;
+ SortModesByResolution(*lcd_output_, &lcd_modes);
+ DCHECK(!lcd_modes.empty());
+
+ vector<ResolutionSelector::Mode> external_modes;
+ if (IsExternalMonitorConnected()) {
+ SortModesByResolution(*external_output_, &external_modes);
+ DCHECK(!external_modes.empty());
+ }
+
+ ResolutionSelector selector;
+ string lcd_resolution, external_resolution, screen_resolution;
+ CHECK(selector.FindBestResolutions(lcd_modes,
+ external_modes,
+ &lcd_resolution,
+ &external_resolution,
+ &screen_resolution));
+
+ SetDeviceResolution(lcd_output_->name, lcd_resolution);
+ if (IsExternalMonitorConnected())
+ SetDeviceResolution(external_output_->name, external_resolution);
+ SetScreenResolution(screen_resolution);
+}
+
void MonitorReconfigureMain::DetermineOutputs() {
+ CHECK(screen_info_->noutput > 1) << "Expected at least two outputs";
XRROutputInfo* first_output =
XRRGetOutputInfo(display_, screen_info_, screen_info_->outputs[0]);
XRROutputInfo* second_output =
XRRGetOutputInfo(display_, screen_info_, screen_info_->outputs[1]);
- static const char* kLcdOutputName = "LVDS1";
if (strcmp(first_output->name, kLcdOutputName) == 0) {
lcd_output_ = first_output;
external_output_ = second_output;
@@ -64,67 +91,19 @@
}
void MonitorReconfigureMain::SortModesByResolution(
- const XRROutputInfo& output_info, vector<XRRModeInfo*>* modes_out) {
+ const XRROutputInfo& output_info,
+ vector<ResolutionSelector::Mode>* modes_out) {
modes_out->clear();
- for (int i = 0; i < output_info.nmode; ++i)
- modes_out->push_back(mode_map_[output_info.modes[i]]);
- sort(modes_out->begin(), modes_out->end(), ModeResolutionComparator());
-}
-bool MonitorReconfigureMain::FindBestResolutions(
- string* lcd_resolution,
- string* external_resolution,
- string* screen_resolution) {
- DCHECK(lcd_resolution);
- DCHECK(external_resolution);
- DCHECK(screen_resolution);
-
- vector<XRRModeInfo*> lcd_modes, external_modes;
- SortModesByResolution(*lcd_output_, &lcd_modes);
- SortModesByResolution(*external_output_, &external_modes);
- DCHECK(!lcd_modes.empty());
- DCHECK(!external_modes.empty());
-
- if ((lcd_modes[0]->width * lcd_modes[0]->height) >=
- (external_modes[0]->width * external_modes[0]->height)) {
- return FindNearestResolutions(
- lcd_modes, external_modes,
- lcd_resolution, external_resolution, screen_resolution);
- } else {
- return FindNearestResolutions(
- external_modes, lcd_modes,
- external_resolution, lcd_resolution, screen_resolution);
- }
-}
-
-bool MonitorReconfigureMain::FindNearestResolutions(
- const vector<XRRModeInfo*>& larger_device_modes,
- const vector<XRRModeInfo*>& smaller_device_modes,
- string* larger_resolution,
- string* smaller_resolution,
- string* screen_resolution) {
- DCHECK(larger_resolution);
- DCHECK(smaller_resolution);
-
- // Start with the best that the smaller device has to offer.
- smaller_resolution->assign(smaller_device_modes[0]->name);
- *screen_resolution = *smaller_resolution;
- int smaller_width = smaller_device_modes[0]->width;
- int smaller_height = smaller_device_modes[0]->height;
-
- for (vector<XRRModeInfo*>::const_reverse_iterator it =
- larger_device_modes.rbegin();
- it != larger_device_modes.rend(); ++it) {
- if ((*it)->width >= smaller_width && (*it)->height >= smaller_height) {
- larger_resolution->assign((*it)->name);
- return true;
- }
+ for (int i = 0; i < output_info.nmode; ++i) {
+ const XRRModeInfo* mode = mode_map_[output_info.modes[i]];
+ DCHECK(mode);
+ modes_out->push_back(
+ ResolutionSelector::Mode(mode->width, mode->height, mode->name));
}
- LOG(WARNING) << "Failed to find a resolution from larger device "
- << "exceeding chosen resolution from smaller device ("
- << *smaller_resolution << ")";
- return false;
+ sort(modes_out->begin(), modes_out->end(),
+ ResolutionSelector::ModeResolutionComparator());
}
bool MonitorReconfigureMain::SetDeviceResolution(
@@ -142,28 +121,6 @@
return system(command.c_str()) == 0;
}
-void MonitorReconfigureMain::Run() {
- // If there's no external monitor connected, just use the highest resolution
- // supported by the LCD.
- if (!IsExternalMonitorConnected()) {
- LOG(INFO) << "No external monitor connected; using max LCD resolution";
- vector<XRRModeInfo*> lcd_modes;
- SortModesByResolution(*lcd_output_, &lcd_modes);
- CHECK(!lcd_modes.empty());
- SetDeviceResolution(lcd_output_->name, lcd_modes[0]->name);
- SetScreenResolution(lcd_modes[0]->name);
- return;
- }
-
- string lcd_resolution, external_resolution, screen_resolution;
- CHECK(FindBestResolutions(&lcd_resolution,
- &external_resolution,
- &screen_resolution));
- SetDeviceResolution(lcd_output_->name, lcd_resolution);
- SetDeviceResolution(external_output_->name, external_resolution);
- SetScreenResolution(screen_resolution);
-}
-
} // end namespace monitor_reconfig
int main(int argc, char** argv) {
@@ -177,7 +134,6 @@
Window window = RootWindow(display, DefaultScreen(display));
XRRScreenResources* screen_info = XRRGetScreenResources(display, window);
monitor_reconfig::MonitorReconfigureMain main_app(display, screen_info);
-
main_app.Run();
return 0;
}
diff --git a/monitor_reconfigure_main.h b/monitor_reconfigure_main.h
index 3aa17d0..aeac10f 100644
--- a/monitor_reconfigure_main.h
+++ b/monitor_reconfigure_main.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -12,6 +12,8 @@
#include <X11/Xlib.h>
#include <X11/extensions/Xrandr.h>
+#include "monitor_reconfig/resolution_selector.h"
+
namespace monitor_reconfig {
// MonitorReconfigureMain is the class responsible for setting the external
@@ -32,38 +34,10 @@
// Returns whether an external monitor is connected
bool IsExternalMonitorConnected();
- // Comparator used by SortModeByResolution().
- // Returns true if |mode_a| has more pixels than |mode_b| and false otherwise.
- class ModeResolutionComparator {
- public:
- bool operator()(XRRModeInfo* mode_a, XRRModeInfo* mode_b) const {
- return mode_a->width * mode_a->height > mode_b->width * mode_b->height;
- }
- };
-
// Sorts |output_info|'s modes by decreasing number of pixels, storing the
// results in |modes_out|.
void SortModesByResolution(const XRROutputInfo& output_info,
- std::vector<XRRModeInfo*>* modes_out);
-
- // Find resolutions to use.
- bool FindBestResolutions(
- std::string* lcd_resolution,
- std::string* external_resolution,
- std::string* screen_resolution);
-
- // Find resolutions to use that are reasonably close together.
- // |larger_device_modes| and |smaller_device_modes| should be sorted by
- // descending resolution. We choose the highest resolution from
- // |smaller_device_modes| and the lowest resolution from |larger_device_modes|
- // that's at least as high as the resolution from the smaller device.
- // |screen_resolution| gets set to |smaller_resolution| to avoid clipping.
- bool FindNearestResolutions(
- const std::vector<XRRModeInfo*>& larger_device_modes,
- const std::vector<XRRModeInfo*>& smaller_device_modes,
- std::string* larger_resolution,
- std::string* smaller_resolution,
- std::string* screen_resolution);
+ std::vector<ResolutionSelector::Mode>* modes_out);
// Set the resolution for a particular display or for the screen.
bool SetDeviceResolution(const std::string& device_name,
diff --git a/resolution_selector.cc b/resolution_selector.cc
new file mode 100644
index 0000000..ac4e31b
--- /dev/null
+++ b/resolution_selector.cc
@@ -0,0 +1,91 @@
+// Copyright (c) 2010 The Chromium OS 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 "monitor_reconfig/resolution_selector.h"
+
+#include "base/logging.h"
+
+namespace monitor_reconfig {
+
+using std::string;
+using std::vector;
+
+
+const int ResolutionSelector::kMaxProjectorPixels = 1280 * 720;
+
+
+bool ResolutionSelector::FindBestResolutions(
+ const vector<Mode>& lcd_modes,
+ const vector<Mode>& external_modes,
+ string* lcd_resolution,
+ string* external_resolution,
+ string* screen_resolution) {
+ DCHECK(!lcd_modes.empty());
+
+ // If there's no external display, just use the highest resolution
+ // available from the LCD.
+ if (external_modes.empty()) {
+ *lcd_resolution = *screen_resolution = lcd_modes[0].name;
+ external_resolution->clear();
+ return true;
+ }
+
+ const int max_lcd_size = lcd_modes[0].width * lcd_modes[0].height;
+ const int max_external_size =
+ external_modes[0].width * external_modes[0].height;
+
+ if (max_lcd_size >= max_external_size) {
+ return FindNearestResolutions(
+ lcd_modes, external_modes,
+ lcd_resolution, external_resolution, screen_resolution);
+ } else {
+ // If the external output is large enough that we think it's a monitor
+ // (as opposed to a projector), then we just use its max resolution and
+ // forget about trying to choose a screen size that'll fit on the
+ // built-in display.
+ if (max_external_size > kMaxProjectorPixels) {
+ *external_resolution = *screen_resolution = external_modes[0].name;
+ *lcd_resolution = lcd_modes[0].name;
+ return true;
+ }
+ return FindNearestResolutions(
+ external_modes, lcd_modes,
+ external_resolution, lcd_resolution, screen_resolution);
+ }
+}
+
+bool ResolutionSelector::FindNearestResolutions(
+ const vector<Mode>& larger_device_modes,
+ const vector<Mode>& smaller_device_modes,
+ std::string* larger_resolution,
+ std::string* smaller_resolution,
+ std::string* screen_resolution) {
+ DCHECK(!larger_device_modes.empty());
+ DCHECK(!smaller_device_modes.empty());
+ DCHECK(larger_resolution);
+ DCHECK(smaller_resolution);
+ DCHECK(screen_resolution);
+
+ // Start with the best that the smaller device has to offer.
+ *smaller_resolution = smaller_device_modes[0].name;
+ *screen_resolution = *smaller_resolution;
+ int smaller_width = smaller_device_modes[0].width;
+ int smaller_height = smaller_device_modes[0].height;
+
+ for (vector<Mode>::const_reverse_iterator it =
+ larger_device_modes.rbegin();
+ it != larger_device_modes.rend(); ++it) {
+ if (it->width >= smaller_width && it->height >= smaller_height) {
+ *larger_resolution = it->name;
+ return true;
+ }
+ }
+
+ LOG(WARNING) << "Failed to find a resolution from larger device "
+ << "exceeding chosen resolution from smaller device ("
+ << *smaller_resolution << ")";
+ return false;
+}
+
+} // namespace monitor_reconfig
diff --git a/resolution_selector.h b/resolution_selector.h
new file mode 100644
index 0000000..e292090
--- /dev/null
+++ b/resolution_selector.h
@@ -0,0 +1,85 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef MONITOR_RECONFIGURE_RESOLUTION_SELECTOR_H_
+#define MONITOR_RECONFIGURE_RESOLUTION_SELECTOR_H_
+
+#include <string>
+#include <vector>
+
+#include "base/basictypes.h"
+
+namespace monitor_reconfig {
+
+// ResolutionSelector takes the sets of resolutions supported by the
+// built-in and external displays as input and attempts to choose a shared
+// resolution that will work well on both devices.
+class ResolutionSelector {
+ public:
+ // A single mode supported by a device, equivalent to the XRRModeInfo
+ // struct.
+ struct Mode {
+ Mode(int width, int height, std::string name)
+ : width(width),
+ height(height),
+ name(name) {
+ }
+
+ // Mode's dimensions.
+ int width;
+ int height;
+
+ // Mode's name from XRandR. This uniquely describes the mode and can
+ // be used to set the devices's resolution later.
+ std::string name;
+ };
+
+ // Maximum screen size for the external output at which we assume that
+ // it's a projector (as opposed to a monitor) and try to find a size that
+ // will also fit on the LCD display. Above this, we just use the
+ // external output's maximum resolution, even if it doesn't fit on the
+ // LCD.
+ static const int kMaxProjectorPixels;
+
+ ResolutionSelector() {}
+ ~ResolutionSelector() {}
+
+ // Comparator used to sort Mode objects.
+ // Returns true if |mode_a| has more pixels than |mode_b| and false otherwise.
+ class ModeResolutionComparator {
+ public:
+ bool operator()(const Mode& mode_a, const Mode& mode_b) const {
+ return mode_a.width * mode_a.height > mode_b.width * mode_b.height;
+ }
+ };
+
+ // Find the "best" resolutions for various outputs.
+ // The returned strings contain |name| values from the passed-in modes.
+ bool FindBestResolutions(
+ const std::vector<Mode>& lcd_modes,
+ const std::vector<Mode>& external_modes,
+ std::string* lcd_resolution,
+ std::string* external_resolution,
+ std::string* screen_resolution);
+
+ private:
+ // Find resolutions to use that are reasonably close together.
+ // |larger_device_modes| and |smaller_device_modes| should be sorted by
+ // descending resolution. We choose the highest resolution from
+ // |smaller_device_modes| and the lowest resolution from |larger_device_modes|
+ // that's at least as high as the resolution from the smaller device.
+ // |screen_resolution| gets set to |smaller_resolution| to avoid clipping.
+ bool FindNearestResolutions(
+ const std::vector<Mode>& larger_device_modes,
+ const std::vector<Mode>& smaller_device_modes,
+ std::string* larger_resolution,
+ std::string* smaller_resolution,
+ std::string* screen_resolution);
+
+ DISALLOW_COPY_AND_ASSIGN(ResolutionSelector);
+};
+
+} // namespace monitor_reconfig
+
+#endif // MONITOR_RECONFIGURE_RESOLUTION_SELECTOR_H_
diff --git a/resolution_selector_test.cc b/resolution_selector_test.cc
new file mode 100644
index 0000000..7cbfb12
--- /dev/null
+++ b/resolution_selector_test.cc
@@ -0,0 +1,165 @@
+// Copyright (c) 2010 The Chromium OS 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 <vector>
+
+#include <gflags/gflags.h>
+#include <gtest/gtest.h>
+
+#include "base/command_line.h"
+#include "base/logging.h"
+#include "base/string_util.h"
+#include "monitor_reconfig/resolution_selector.h"
+
+DEFINE_bool(logtostderr, false,
+ "Print debugging messages to stderr (suppressed otherwise)");
+
+namespace monitor_reconfig {
+
+using std::string;
+using std::vector;
+
+class ResolutionSelectorTest : public ::testing::Test {
+ protected:
+ virtual void SetUp() {}
+
+ // Add a mode to |lcd_modes_|.
+ void AddLcdMode(int width, int height) {
+ lcd_modes_.push_back(
+ ResolutionSelector::Mode(
+ width, height, StringPrintf("%dx%d", width, height)));
+ }
+
+ // Add a mode to |external_modes_|.
+ void AddExternalMode(int width, int height) {
+ external_modes_.push_back(
+ ResolutionSelector::Mode(
+ width, height, StringPrintf("%dx%d", width, height)));
+ }
+
+ // Ask |selector_| for the best resolutions and store them in
+ // |lcd_resolution_|, |external_resolution_|, and |screen_resolution_|.
+ // The return value from |FindBestResolutions()| is returned.
+ bool GetResolutions() {
+ return selector_.FindBestResolutions(lcd_modes_,
+ external_modes_,
+ &lcd_resolution_,
+ &external_resolution_,
+ &screen_resolution_);
+ }
+
+ ResolutionSelector selector_;
+
+ vector<ResolutionSelector::Mode> lcd_modes_;
+ vector<ResolutionSelector::Mode> external_modes_;
+
+ string lcd_resolution_;
+ string external_resolution_;
+ string screen_resolution_;
+};
+
+// We should use the LCD's max resolution when there's no external output
+// connected.
+TEST_F(ResolutionSelectorTest, NoExternalOutput) {
+ AddLcdMode(1024, 768);
+ AddLcdMode(800, 600);
+ ASSERT_TRUE(GetResolutions());
+ EXPECT_EQ("1024x768", lcd_resolution_);
+ EXPECT_EQ("", external_resolution_);
+ EXPECT_EQ("1024x768", screen_resolution_);
+}
+
+// When both outputs have the same max resolution, we should use it.
+TEST_F(ResolutionSelectorTest, MatchingTopResolutions) {
+ AddLcdMode(1024, 768);
+ AddLcdMode(800, 600);
+ AddExternalMode(1024, 768);
+ AddExternalMode(800, 600);
+ ASSERT_TRUE(GetResolutions());
+ EXPECT_EQ("1024x768", lcd_resolution_);
+ EXPECT_EQ("1024x768", external_resolution_);
+ EXPECT_EQ("1024x768", screen_resolution_);
+}
+
+// We should use the highest shared resolution when the LCD has the higher
+// max resolution.
+TEST_F(ResolutionSelectorTest, LcdHasHigherResolution) {
+ AddLcdMode(1024, 768);
+ AddLcdMode(800, 600);
+ AddLcdMode(640, 480);
+ AddExternalMode(800, 600);
+ AddExternalMode(640, 480);
+ ASSERT_TRUE(GetResolutions());
+ EXPECT_EQ("800x600", lcd_resolution_);
+ EXPECT_EQ("800x600", external_resolution_);
+ EXPECT_EQ("800x600", screen_resolution_);
+}
+
+// We should use the highest shared resolution when the external output has
+// the higher max resolution.
+TEST_F(ResolutionSelectorTest, ExternalHasHigherResolution) {
+ AddLcdMode(800, 600);
+ AddLcdMode(640, 480);
+ AddExternalMode(1024, 768);
+ AddExternalMode(800, 600);
+ AddExternalMode(640, 480);
+ ASSERT_TRUE(GetResolutions());
+ EXPECT_EQ("800x600", lcd_resolution_);
+ EXPECT_EQ("800x600", external_resolution_);
+ EXPECT_EQ("800x600", screen_resolution_);
+}
+
+// When the maximum resolution offered by the two outputs is different, we
+// should use the max resolution from the lower-res output.
+TEST_F(ResolutionSelectorTest, MismatchedMaxResolution) {
+ AddLcdMode(1024, 600);
+ AddLcdMode(800, 600);
+ AddExternalMode(1280, 720);
+ AddExternalMode(1024, 768);
+ AddExternalMode(800, 600);
+ ASSERT_TRUE(GetResolutions());
+ EXPECT_EQ("1024x600", lcd_resolution_);
+ EXPECT_EQ("1024x768", external_resolution_);
+ EXPECT_EQ("1024x600", screen_resolution_);
+}
+
+// When the external output is large enough that we think it's a monitor,
+// we should just use its maximum resolution instead of trying to find a
+// size that'll also work for the LCD output.
+TEST_F(ResolutionSelectorTest, ExternalOutputIsMonitor) {
+ AddLcdMode(1024, 768);
+ AddLcdMode(800, 600);
+ AddExternalMode(1600, 1200);
+ AddExternalMode(1280, 960);
+ AddExternalMode(1024, 768);
+ ASSERT_GE(external_modes_[0].width * external_modes_[0].height,
+ ResolutionSelector::kMaxProjectorPixels);
+ ASSERT_TRUE(GetResolutions());
+ EXPECT_EQ("1024x768", lcd_resolution_);
+ EXPECT_EQ("1600x1200", external_resolution_);
+ EXPECT_EQ("1600x1200", screen_resolution_);
+}
+
+// We should just fail if there's no common resolution between the two
+// outputs.
+TEST_F(ResolutionSelectorTest, FailIfNoCommonResolution) {
+ AddLcdMode(1024, 768);
+ AddExternalMode(1280, 600);
+ EXPECT_FALSE(GetResolutions());
+}
+
+} // namespace monitor_reconfig
+
+int main(int argc, char** argv) {
+ google::ParseCommandLineFlags(&argc, &argv, true);
+ CommandLine::Init(argc, argv);
+ logging::InitLogging(NULL,
+ FLAGS_logtostderr ?
+ logging::LOG_ONLY_TO_SYSTEM_DEBUG_LOG :
+ logging::LOG_NONE,
+ logging::DONT_LOCK_LOG_FILE,
+ logging::APPEND_TO_OLD_LOG_FILE);
+ ::testing::InitGoogleTest(&argc, argv);
+ return RUN_ALL_TESTS();
+}
