Switchboard Display Plug

Merge lp:~philip.scott/switchboard-plug-display/snaps into lp:~elementary-apps/switchboard-plug-display/trunk

  1. snaps
  2. Merge into trunk
Proposed by Felipe Escoto
Status: Merged
Approved by: Adam Bieńkowski
Approved revision: 173
Merged at revision: 163
Proposed branch: lp:~philip.scott/switchboard-plug-display/snaps
Merge into: lp:~elementary-apps/switchboard-plug-display/trunk
Diff against target: 437 lines (+246/-36)
3 files modified
src/DisplayWidget.vala (+93/-35)
src/DisplaysView.vala (+152/-1)
src/MirrorDisplay.vala (+1/-0)
To merge this branch: bzr merge lp:~philip.scott/switchboard-plug-display/snaps
Reviewer Review Type Date Requested Status
Adam Bieńkowski (community) code Approve
Cassidy James Blaede ux Approve
Review via email: mp+302214@code.launchpad.net

Commit message

- Sanitize extra space
- monitor snapping
- show vertical monitors as vertical
- Auto select a resolution when enabling a display

Description of the change

Fixes a whole bunch of stuff!

To post a comment you must log in.
Revision history for this message
Cassidy James Blaede (cassidyjames) wrote :

This looks mostly great from a UX perspective and moves us much farther from broken and closer to perfect. ;)

One thing I found was that changing the rotation (i.e. to Clockwise) and then back to None does not properly re-size the display when going back to None.

review: Needs Fixing (ux)
Revision history for this message
Cassidy James Blaede (cassidyjames) wrote :

