Mir

Merge lp:~alan-griffiths/mir/remove-constraints-on-surface-placement into lp:mir

  1. remove-constraints-on-surface-placement
  2. Merge into development-branch
Proposed by Alan Griffiths
Status: Merged
Approved by: Alan Griffiths
Approved revision: no longer in the source branch.
Merged at revision: 2524
Proposed branch: lp:~alan-griffiths/mir/remove-constraints-on-surface-placement
Merge into: lp:mir
Prerequisite: lp:~alan-griffiths/mir/more-surface-resize
Diff against target: 162 lines (+8/-92)
2 files modified
examples/server_example_canonical_window_manager.cpp (+4/-46)
src/server/shell/canonical_window_manager.cpp (+4/-46)
To merge this branch: bzr merge lp:~alan-griffiths/mir/remove-constraints-on-surface-placement
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+257114@code.launchpad.net

Commit message

shell, examples: remove excessive constraint on surface position
(LP: #1447882)

Description of the change

shell, examples: remove excessive constraint on surface position

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:2509
http://jenkins.qa.ubuntu.com/job/mir-ci/3613/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2174
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/2173
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2124
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1610/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2124
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2124/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5041
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19901

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/3613/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

7: [ FAILED ] TestClientInput.keymap_changes_change_keycode_received

Not touched - lp:1448210

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2511
http://jenkins.qa.ubuntu.com/job/mir-ci/3621/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2186
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/2185
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2136
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1618
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1618/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2136
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2136/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5053
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19928

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/3621/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2512
http://jenkins.qa.ubuntu.com/job/mir-ci/3623/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2188
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/2187
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2138
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1620
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1620/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2138
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2138/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5055
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19938

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/3623/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2513
http://jenkins.qa.ubuntu.com/job/mir-ci/3625/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2191
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/2190
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2141
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1622
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1622/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2141
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2141/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5058
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19945

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/3625/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2514
http://jenkins.qa.ubuntu.com/job/mir-ci/3634/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2207
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/2206
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2157
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1631
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1631/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2157
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2157/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5071
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19970

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/3634/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2515
http://jenkins.qa.ubuntu.com/job/mir-ci/3640/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2213
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/2212
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2163
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1637
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1637/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2163
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2163/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5076
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19990

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/3640/rebuild

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

That feels better.

(1) This looks like a TODO (which it kind of is, but isn't):
46 + // placeholder - constrain onscreen
If anything in future you'd just want a little logic that ensures a window rectangle always intersects the screen rectangle somehow so you don't lose it entirely. But even that's not so such an issue with dragging as with display layout reconfiguration.

(2) Keep in mind that window snapping would eventually need more than a single rectangle:
93 + Rectangle const& /*bounds*/)
You would indeed use a bounding rectangle for the screen, but also the set of all window rectangles whose exterior edges can be snapped to.

review: Approve
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> That feels better.
>
> (1) This looks like a TODO (which it kind of is, but isn't):
> 46 + // placeholder - constrain onscreen
> If anything in future you'd just want a little logic that ensures a window
> rectangle always intersects the screen rectangle somehow so you don't lose it
> entirely. But even that's not so such an issue with dragging as with display
> layout reconfiguration.
>
> (2) Keep in mind that window snapping would eventually need more than a single
> rectangle:
> 93 + Rectangle const& /*bounds*/)
> You would indeed use a bounding rectangle for the screen, but also the set of
> all window rectangles whose exterior edges can be snapped to.

Agreed

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'examples/server_example_canonical_window_manager.cpp'
2--- examples/server_example_canonical_window_manager.cpp 2015-04-28 16:24:07 +0000
3+++ examples/server_example_canonical_window_manager.cpp 2015-04-28 16:24:07 +0000
4@@ -722,7 +722,7 @@
5 Size const& requested_size,
6 bool const left_resize,
7 bool const top_resize,
8- Rectangle const& bounds)
9+ Rectangle const& /*bounds*/)
10 {
11 auto const& surface_info = tools->info_for(surface);
12
13@@ -816,35 +816,7 @@
14 if (top_resize)
15 new_pos.y += new_size.height - requested_size.height;
16
17- if (left_resize)
18- {
19- if (new_pos.x < bounds.top_left.x)
20- {
21- new_size.width = new_size.width + (new_pos.x - bounds.top_left.x);
22- new_pos.x = bounds.top_left.x;
23- }
24- }
25- else
26- {
27- auto to_bottom_right = bounds.bottom_right() - (new_pos + as_displacement(new_size));
28- if (to_bottom_right.dx < DeltaX{0})
29- new_size.width = new_size.width + to_bottom_right.dx;
30- }
31-
32- if (top_resize)
33- {
34- if (new_pos.y < bounds.top_left.y)
35- {
36- new_size.height = new_size.height + (new_pos.y - bounds.top_left.y);
37- new_pos.y = bounds.top_left.y;
38- }
39- }
40- else
41- {
42- auto to_bottom_right = bounds.bottom_right() - (new_pos + as_displacement(new_size));
43- if (to_bottom_right.dy < DeltaY{0})
44- new_size.height = new_size.height + to_bottom_right.dy;
45- }
46+ // placeholder - constrain onscreen
47
48 switch (surface_info.state)
49 {
50@@ -884,7 +856,7 @@
51 return true;
52 }
53
54-bool me::CanonicalWindowManagerPolicyCopy::drag(std::shared_ptr<ms::Surface> surface, Point to, Point from, Rectangle bounds)
55+bool me::CanonicalWindowManagerPolicyCopy::drag(std::shared_ptr<ms::Surface> surface, Point to, Point from, Rectangle /*bounds*/)
56 {
57 if (!surface)
58 return false;
59@@ -892,23 +864,9 @@
60 if (!surface->input_area_contains(from) && !tools->info_for(surface).titlebar)
61 return false;
62
63- auto const top_left = surface->top_left();
64- auto const surface_size = surface->size();
65- auto const bottom_right = top_left + as_displacement(surface_size);
66-
67 auto movement = to - from;
68
69- if (movement.dx < DeltaX{0})
70- movement.dx = std::max(movement.dx, (bounds.top_left - top_left).dx);
71-
72- if (movement.dy < DeltaY{0})
73- movement.dy = std::max(movement.dy, (bounds.top_left - top_left).dy);
74-
75- if (movement.dx > DeltaX{0})
76- movement.dx = std::min(movement.dx, (bounds.bottom_right() - bottom_right).dx);
77-
78- if (movement.dy > DeltaY{0})
79- movement.dy = std::min(movement.dy, (bounds.bottom_right() - bottom_right).dy);
80+ // placeholder - constrain onscreen
81
82 move_tree(surface, movement);
83
84
85=== modified file 'src/server/shell/canonical_window_manager.cpp'
86--- src/server/shell/canonical_window_manager.cpp 2015-04-28 16:24:07 +0000
87+++ src/server/shell/canonical_window_manager.cpp 2015-04-28 16:24:07 +0000
88@@ -637,7 +637,7 @@
89 Size const& requested_size,
90 bool const left_resize,
91 bool const top_resize,
92- Rectangle const& bounds)
93+ Rectangle const& /*bounds*/)
94 {
95 auto const& surface_info = tools->info_for(surface);
96
97@@ -731,35 +731,7 @@
98 if (top_resize)
99 new_pos.y += new_size.height - requested_size.height;
100
101- if (left_resize)
102- {
103- if (new_pos.x < bounds.top_left.x)
104- {
105- new_size.width = new_size.width + (new_pos.x - bounds.top_left.x);
106- new_pos.x = bounds.top_left.x;
107- }
108- }
109- else
110- {
111- auto to_bottom_right = bounds.bottom_right() - (new_pos + as_displacement(new_size));
112- if (to_bottom_right.dx < DeltaX{0})
113- new_size.width = new_size.width + to_bottom_right.dx;
114- }
115-
116- if (top_resize)
117- {
118- if (new_pos.y < bounds.top_left.y)
119- {
120- new_size.height = new_size.height + (new_pos.y - bounds.top_left.y);
121- new_pos.y = bounds.top_left.y;
122- }
123- }
124- else
125- {
126- auto to_bottom_right = bounds.bottom_right() - (new_pos + as_displacement(new_size));
127- if (to_bottom_right.dy < DeltaY{0})
128- new_size.height = new_size.height + to_bottom_right.dy;
129- }
130+ // placeholder - constrain onscreen
131
132 switch (surface_info.state)
133 {
134@@ -798,27 +770,13 @@
135 return true;
136 }
137
138-bool msh::CanonicalWindowManagerPolicy::drag(std::shared_ptr<ms::Surface> surface, Point to, Point from, Rectangle bounds)
139+bool msh::CanonicalWindowManagerPolicy::drag(std::shared_ptr<ms::Surface> surface, Point to, Point from, Rectangle /*bounds*/)
140 {
141 if (surface && surface->input_area_contains(from))
142 {
143- auto const top_left = surface->top_left();
144- auto const surface_size = surface->size();
145- auto const bottom_right = top_left + as_displacement(surface_size);
146-
147 auto movement = to - from;
148
149- if (movement.dx < DeltaX{0})
150- movement.dx = std::max(movement.dx, (bounds.top_left - top_left).dx);
151-
152- if (movement.dy < DeltaY{0})
153- movement.dy = std::max(movement.dy, (bounds.top_left - top_left).dy);
154-
155- if (movement.dx > DeltaX{0})
156- movement.dx = std::min(movement.dx, (bounds.bottom_right() - bottom_right).dx);
157-
158- if (movement.dy > DeltaY{0})
159- movement.dy = std::min(movement.dy, (bounds.bottom_right() - bottom_right).dy);
160+ // placeholder - constrain onscreen
161
162 move_tree(surface, movement);
163

Subscribers

People subscribed via source and target branches