Merge lp:~kaihengfeng/unity/set-hidpi-scale-factor into lp:unity

[Kai-Heng Feng]

UnitySettings: If scale-factor is not set, find and set right scale for HiDPI displays.

[Kai-Heng Feng (kaihengfeng)]

Set gsettings value is needed, otherwise Unity Control Center's Display will reset it because scale-factor is empty.

[Marco Trevisan (Treviño) (3v1n0)]

Mh, this is something I always wanted to do, so thanks.

However I'm not sure is something that should be done at unity level, since it could cause to override the user setting.
Maybe this should be done only if the value is not the default one, I guess.

The preferred way to do this, would be using a migration script though. You can write it in python and follow the same we do for ./tools/migration-scripts/*.

I'd like to discuss about the algorithm to find the proper scaling value. Since we also support non-integer values, it would be nicer to define a scaling to be proportional to display size by using some thresholds.

[Kai-Heng Feng (kaihengfeng)]

I think this won't override user setting, as long as it's greater than 0.

With this algorithm, a 27 inches 4K display will have scale factor 1.
So yes, I think if the scale can be proportional to display size would be great.

[Marco Trevisan (Treviño) (3v1n0)]

Ok, overriding a 0 value is fine then.

As for the thresholds calculations, can we do some testing?

[Kai-Heng Feng (kaihengfeng)]

Sure.

I think we can make the scale factor being proportional to the DPI, if the DPI is in range of DEFAULT_DPI to DEFAULT_DPI * 2. Your thought?

[Marco Trevisan (Treviño) (3v1n0)]

Yeah, I agree.

[Kai-Heng Feng (kaihengfeng)]

I choose 140 DPI as a base because it's the DPI on a 15" FHD screen. And both 15" UHD screen and 13" QHD+ screen has roughly 280 DPI, both of them look great with scale factor 2.

[Marco Trevisan (Treviño) (3v1n0)]

We're there, just a few style fixes.

[Kai-Heng Feng (kaihengfeng)]

Since the value eventually set into scale-factor is an integer, I use the same type for DPI_SCALING_STEP.

[Andrea Azzarone (azzar1)]

Can you explain why the check on the sizes as per diff comment?

review: Needs Information

[Kai-Heng Feng (kaihengfeng)]

It's from Unity Settings Daemon [1].
I believe this size check is cherry-picked from Gnome Settings Daemon.

[1] https://bazaar.launchpad.net/~unity-settings-daemon-team/unity-settings-daemon/trunk/view/head:/plugins/xsettings/gsd-xsettings-manager.c#L491

[Kai-Heng Feng (kaihengfeng)]

Any chance to merge this patch? :)

[Kai-Heng Feng (kaihengfeng)]

Currently our team has several high-end machines with UHD/QHD display need to be enabled, so auto-scale will be great if it can work out of the box.

Is there any improvement for this patch I can make?

[Andrea Azzarone (azzar1)]

Code looks good to me and it works fine on my 4k monitor.

review: Approve

=== modified file 'unity-shared/UScreen.cpp'
--- unity-shared/UScreen.cpp	2014-12-18 18:43:36 +0000
+++ unity-shared/UScreen.cpp	2016-07-07 07:45:15 +0000
@@ -88,6 +88,11 @@
   return monitors_[monitor];
 }
 
+nux::Size const& UScreen::GetMonitorPhysicalSize(int monitor) const
+{
+  return physical_monitors_[monitor];
+}
+
 std::vector<nux::Geometry> const& UScreen::GetMonitors() const
 {
   return monitors_;
@@ -154,6 +159,7 @@
 
   nux::Geometry last_geo;
   monitors_.clear();
+  physical_monitors_.clear();
   primary_ = gdk_screen_get_primary_monitor(screen_);
   int monitors = gdk_screen_get_n_monitors(screen_);
 
@@ -164,6 +170,9 @@
 
     float scale = gdk_screen_get_monitor_scale_factor(screen_, i);
     nux::Geometry geo(rect.x, rect.y, rect.width, rect.height);
+    
+    nux::Size physical_size(gdk_screen_get_monitor_width_mm(screen_, i),
+      gdk_screen_get_monitor_height_mm(screen_, i));
 
     if (scale != 1.0)
       geo = geo * scale;
@@ -174,6 +183,7 @@
 
     last_geo = geo;
     monitors_.push_back(geo);
+    physical_monitors_.push_back(physical_size);
 
     LOG_DEBUG(logger) << "Monitor " << i << " has geometry " << geo.x << "x"
                       << geo.y << "x" << geo.width << "x" << geo.height;

=== modified file 'unity-shared/UScreen.h'
--- unity-shared/UScreen.h	2014-10-21 15:43:16 +0000
+++ unity-shared/UScreen.h	2016-07-07 07:45:15 +0000
@@ -45,6 +45,7 @@
   int GetMonitorWithMouse() const;
   int GetMonitorAtPosition(int x, int y) const;
   nux::Geometry const& GetMonitorGeometry(int monitor) const;
+  nux::Size const& GetMonitorPhysicalSize(int monitor) const;
 
   std::vector<nux::Geometry> const& GetMonitors() const;
   nux::Geometry GetScreenGeometry() const;
@@ -63,6 +64,7 @@
 protected:
   static UScreen* default_screen_;
   std::vector<nux::Geometry> monitors_;
+  std::vector<nux::Size> physical_monitors_;
   int primary_;
 
 private:

=== modified file 'unity-shared/UnitySettings.cpp'
--- unity-shared/UnitySettings.cpp	2016-07-04 14:21:00 +0000
+++ unity-shared/UnitySettings.cpp	2016-07-07 07:45:15 +0000
@@ -76,6 +76,8 @@
 const int MINIMUM_DESKTOP_HEIGHT = 800;
 const int GNOME_SETTINGS_CHANGED_WAIT_SECONDS = 1;
 const double DEFAULT_DPI = 96.0f;
+const double DPI_SCALING_LIMIT = 140.0f;
+const int DPI_SCALING_STEP = 8;
 }
 
 //
@@ -322,6 +324,34 @@
     return em_converters_[monitor];
   }
 
+  int FindOptimalScale(const UScreen* uscreen, const int monitor)
+  {
+    auto const& geo = uscreen->GetMonitorGeometry(monitor);
+    auto const& size = uscreen->GetMonitorPhysicalSize(monitor);
+    auto scale = DPI_SCALING_STEP;
+      
+    if ((size.width == 160 && size.height == 90) ||
+        (size.width == 160 && size.height == 100) ||
+        (size.width == 16 && size.height == 9) ||
+        (size.width == 16 && size.height == 10))
+    {
+      return scale;
+    }
+
+    if (size.width > 0 && size.height > 0)
+    {
+      const double dpi_x = static_cast<double>(geo.width) / (size.width / 25.4);  
+      const double dpi_y = static_cast<double>(geo.height) / (size.height / 25.4);
+
+      const auto dpi = std::max(dpi_x, dpi_y);
+
+      if (dpi > DPI_SCALING_LIMIT)
+        scale = static_cast<int>(std::lround(scale * dpi / DPI_SCALING_LIMIT));
+    }
+
+    return scale;
+  }
+
   void UpdateDPI()
   {
     auto* uscreen = UScreen::GetDefault();
@@ -332,6 +362,10 @@
     glib::Variant dict;
     g_settings_get(ubuntu_ui_settings_, SCALE_FACTOR.c_str(), "@a{si}", &dict);
 
+    bool dict_changed = false;
+    GVariantBuilder builder;
+    g_variant_builder_init(&builder, G_VARIANT_TYPE("a{si}"));
+
     glib::String app_target_monitor(g_settings_get_string(ui_settings_, APP_SCALE_MONITOR.c_str()));
     double app_target_scale = 0;
 
@@ -345,8 +379,18 @@
         double ui_scale = 1.0f;
         int value;
 
-        if (g_variant_lookup(dict, monitor_name.c_str(), "i", &value) && value > 0)
-          ui_scale = static_cast<double>(value)/8.0f;
+        if (g_variant_lookup(dict, monitor_name.c_str(), "i", &value))
+        {
+          if (value > 0)
+            ui_scale = static_cast<double>(value)/DPI_SCALING_STEP;
+        }
+        else 
+        {
+          value = FindOptimalScale(uscreen, monitor);
+          ui_scale = static_cast<double>(value)/DPI_SCALING_STEP;
+          dict_changed = true;
+        }
+        g_variant_builder_add(&builder, "{si}", monitor_name.c_str(), value);
 
         if (app_target_monitor.Str() == monitor_name)
           app_target_scale = ui_scale;
@@ -360,6 +404,12 @@
         any_changed = true;
     }
 
+    glib::Variant new_dict(g_variant_builder_end(&builder));
+    if (dict_changed)
+    {
+      g_settings_set_value(ubuntu_ui_settings_, SCALE_FACTOR.c_str(), new_dict);
+    }
+
     if (app_target_scale == 0)
       app_target_scale = (g_settings_get_boolean(ui_settings_, APP_USE_MAX_SCALE.c_str())) ? max_scale : min_scale;