Rotation does appear to be working as expected now. I think my one last gripe is that sometimes disabled displays are corner-snapping to the top-right of my enabled display. This appears only to happen when the enabled display is the "blue" one and the disabled is the "red" one (however that's being numbered under-the-hood). When "red" is enabled and "blue" is disabled, it works as expected.

Revision history for this message
Adam Bieńkowski (donadigo) wrote :

Made a few diff comments, not tested though. Some code style issues, nothing really important, other than that it's a great job!

Revision history for this message
Cassidy James Blaede (cassidyjames) wrote :

Disabled displays appear to have been fixed as well. Giving this a thumbs up UX-wise.

review: Approve (ux)
lp:~philip.scott/switchboard-plug-display/snaps updated
172. By Felipe Escoto

Comapare structs, not strings

173. By Felipe Escoto

use constant

Revision history for this message
Adam Bieńkowski (donadigo) wrote :

Looks okay now! Thanks.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/DisplayWidget.vala'
2--- src/DisplayWidget.vala 2016-04-20 23:26:37 +0000
3+++ src/DisplayWidget.vala 2016-08-08 18:01:16 +0000
4@@ -41,6 +41,11 @@
5 private int real_height = 0;
6 private int real_x = 0;
7 private int real_y = 0;
8+
9+ struct Resolution {
10+ uint width;
11+ uint height;
12+ }
13
14 public DisplayWidget (Gnome.RROutputInfo output_info, Gnome.RROutput output) {
15 display_window = new DisplayWindow (output_info);
16@@ -51,8 +56,8 @@
17 this.output = output;
18 output_info.get_geometry (out real_x, out real_y, out real_width, out real_height);
19 if (!output_info.is_active ()) {
20- real_width = output_info.get_preferred_width ();
21- real_height = output_info.get_preferred_width ();
22+ real_width = 1280;
23+ real_height = 720;
24 }
25
26 primary_image = new Gtk.Button.from_icon_name ("non-starred-symbolic", Gtk.IconSize.MENU);
27@@ -117,6 +122,8 @@
28 rotation_combobox.pack_start (text_renderer, true);
29 rotation_combobox.add_attribute (text_renderer, "text", 0);
30
31+ Resolution[] resolutions = {};
32+ bool resolution_set = false;
33 foreach (unowned Gnome.RRMode mode in output.list_modes ()) {
34 var mode_width = mode.get_width ();
35 var mode_height = mode.get_height ();
36@@ -129,11 +136,17 @@
37 text = "%u × %u".printf (mode_width, mode_height);
38 }
39
40+ Resolution res = {mode_width, mode_height};
41+ if (res in resolutions) continue;
42+ resolutions += res;
43+
44 Gtk.TreeIter iter;
45 resolution_list_store.append (out iter);
46 resolution_list_store.set (iter, 0, text, 1, mode);
47+
48 if (output.get_current_mode () == mode) {
49 resolution_combobox.set_active_iter (iter);
50+ resolution_set = true;
51 }
52 }
53
54@@ -150,56 +163,54 @@
55 output_info.set_active (use_switch.active);
56 resolution_combobox.sensitive = use_switch.active;
57 rotation_combobox.sensitive = use_switch.active;
58+
59+ if (rotation_combobox.active == -1) rotation_combobox.set_active (0);
60+ if (resolution_combobox.active == -1) resolution_combobox.set_active (0);
61+
62+ if (use_switch.active) {
63+ get_style_context ().remove_class ("disabled");
64+ } else {
65+ get_style_context ().add_class ("disabled");
66+ }
67+
68 configuration_changed ();
69 active_changed ();
70 });
71
72- Gtk.TreeIter iter;
73- rotation_list_store.append (out iter);
74- rotation_list_store.set (iter, 0, _("None"), 1, Gnome.RRRotation.ROTATION_0);
75- rotation_combobox.set_active_iter (iter);
76-
77- if (output_info.supports_rotation (Gnome.RRRotation.ROTATION_90)) {
78- rotation_list_store.append (out iter);
79- rotation_list_store.set (iter, 0, _("Clockwise"), 1, Gnome.RRRotation.ROTATION_90);
80- if (output_info.get_rotation () == Gnome.RRRotation.ROTATION_90) {
81- rotation_combobox.set_active_iter (iter);
82- label.angle = 90;
83- }
84- }
85-
86- if (output_info.supports_rotation (Gnome.RRRotation.ROTATION_180)) {
87- rotation_list_store.append (out iter);
88- rotation_list_store.set (iter, 0, _("Flipped"), 1, Gnome.RRRotation.ROTATION_180);
89- if (output_info.get_rotation () == Gnome.RRRotation.ROTATION_180) {
90- rotation_combobox.set_active_iter (iter);
91- label.angle = 180;
92- }
93- }
94-
95- if (output_info.supports_rotation (Gnome.RRRotation.ROTATION_270)) {
96- rotation_list_store.append (out iter);
97- rotation_list_store.set (iter, 0, _("Counterclockwise"), 1, Gnome.RRRotation.ROTATION_270);
98- if (output_info.get_rotation () == Gnome.RRRotation.ROTATION_270) {
99- rotation_combobox.set_active_iter (iter);
100- label.angle = 270;
101- }
102- }
103-
104+ if (!output_info.is_active ()) {
105+ get_style_context ().add_class ("disabled");
106+ }
107+
108+ bool rotation_set = false;
109 resolution_combobox.changed.connect (() => {
110 Value val;
111+ Gtk.TreeIter iter;
112 resolution_combobox.get_active_iter (out iter);
113 resolution_list_store.get_value (iter, 1, out val);
114 set_geometry (real_x, real_y, (int)((Gnome.RRMode) val).get_width (), (int)((Gnome.RRMode) val).get_height ());
115+ rotation_set = false;
116+ rotation_combobox.set_active (0);
117+ rotation_set = true;
118 configuration_changed ();
119 check_position ();
120 });
121
122+ if (!resolution_set) {
123+ resolution_combobox.set_active (0);
124+ }
125+
126 rotation_combobox.changed.connect (() => {
127 Value val;
128+ Gtk.TreeIter iter;
129 rotation_combobox.get_active_iter (out iter);
130 rotation_list_store.get_value (iter, 1, out val);
131- var old_rotation = output_info.get_rotation ();
132+
133+ Gnome.RRRotation old_rotation;
134+ if (!rotation_set) {
135+ old_rotation = Gnome.RRRotation.ROTATION_0;
136+ } else {
137+ old_rotation = output_info.get_rotation ();
138+ }
139 output_info.set_rotation ((Gnome.RRRotation) val);
140 switch ((Gnome.RRRotation) val) {
141 case Gnome.RRRotation.ROTATION_90:
142@@ -240,9 +251,52 @@
143 break;
144 }
145
146+ rotation_set = true;
147 configuration_changed ();
148 check_position ();
149 });
150+
151+ Gtk.TreeIter iter;
152+
153+ rotation_list_store.append (out iter);
154+ rotation_list_store.set (iter, 0, _("None"), 1, Gnome.RRRotation.ROTATION_0);
155+
156+ if (output_info.supports_rotation (Gnome.RRRotation.ROTATION_90)) {
157+ rotation_list_store.append (out iter);
158+ rotation_list_store.set (iter, 0, _("Clockwise"), 1, Gnome.RRRotation.ROTATION_90);
159+ if (output_info.get_rotation () == Gnome.RRRotation.ROTATION_90) {
160+ rotation_combobox.set_active_iter (iter);
161+ label.angle = 90;
162+ rotation_set = true;
163+ }
164+ }
165+
166+ if (output_info.supports_rotation (Gnome.RRRotation.ROTATION_180)) {
167+ rotation_list_store.append (out iter);
168+ rotation_list_store.set (iter, 0, _("Flipped"), 1, Gnome.RRRotation.ROTATION_180);
169+ if (output_info.get_rotation () == Gnome.RRRotation.ROTATION_180) {
170+ rotation_combobox.set_active_iter (iter);
171+ label.angle = 180;
172+ rotation_set = true;
173+ }
174+ }
175+
176+ if (output_info.supports_rotation (Gnome.RRRotation.ROTATION_270)) {
177+ rotation_list_store.append (out iter);
178+ rotation_list_store.set (iter, 0, _("Counterclockwise"), 1, Gnome.RRRotation.ROTATION_270);
179+ if (output_info.get_rotation () == Gnome.RRRotation.ROTATION_270) {
180+ rotation_combobox.set_active_iter (iter);
181+ label.angle = 270;
182+ rotation_set = true;
183+ }
184+ }
185+
186+ if (!rotation_set) {
187+ rotation_combobox.set_active (0);
188+ }
189+
190+ configuration_changed ();
191+ check_position ();
192 }
193
194 public override bool button_press_event (Gdk.EventButton event) {
195@@ -306,6 +360,10 @@
196 output_info.set_geometry (real_x, real_y, real_width, real_height);
197 }
198
199+ public bool equals (DisplayWidget sibling) {
200+ return output_info.get_display_name () == sibling.output_info.get_display_name ();
201+ }
202+
203 // copied from GCC panel
204 public static string? make_aspect_string (uint width, uint height) {
205 uint ratio;
206
207=== modified file 'src/DisplaysView.vala'
208--- src/DisplaysView.vala 2016-04-20 23:26:37 +0000
209+++ src/DisplaysView.vala 2016-08-08 18:01:16 +0000
210@@ -20,8 +20,11 @@
211 */
212
213 public class Display.DisplaysView : Gtk.Overlay {
214+ private const int SNAP_LIMIT = int.MAX - 1;
215+
216 public signal void configuration_changed (bool changed);
217
218+ private bool scanning = false;
219 private double current_ratio = 1.0f;
220 private int current_allocated_width = 0;
221 private int current_allocated_height = 0;
222@@ -37,6 +40,10 @@
223 background-color: %s;
224 color: %s;
225 }
226+
227+ .colored.disabled {
228+ background-color: #aaa;
229+ }
230 """;
231
232 public DisplaysView () {
233@@ -76,6 +83,7 @@
234
235 public void apply_configuration () {
236 try {
237+ rr_config.sanitize ();
238 rr_config.apply_persistent (rr_screen);
239 } catch (Error e) {
240 critical (e.message);
241@@ -83,6 +91,7 @@
242 }
243
244 public void rescan_displays () {
245+ scanning = true;
246 get_children ().foreach ((child) => {
247 if (child is DisplayWidget) {
248 child.destroy ();
249@@ -97,6 +106,8 @@
250 }
251
252 change_active_displays_sensitivity ();
253+ calculate_ratio ();
254+ scanning = false;
255 }
256
257 public void show_windows () {
258@@ -157,7 +168,7 @@
259 });
260 current_allocated_width = get_allocated_width ();
261 current_allocated_height = get_allocated_height ();
262- current_ratio = double.min ((double)(get_allocated_width ()-24) / (double) added_width, (double)(get_allocated_height ()-24) / (double) added_height);
263+ current_ratio = double.min ((double)(get_allocated_width () -24) / (double) added_width, (double)(get_allocated_height ()-24) / (double) added_height);
264 default_x_margin = (int) ((get_allocated_width () - max_width*current_ratio)/2);
265 default_y_margin = (int) ((get_allocated_height () - max_height*current_ratio)/2);
266 }
267@@ -194,6 +205,8 @@
268 display_widget.active_changed.connect (() => {
269 active_displays += display_widget.output_info.is_active () ? 1 : -1;
270 change_active_displays_sensitivity ();
271+ check_configuration_changed ();
272+ calculate_ratio ();
273 });
274
275 if (!rr_config.get_clone () && output_info.is_active ()) {
276@@ -206,6 +219,8 @@
277 display_widget.set_geometry ((int)(delta_x / current_ratio) + x, (int)(delta_y / current_ratio) + y, width, height);
278 display_widget.queue_resize_no_redraw ();
279 check_configuration_changed ();
280+ snap_edges (display_widget);
281+ calculate_ratio ();
282 });
283
284 check_intersects (display_widget);
285@@ -286,4 +301,140 @@
286
287 source_display_widget.queue_resize_no_redraw ();
288 }
289+
290+ public void snap_edges (DisplayWidget last_moved) {
291+ if (scanning) return;
292+ // Snap last_moved
293+ debug ("Snapping displays");
294+ var anchors = new List<DisplayWidget>();
295+ get_children ().foreach ((child) => {
296+ if (!(child is DisplayWidget) || last_moved.equals ((DisplayWidget)child)) return;
297+ anchors.append ((DisplayWidget) child);
298+ });
299+
300+ snap_widget (last_moved, anchors);
301+
302+ /*/ FIXME: Re-Snaping with 3 or more displays is broken
303+ // This is used to make sure all displays are connected
304+ anchors = new List<DisplayWidget>();
305+ get_children ().foreach ((child) => {
306+ if (!(child is DisplayWidget)) return;
307+ snap_widget ((DisplayWidget) child, anchors);
308+ anchors.append ((DisplayWidget) child);
309+ });*/
310+ }
311+
312+ /******************************************************************************************
313+ * Widget snaping is done by trying to snap a child to other widgets called Anchors. *
314+ * It first calculates the distance between each anchor and the child, and afterwards *
315+ * snaps the child to the closest edge *
316+ * *
317+ * Cases: C = child, A = current anchor *
318+ * *
319+ * 1. 2. 3. 4. *
320+ * A C C A A C *
321+ * C A *
322+ * *
323+ ******************************************************************************************/
324+ private void snap_widget (Display.DisplayWidget child, List<Display.DisplayWidget> anchors) {
325+ if (anchors.length () == 0) return;
326+ int child_x, child_y, child_width, child_height;
327+ child.get_geometry (out child_x, out child_y, out child_width, out child_height);
328+ debug ("Child: %d %d %d %d\n", child_x, child_y, child_width, child_height);
329+
330+ bool snap_y = false, snap_x = false, move = false, diagonally = false;
331+ int case_1 = int.MAX, case_2 = int.MAX, case_3 = int.MAX, case_4 = int.MAX;
332+
333+ foreach (var anchor in anchors) {
334+ if (child.equals (anchor)) continue;
335+
336+ int anchor_x, anchor_y, anchor_width, anchor_height;
337+ anchor.get_geometry (out anchor_x, out anchor_y, out anchor_width, out anchor_height);
338+ debug ("Anchor: %d %d %d %d\n", anchor_x, anchor_y, anchor_width, anchor_height);
339+
340+ int case_1_t = child_x - anchor_x - anchor_width;
341+ int case_2_t = child_x - anchor_x + child_width;
342+ int case_3_t = child_y - anchor_y - anchor_height;
343+ int case_4_t = child_height + child_y - anchor_y;
344+
345+ // Check projections
346+ if (is_projected (child_y, child_height, anchor_y, anchor_height)) {
347+ debug ("Child is on the X axis of Anchor %s\n", anchor.output_info.get_display_name ());
348+ case_1 = is_x_smaller_absolute (case_1, case_1_t) && !diagonally ? case_1 : case_1_t;
349+ case_2 = is_x_smaller_absolute (case_2, case_2_t) && !diagonally ? case_2 : case_2_t;
350+ snap_x = true;
351+ move = true;
352+ } else if (is_projected (child_x, child_width, anchor_x, anchor_width)) {
353+ debug ("Child is on the Y axis of Anchor %s\n", anchor.output_info.get_display_name ());
354+ case_3 = is_x_smaller_absolute (case_3, case_3_t) && !diagonally ? case_3 : case_3_t;
355+ case_4 = is_x_smaller_absolute (case_4, case_4_t) && !diagonally ? case_4 : case_4_t;
356+ snap_y = true;
357+ move = true;
358+ } else {
359+ debug ("Child is diagonally of Anchor %s\n", anchor.output_info.get_display_name ());
360+ if (!move) {
361+ diagonally = true;
362+ case_1 = is_x_smaller_absolute (case_1, case_1_t) ? case_1 : case_1_t;
363+ case_2 = is_x_smaller_absolute (case_2, case_2_t) ? case_2 : case_2_t;
364+ case_3 = is_x_smaller_absolute (case_3, case_3_t) ? case_3 : case_3_t;
365+ case_4 = is_x_smaller_absolute (case_4, case_4_t) ? case_4 : case_4_t;
366+ }
367+ }
368+ }
369+
370+ int shortest_x = is_x_smaller_absolute (case_1, case_2) ? case_1 : case_2;
371+ int shortest_y = is_x_smaller_absolute (case_3, case_4) ? case_3 : case_4;
372+
373+ // X Snapping
374+ if (!snap_y || is_x_smaller_absolute (shortest_x, shortest_y)) {
375+ if (snap_x & move) {
376+ debug ("moving child %d on X\n", shortest_x);
377+ if (shortest_x < SNAP_LIMIT) ((DisplayWidget) child).set_geometry (child_x - shortest_x, child_y , child_width, child_height);
378+ }
379+ }
380+
381+ // Y Snapping
382+ if (!snap_x || is_x_smaller_absolute (shortest_y, shortest_x)) {
383+ if (snap_y & move) {
384+ debug ("moving child %d on Y\n", shortest_y);
385+ if (shortest_y < SNAP_LIMIT) ((DisplayWidget) child).set_geometry (child_x , child_y - shortest_y, child_width, child_height);
386+ }
387+ }
388+
389+ // X & Y Snapping
390+ if (!snap_x && !snap_y) {
391+ if (shortest_x < SNAP_LIMIT && shortest_y < SNAP_LIMIT) {
392+ ((DisplayWidget) child).set_geometry (child_x - shortest_x, child_y - shortest_y , child_width, child_height);
393+ debug ("moving child %d on X & %d on Y\n", shortest_x, shortest_y);
394+ } else {
395+ debug ("too large");
396+ }
397+ }
398+ }
399+
400+ static bool equals = false;
401+ private bool is_projected (int child_x, int child_length, int anchor_x, int anchor_length) {
402+ var numberline = new List<int> ();
403+
404+ equals = false;
405+ CompareFunc<int> intcmp = (a, b) => {
406+ if (a == b) equals = true;
407+ return (int) (a > b) - (int) (a < b);
408+ };
409+
410+ var child_x2 = child_x + child_length;
411+ var anchor_x2 = anchor_x + anchor_length;
412+
413+ numberline.insert_sorted (child_x, intcmp);
414+ numberline.insert_sorted (child_x2, intcmp);
415+ numberline.insert_sorted (anchor_x, intcmp);
416+ numberline.insert_sorted (anchor_x2, intcmp);
417+
418+ if (equals) return false;
419+ return !((numberline.index (child_x) - numberline.index (child_x2)).abs () == 1 && (numberline.index (anchor_x) - numberline.index (anchor_x2)).abs () == 1);
420+ }
421+
422+ private bool is_x_smaller_absolute (int x, int y) {
423+ return x.abs () < y.abs ();
424+ }
425 }
426
427=== modified file 'src/MirrorDisplay.vala'
428--- src/MirrorDisplay.vala 2016-04-20 23:26:37 +0000
429+++ src/MirrorDisplay.vala 2016-08-08 18:01:16 +0000
430@@ -97,6 +97,7 @@
431
432 public void apply_configuration () {
433 try {
434+ rr_config.sanitize ();
435 rr_config.apply_persistent (rr_screen);
436 } catch (Error e) {
437 critical (e.message);

Subscribers

People subscribed via source and target branches

to all changes: