Merge lp:~alan-griffiths/mir/more-surface-resize into lp:mir
- more-surface-resize
- Merge into development-branch
Status: | Merged |
---|---|
Approved by: | Alan Griffiths |
Approved revision: | no longer in the source branch. |
Merged at revision: | 2523 |
Proposed branch: | lp:~alan-griffiths/mir/more-surface-resize |
Merge into: | lp:mir |
Diff against target: |
1181 lines (+634/-172) 14 files modified
examples/server_example_canonical_window_manager.cpp (+115/-58) examples/server_example_canonical_window_manager.h (+6/-2) include/client/mir_toolkit/mir_surface.h (+53/-0) include/server/mir/scene/surface_creation_parameters.h (+5/-0) include/server/mir/shell/surface_specification.h (+6/-0) src/client/mir_surface.cpp (+34/-0) src/client/mir_surface.h (+6/-0) src/client/mir_surface_api.cpp (+48/-0) src/client/symbols.map (+6/-2) src/protobuf/mir_protobuf.proto (+14/-0) src/server/frontend/session_mediator.cpp (+26/-0) src/server/shell/canonical_window_manager.cpp (+115/-55) src/server/shell/canonical_window_manager.h (+6/-2) tests/acceptance-tests/test_surface_modifications.cpp (+194/-53) |
To merge this branch: | bzr merge lp:~alan-griffiths/mir/more-surface-resize |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot (community) | continuous-integration | Approve | |
Alexandros Frantzis (community) | Approve | ||
Kevin DuBois (community) | Approve | ||
Alan Griffiths | Abstain | ||
Alberto Aguirre (community) | Approve | ||
Andreas Pokorny (community) | Approve | ||
Daniel van Vugt | Needs Fixing | ||
Review via email: mp+256912@code.launchpad.net |
Commit message
shell: handle size increments and aspect ratios
Description of the change
shell: handle size increments and aspect ratios
PS Jenkins bot (ps-jenkins) wrote : | # |
Daniel van Vugt (vanvugt) wrote : | # |
This seems a bit odd and smells fishy:
examples/
Why any change to examples? The policy and logic for resize constraints should be entirely in src/server/. Nothing should need to be implemented by consumers of CanonicalWindow
The default policy should just use best fit and modify any requested dimensions so that they satisfy the constraints. For example:
* Policy is 16:9 aspect ratio and shell tries to set 80x80 -> new size is 80x45
* Policy is increments of 8x16 (a terminal) and shell tries to set 812x323 -> new size is 808x320 or 816x336 (depending on the previous dimensions and direction of resize).
So the goal is policy enforcement with the simplest interface possible, which is no code change required to any shell.
Alan Griffiths (alan-griffiths) wrote : | # |
The changes to examples are because the team decided that me::CanonicalWi
Daniel van Vugt (vanvugt) wrote : | # |
(surprised but question answered :)
Daniel van Vugt (vanvugt) wrote : | # |
(1) Setting aspect ratio the user usually just wants it fixed (video players) or completely free (say zeros). So it would ease usability if these were a single function with five parameters:
458 +bool mir_surface_
470 +bool mir_surface_
Similarly, when you set the resize granularity, it should be a single function instead of two:
236 +bool mir_surface_
249 +bool mir_surface_
Arguably all the resize constraints should be bundled into a single structure, but I think that would contradict the new design too much.
Alan Griffiths (alan-griffiths) wrote : | # |
*Needs Discussion*
> (1) Setting aspect ratio the user usually just wants it fixed (video players)
> or completely free (say zeros). So it would ease usability if these were a
> single function with five parameters:
>
> 458 +bool mir_surface_
> unsigned x, unsigned y)
> 470 +bool mir_surface_
> unsigned x, unsigned y)
>
> Similarly, when you set the resize granularity, it should be a single function
> instead of two:
>
> 236 +bool mir_surface_
> width_inc);
> 249 +bool mir_surface_
> height_inc);
>
> Arguably all the resize constraints should be bundled into a single structure,
> but I think that would contradict the new design too much.
I'm not a fan of interfaces that use "magic values" (say zeros) but will go with what the majority prefers.
Andreas Pokorny (andreas-pokorny) wrote : | # |
Nit: + 240 s/hight/height
Maintaining ABI compatibility of caller stored structures seems harder than having separate functions. Given the use cases I can think of right now, most clients might only use the width/height increment functions or the aspect ration functions. Hence those could be combined to single functions. Then again most of the existing setter 'methods' are unary.
Kevin DuBois (kdub) wrote : | # |
I have a slight preference for more a well-defined functions in the api as opposed to one function that rolls a lot of tasks in.
I'd be okay with the api as-is, or reducing to 3 new functions seems good to me as well:
mir_surface_
mir_surface_
mir_surface_
needs-info:
Do we need a way to reset the width_inc/
Alan Griffiths (alan-griffiths) wrote : | # |
> I have a slight preference for more a well-defined functions in the api as
> opposed to one function that rolls a lot of tasks in.
> I'd be okay with the api as-is, or reducing to 3 new functions seems good to
> me as well:
> mir_surface_
> mir_surface_
> mir_surface_
I think this needs to be consistent. If we take this approach we should also merge:
mir_surface_
mir_surface_
> needs-info:
> Do we need a way to reset the width_inc/
Good question!
One that should also apply to [min|max]
Alberto Aguirre (albaguirre) wrote : | # |
I think the 4 apis are just fine. It's just a surface spec, so the value are not in flight until it's commited.
Having them separate is consistent with other existing API's ( set_width and set_height, etc.)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:2514
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Alan Griffiths (alan-griffiths) wrote : | # |
I think the majority view is that this API is fine.
~~~~
But we may need another MP to add a bunch of "reset" functions for surface properties that can be set.
Or, alternatively, have some values that give default behavior:
mir_surface_
mir_surface_
mir_surface_
etc.
Gerry Boland (gerboland) wrote : | # |
As outside perspective, I like this api. I modify the MirSurfaceSpec one property at a time, and know if each modification was allowed or not. When ready, commit.
While on that subject, is there a possibility of a race between me calling mir_surface_
Nitpick:
Mir never shied away from long descriptive function names. Why abbreviate "increment" to "inc"? My first guess for the meaning of "inc" is not increment.
Alan Griffiths (alan-griffiths) wrote : | # |
> As outside perspective, I like this api. I modify the MirSurfaceSpec one
> property at a time, and know if each modification was allowed or not. When
> ready, commit.
>
> While on that subject, is there a possibility of a race between me calling
> mir_surface_
> and the client changing those values?
Well, by definition, that is a race. What will happen is that the server will handle the requests in some order. The resulting surface size could depend upon the order.
> Nitpick:
> Mir never shied away from long descriptive function names. Why abbreviate
> "increment" to "inc"? My first guess for the meaning of "inc" is not
> increment.
Fair. I'll fix.
Kevin DuBois (kdub) wrote : | # |
> But we may need another MP to add a bunch of "reset" functions for surface
> properties that can be set.
So, after the discussion today, I think that its something we might need, but could come once the need arises.
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2515
http://
Executed test runs:
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Alan Griffiths (alan-griffiths) wrote : | # |
101: [ RUN ] AndroidInputRec
101: /mir/tests/
101: Expected: (duration) > (1ms), actual: 8-byte object <C7-2B 0F-00 00-00 00-00> vs 8-byte object <01-00 00-00 00-00 00-00>
101: [ FAILED ] AndroidInputRec
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:2516
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Kevin DuBois (kdub) wrote : | # |
needs-fixings:
122+ auto const error = new_size.
c-cast
264+ * \param [in] x numerator
265+ * \param [in] y denominator
Its obvious from context that the ratio is x/y, but might make it clearer that that the ratio is x/y (and also if the ratio gets affected by 90 degree rotation requests)
around l383, should we check that the min aspect ratio is less than the max?
227+ * Defines an arithmetic progression of sizes starting with min_width (if set, otherwise 0)
would a default of 1 be better? That way the server wouldn't be stepping over the clients default of 0 by default. (zero seems to be a request to fix the surface size to min_width, min_height)
Alexandros Frantzis (afrantzis) wrote : | # |
267 + * \note The requested aspect ration is a hint only. The server is not guaranteed
279 + * \note The requested aspect ration is a hint only. The server is not guaranteed
408+ struct AspectRation { unsigned x; unsigned y; };
Typo 'ration'.
261+ * Set the minimum aspect ratio
262+ *
263+ * \param [in] spec Specification to mutate
264+ * \param [in] x numerator
265+ * \param [in] y denominator
I think it would be clearer to express the aspect ratio in terms of width and height, not x and y.
715+ auto const error = new_size.
716+
717+ if (error > 0)
Should this be the other way around?
error > 0 => W*y-H*x > 0 => W/H > x/y => the new aspect ratio is larger than the min aspect ratio so we shouldn't need to touch the W,H values, or am I misunderstanding what "min aspect ratio" means?
Needs fixing/info
Alan Griffiths (alan-griffiths) wrote : | # |
> 267 + * \note The requested aspect ration is a hint only. The server is
> not guaranteed
> 279 + * \note The requested aspect ration is a hint only. The server is
> not guaranteed
>
> 408+ struct AspectRation { unsigned x; unsigned y; };
>
> Typo 'ration'.
>
> 261+ * Set the minimum aspect ratio
> 262+ *
> 263+ * \param [in] spec Specification to mutate
> 264+ * \param [in] x numerator
> 265+ * \param [in] y denominator
>
> I think it would be clearer to express the aspect ratio in terms of width and
> height, not x and y.
>
> 715+ auto const error = new_size.
> new_size.
> 716+
> 717+ if (error > 0)
>
> Should this be the other way around?
>
> error > 0 => W*y-H*x > 0 => W/H > x/y => the new aspect ratio is larger than
> the min aspect ratio so we shouldn't need to touch the W,H values, or am I
> misunderstanding what "min aspect ratio" means?
>
> Needs fixing/info
fixed
Alan Griffiths (alan-griffiths) wrote : | # |
> needs-fixings:
> 122+ auto const error = new_size.
> new_size.
> c-cast
fixed
> 264+ * \param [in] x numerator
> 265+ * \param [in] y denominator
> Its obvious from context that the ratio is x/y, but might make it clearer that
> that the ratio is x/y (and also if the ratio gets affected by 90 degree
> rotation requests)
I'm not clear what this is asking for.
> around l383, should we check that the min aspect ratio is less than the max?
We could, but would we handle it any differently?
> 227+ * Defines an arithmetic progression of sizes starting with min_width (if
> set, otherwise 0)
> would a default of 1 be better?
I don't follow. If min_width isn't set the series is:
0, width_inc, 2*width_inc, ...
You're suggesting this:
1, 1+width_inc, 1+2*width_inc, ...
> That way the server wouldn't be stepping over
> the clients default of 0 by default. (zero seems to be a request to fix the
> surface size to min_width, min_height)
And I don't follow this.
Alexandros Frantzis (afrantzis) wrote : | # |
122+ auto const error = new_size.
123+
124+ if (error > 0)
125+ {
126+ if (new_size.
127+ {
128+ new_size.width = new_size.width + DeltaX(
129+ }
130+ else
131+ {
132+ new_size.height = new_size.height - DeltaY(
133+ }
134+ }
Some questions (Aw=aspect W, Ah=aspect H):
1. In this case, since W/H < Aw/Ah, we can either increase W or decrease H to increase the ratio, as is done in the two branches of the if statement. What's the rationale behind using the relation between W and H to decide whether to change W or H?
2. What's the logic behind the formula in DeltaX (and DeltaY respectively)? If we want W/H to reach Aw/Ah we could increase W by error/Ah, so it's not clear why we need the other terms (the proposed formula leads to an increase of error/Ah + 1 + 1/Ah).
It would be good to add a comment documenting how the proposed computations and decisions work.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:2523
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Kevin DuBois (kdub) wrote : | # |
> > 227+ * Defines an arithmetic progression of sizes starting with min_width
> (if
> > set, otherwise 0)
> > would a default of 1 be better?
>
> I don't follow. If min_width isn't set the series is:
>
> 0, width_inc, 2*width_inc, ...
>
> You're suggesting this:
>
> 1, 1+width_inc, 1+2*width_inc, ...
>
> > That way the server wouldn't be stepping over
> > the clients default of 0 by default. (zero seems to be a request to fix the
> > surface size to min_width, min_height)
>
> And I don't follow this.
So, I was suggesting that min_width is always set and gives the sequence:
min_width, min_width + 1*width_inc, min_width + 2*width_inc, etc
And, with that 1 would be a good default value, because then you get:
min_width, min_width+1, min_width+2, etc.
effectively, saying that the server can resize to any width and still fit within the client's setting.
If min_width is set to zero, then the client's sequence is:
min_width, min_width, min_width, etc...
and the client requests no server resizes.
Kevin DuBois (kdub) wrote : | # |
> > 264+ * \param [in] x numerator
> > 265+ * \param [in] y denominator
> > Its obvious from context that the ratio is x/y, but might make it clearer
> that
> > that the ratio is x/y (and also if the ratio gets affected by 90 degree
> > rotation requests)
>
> I'm not clear what this is asking for.
Just some more comments in the vein of:
261+ * Set the minimum aspect ratio
to
* Set the minimum aspect ratio. Aspect ratio is defined as the ratio of the x direction over
* the y direction. Aspect ratio is not inverted/affected by mir_surface_
(or how the ratio is treated if its rotated 90/270 degrees)
Alan Griffiths (alan-griffiths) wrote : | # |
> Some questions (Aw=aspect W, Ah=aspect H):
>
> 1. In this case, since W/H < Aw/Ah, we can either increase W or decrease H to
> increase the ratio, as is done in the two branches of the if statement. What's
> the rationale behind using the relation between W and H to decide whether to
> change W or H?
I admit it is arbitrary. And now I examine it, probably wrong.
My (misguided) thought was that changing the smaller one leads to a bigger effect on the ratio for a smaller change. But suppose we want a ratio of 10:1, but have 10:20. Then my approach leads to 200:20 when 10:2 would probably be better.
> 2. What's the logic behind the formula in DeltaX (and DeltaY respectively)? If
> we want W/H to reach Aw/Ah we could increase W by error/Ah, so it's not clear
> why we need the other terms (the proposed formula leads to an increase of
> error/Ah + 1 + 1/Ah).
It's to ensure rounding up. (I didn't think common incantation in integer arithmetic needed explaining - I was wrong.)
> It would be good to add a comment documenting how the proposed computations
> and decisions work.
And even better to get it right. ;)
Alexandros Frantzis (afrantzis) wrote : | # |
> It's to ensure rounding up. (I didn't think common incantation in integer arithmetic needed explaining - I was wrong.)
Yes, this will benefit from being more explicit (I missed it even though I have used/implemented it in the past [1]).
[1] src/platforms/
Alan Griffiths (alan-griffiths) wrote : | # |
> > > 227+ * Defines an arithmetic progression of sizes starting with min_width
> > (if
> > > set, otherwise 0)
> > > would a default of 1 be better?
> >
> > I don't follow. If min_width isn't set the series is:
> >
> > 0, width_inc, 2*width_inc, ...
> >
> > You're suggesting this:
> >
> > 1, 1+width_inc, 1+2*width_inc, ...
> >
> > > That way the server wouldn't be stepping over
> > > the clients default of 0 by default. (zero seems to be a request to fix
> the
> > > surface size to min_width, min_height)
> >
> > And I don't follow this.
>
> So, I was suggesting that min_width is always set and gives the sequence:
> min_width, min_width + 1*width_inc, min_width + 2*width_inc, etc
> And, with that 1 would be a good default value, because then you get:
> min_width, min_width+1, min_width+2, etc.
OK, I think you're writing "*min_width* is always set" when you're suggesting a default for *width_inc*.
So, we're actually talking about the same behavior except, possibly, in cases where the client applies a spec to a surface that has previously had width_inc set:
What we have is a surface spec where width_inc is optional and only if that is set by the client will it update the width_inc on the server.
Are you suggesting that the surface spec should always have a width_inc (default 1) and that this should override the server setting if not set by the client? I don't think that is right.
Alan Griffiths (alan-griffiths) wrote : | # |
> And even better to get it right. ;)
fixed
Alan Griffiths (alan-griffiths) : | # |
Kevin DuBois (kdub) wrote : | # |
cleared up my confusion over irc (re the width_inc, height_inc), lgtm
Alexandros Frantzis (afrantzis) wrote : | # |
Looks good.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:2526
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Daniel van Vugt (vanvugt) wrote : | # |
(1) Still not a fan of the extraneous functions and longer names. And "inc" is better than "increment" because the former is actually more common in programming circles and less likely to take you off the edge of the screen when typing.
(2) Does anyone know why having two aspect ratios is useful? I know X has it, but why? In my experience in building toolkits only two options (one ratio) are useful:
(a) Fixed aspect ratio (e.g. video player); or
(b) No fixed aspect ratio (although the toolkit will feed back and modify the min/max dimensions according to how well its widgets fit, that's not an aspect thing).
Alan Griffiths (alan-griffiths) wrote : | # |
> (1) Still not a fan of the extraneous functions and longer names. And "inc" is
> better than "increment" because the former is actually more common in
> programming circles and less likely to take you off the edge of the screen
> when typing.
"Do not use abbreviations unless they are extremely well known outside your project"
http://
It is a fine line - "min" and "max" have been accepted as common abbreviations whereas "inc" isn't common enough.
> (2) Does anyone know why having two aspect ratios is useful? I know X has it,
> but why? In my experience in building toolkits only two options (one ratio)
> are useful:
> (a) Fixed aspect ratio (e.g. video player); or
> (b) No fixed aspect ratio (although the toolkit will feed back and modify
> the min/max dimensions according to how well its widgets fit, that's not an
> aspect thing).
No, but following established practice in the domain seems prudent.
Preview Diff
1 | === modified file 'examples/server_example_canonical_window_manager.cpp' |
2 | --- examples/server_example_canonical_window_manager.cpp 2015-04-23 07:11:04 +0000 |
3 | +++ examples/server_example_canonical_window_manager.cpp 2015-04-28 16:01:14 +0000 |
4 | @@ -63,11 +63,15 @@ |
5 | state{mir_surface_state_restored}, |
6 | restore_rect{surface->top_left(), surface->size()}, |
7 | session{session}, |
8 | - parent{surface->parent()}, |
9 | + parent{params.parent}, |
10 | min_width{params.min_width}, |
11 | min_height{params.min_height}, |
12 | max_width{params.max_width}, |
13 | - max_height{params.max_height} |
14 | + max_height{params.max_height}, |
15 | + width_inc{params.width_inc}, |
16 | + height_inc{params.height_inc}, |
17 | + min_aspect{params.min_aspect}, |
18 | + max_aspect{params.max_aspect} |
19 | { |
20 | } |
21 | |
22 | @@ -323,21 +327,25 @@ |
23 | { |
24 | auto& surface_info = tools->info_for(surface); |
25 | |
26 | + #define COPY_IF_SET(field)\ |
27 | + if (modifications.field.is_set())\ |
28 | + surface_info.field = modifications.field |
29 | + |
30 | + COPY_IF_SET(min_width); |
31 | + COPY_IF_SET(min_height); |
32 | + COPY_IF_SET(max_width); |
33 | + COPY_IF_SET(max_height); |
34 | + COPY_IF_SET(min_width); |
35 | + COPY_IF_SET(width_inc); |
36 | + COPY_IF_SET(height_inc); |
37 | + COPY_IF_SET(min_aspect); |
38 | + COPY_IF_SET(max_aspect); |
39 | + |
40 | + #undef COPY_IF_SET |
41 | + |
42 | if (modifications.name.is_set()) |
43 | surface->rename(modifications.name.value()); |
44 | |
45 | - if (modifications.min_width.is_set()) |
46 | - surface_info.min_width = modifications.min_width; |
47 | - |
48 | - if (modifications.min_height.is_set()) |
49 | - surface_info.min_height = modifications.min_height; |
50 | - |
51 | - if (modifications.max_width.is_set()) |
52 | - surface_info.max_width = modifications.max_width; |
53 | - |
54 | - if (modifications.max_height.is_set()) |
55 | - surface_info.max_height = modifications.max_height; |
56 | - |
57 | if (modifications.width.is_set() || modifications.height.is_set()) |
58 | { |
59 | auto new_size = surface->size(); |
60 | @@ -710,53 +718,103 @@ |
61 | |
62 | bool me::CanonicalWindowManagerPolicyCopy::constrained_resize( |
63 | std::shared_ptr<ms::Surface> const& surface, |
64 | - Point new_pos, |
65 | - Size new_size, |
66 | + Point const& requested_pos, |
67 | + Size const& requested_size, |
68 | bool const left_resize, |
69 | bool const top_resize, |
70 | Rectangle const& bounds) |
71 | { |
72 | auto const& surface_info = tools->info_for(surface); |
73 | |
74 | - if (surface_info.min_width.is_set() && surface_info.min_width.value() > new_size.width) |
75 | - { |
76 | - if (left_resize) |
77 | - { |
78 | - new_pos.x += surface_info.min_width.value() - new_size.width; |
79 | - } |
80 | - |
81 | - new_size.width = surface_info.min_width.value(); |
82 | - } |
83 | - |
84 | - if (surface_info.min_height.is_set() && surface_info.min_height.value() > new_size.height) |
85 | - { |
86 | - if (top_resize) |
87 | - { |
88 | - new_pos.y += surface_info.min_height.value() - new_size.height; |
89 | - } |
90 | - |
91 | - new_size.height = surface_info.min_height.value(); |
92 | - } |
93 | - |
94 | - if (surface_info.max_width.is_set() && surface_info.max_width.value() < new_size.width) |
95 | - { |
96 | - if (left_resize) |
97 | - { |
98 | - new_pos.x += surface_info.max_width.value() - new_size.width; |
99 | - } |
100 | - |
101 | - new_size.width = surface_info.max_width.value(); |
102 | - } |
103 | - |
104 | - if (surface_info.max_height.is_set() && surface_info.max_height.value() < new_size.height) |
105 | - { |
106 | - if (top_resize) |
107 | - { |
108 | - new_pos.y += surface_info.max_height.value() - new_size.height; |
109 | - } |
110 | - |
111 | - new_size.height = surface_info.max_height.value(); |
112 | - } |
113 | + auto const min_width = surface_info.min_width.is_set() ? surface_info.min_width.value() : Width{}; |
114 | + auto const min_height = surface_info.min_height.is_set() ? surface_info.min_height.value() : Height{}; |
115 | + auto const max_width = surface_info.max_width.is_set() ? |
116 | + surface_info.max_width.value() : Width{std::numeric_limits<int>::max()}; |
117 | + auto const max_height = surface_info.max_height.is_set() ? |
118 | + surface_info.max_height.value() : Height{std::numeric_limits<int>::max()}; |
119 | + |
120 | + Point new_pos = requested_pos; |
121 | + Size new_size = requested_size; |
122 | + |
123 | + if (surface_info.min_aspect.is_set()) |
124 | + { |
125 | + auto const ar = surface_info.min_aspect.value(); |
126 | + |
127 | + auto const error = new_size.height.as_int()*long(ar.width) - new_size.width.as_int()*long(ar.height); |
128 | + |
129 | + if (error > 0) |
130 | + { |
131 | + // Add (denominator-1) to numerator to ensure rounding up |
132 | + auto const width_correction = (error+(ar.height-1))/ar.height; |
133 | + auto const height_correction = (error+(ar.width-1))/ar.width; |
134 | + |
135 | + if (width_correction < height_correction) |
136 | + { |
137 | + new_size.width = new_size.width + DeltaX(width_correction); |
138 | + } |
139 | + else |
140 | + { |
141 | + new_size.height = new_size.height - DeltaY(height_correction); |
142 | + } |
143 | + } |
144 | + } |
145 | + |
146 | + if (surface_info.max_aspect.is_set()) |
147 | + { |
148 | + auto const ar = surface_info.max_aspect.value(); |
149 | + |
150 | + auto const error = new_size.width.as_int()*long(ar.height) - new_size.height.as_int()*long(ar.width); |
151 | + |
152 | + if (error > 0) |
153 | + { |
154 | + // Add (denominator-1) to numerator to ensure rounding up |
155 | + auto const height_correction = (error+(ar.width-1))/ar.width; |
156 | + auto const width_correction = (error+(ar.height-1))/ar.height; |
157 | + |
158 | + if (width_correction < height_correction) |
159 | + { |
160 | + new_size.width = new_size.width - DeltaX(width_correction); |
161 | + } |
162 | + else |
163 | + { |
164 | + new_size.height = new_size.height + DeltaY(height_correction); |
165 | + } |
166 | + } |
167 | + } |
168 | + |
169 | + if (min_width > new_size.width) |
170 | + new_size.width = min_width; |
171 | + |
172 | + if (min_height > new_size.height) |
173 | + new_size.height = min_height; |
174 | + |
175 | + if (max_width < new_size.width) |
176 | + new_size.width = max_width; |
177 | + |
178 | + if (max_height < new_size.height) |
179 | + new_size.height = max_height; |
180 | + |
181 | + if (surface_info.width_inc.is_set()) |
182 | + { |
183 | + auto const width = new_size.width.as_int() - min_width.as_int(); |
184 | + auto inc = surface_info.width_inc.value().as_int(); |
185 | + if (width % inc) |
186 | + new_size.width = min_width + DeltaX{inc*(((2L*width + inc)/2)/inc)}; |
187 | + } |
188 | + |
189 | + if (surface_info.height_inc.is_set()) |
190 | + { |
191 | + auto const height = new_size.height.as_int() - min_height.as_int(); |
192 | + auto inc = surface_info.height_inc.value().as_int(); |
193 | + if (height % inc) |
194 | + new_size.height = min_height + DeltaY{inc*(((2L*height + inc)/2)/inc)}; |
195 | + } |
196 | + |
197 | + if (left_resize) |
198 | + new_pos.x += new_size.width - requested_size.width; |
199 | + |
200 | + if (top_resize) |
201 | + new_pos.y += new_size.height - requested_size.height; |
202 | |
203 | if (left_resize) |
204 | { |
205 | @@ -788,8 +846,7 @@ |
206 | new_size.height = new_size.height + to_bottom_right.dy; |
207 | } |
208 | |
209 | - auto& info = tools->info_for(surface); |
210 | - switch (info.state) |
211 | + switch (surface_info.state) |
212 | { |
213 | case mir_surface_state_restored: |
214 | break; |
215 | @@ -816,7 +873,7 @@ |
216 | return true; |
217 | } |
218 | |
219 | - info.titlebar->resize({new_size.width, Height{title_bar_height}}); |
220 | + surface_info.titlebar->resize({new_size.width, Height{title_bar_height}}); |
221 | surface->resize(new_size); |
222 | |
223 | // TODO It is rather simplistic to move a tree WRT the top_left of the root |
224 | |
225 | === modified file 'examples/server_example_canonical_window_manager.h' |
226 | --- examples/server_example_canonical_window_manager.h 2015-04-17 14:31:42 +0000 |
227 | +++ examples/server_example_canonical_window_manager.h 2015-04-28 16:01:14 +0000 |
228 | @@ -54,6 +54,10 @@ |
229 | optional_value<geometry::Height> min_height; |
230 | optional_value<geometry::Width> max_width; |
231 | optional_value<geometry::Height> max_height; |
232 | + mir::optional_value<geometry::DeltaX> width_inc; |
233 | + mir::optional_value<geometry::DeltaY> height_inc; |
234 | + mir::optional_value<shell::SurfaceAspectRatio> min_aspect; |
235 | + mir::optional_value<shell::SurfaceAspectRatio> max_aspect; |
236 | }; |
237 | |
238 | // standard window management algorithm: |
239 | @@ -132,8 +136,8 @@ |
240 | void raise_tree(std::shared_ptr<scene::Surface> const& root) const; |
241 | bool constrained_resize( |
242 | std::shared_ptr<scene::Surface> const& surface, |
243 | - geometry::Point new_pos, |
244 | - geometry::Size new_size, |
245 | + geometry::Point const& requested_pos, |
246 | + geometry::Size const& requested_size, |
247 | const bool left_resize, |
248 | const bool top_resize, |
249 | geometry::Rectangle const& bounds); |
250 | |
251 | === modified file 'include/client/mir_toolkit/mir_surface.h' |
252 | --- include/client/mir_toolkit/mir_surface.h 2015-04-28 10:29:48 +0000 |
253 | +++ include/client/mir_toolkit/mir_surface.h 2015-04-28 16:01:14 +0000 |
254 | @@ -239,6 +239,32 @@ |
255 | bool mir_surface_spec_set_height(MirSurfaceSpec* spec, unsigned height); |
256 | |
257 | /** |
258 | + * Set the requested width increment, in pixels. |
259 | + * Defines an arithmetic progression of sizes starting with min_width (if set, otherwise 0) |
260 | + * into which the surface prefers to be resized. |
261 | + * |
262 | + * \param [in] spec Specification to mutate |
263 | + * \param [in] width_inc Requested width increment. |
264 | + * \return False if width increment is invalid for a surface of this type |
265 | + * \note The requested dimensions are a hint only. The server is not guaranteed to |
266 | + * create a surface of any specific width or height. |
267 | + */ |
268 | +bool mir_surface_spec_set_width_increment(MirSurfaceSpec *spec, unsigned width_inc); |
269 | + |
270 | +/** |
271 | + * Set the requested height increment, in pixels |
272 | + * Defines an arithmetic progression of sizes starting with min_height (if set, otherwise 0) |
273 | + * into which the surface prefers to be resized. |
274 | + * |
275 | + * \param [in] spec Specification to mutate |
276 | + * \param [in] height_inc Requested height increment. |
277 | + * \return False if height increment is invalid for a surface of this type |
278 | + * \note The requested dimensions are a hint only. The server is not guaranteed to |
279 | + * create a surface of any specific width or height. |
280 | + */ |
281 | +bool mir_surface_spec_set_height_increment(MirSurfaceSpec *spec, unsigned height_inc); |
282 | + |
283 | +/** |
284 | * Set the minimum width, in pixels |
285 | * |
286 | * \param [in] spec Specification to mutate |
287 | @@ -280,6 +306,33 @@ |
288 | * surface of any specific width or height. |
289 | */ |
290 | bool mir_surface_spec_set_max_height(MirSurfaceSpec* spec, unsigned max_height); |
291 | + |
292 | +/** |
293 | + * Set the minimum aspect ratio. This is the minimum ratio of surface width to height. |
294 | + * It is independent of orientation changes and/or preferences. |
295 | + * |
296 | + * \param [in] spec Specification to mutate |
297 | + * \param [in] width numerator |
298 | + * \param [in] height denominator |
299 | + * \return False if minimum aspect is invalid for a surface of this type |
300 | + * \note The requested aspect ratio is a hint only. The server is not guaranteed |
301 | + * to create a surface of any specific aspect. |
302 | + */ |
303 | +bool mir_surface_spec_set_min_aspect_ratio(MirSurfaceSpec* spec, unsigned width, unsigned height); |
304 | + |
305 | +/** |
306 | + * Set the maximum aspect ratio. This is the maximum ratio of surface width to height. |
307 | + * It is independent of orientation changes and/or preferences. |
308 | + * |
309 | + * \param [in] spec Specification to mutate |
310 | + * \param [in] width numerator |
311 | + * \param [in] height denominator |
312 | + * \return False if maximum aspect is invalid for a surface of this type |
313 | + * \note The requested aspect ratio is a hint only. The server is not guaranteed |
314 | + * to create a surface of any specific aspect. |
315 | + */ |
316 | +bool mir_surface_spec_set_max_aspect_ratio(MirSurfaceSpec* spec, unsigned width, unsigned height); |
317 | + |
318 | /** |
319 | * Set the requested pixel format. |
320 | * \param [in] spec Specification to mutate |
321 | |
322 | === modified file 'include/server/mir/scene/surface_creation_parameters.h' |
323 | --- include/server/mir/scene/surface_creation_parameters.h 2015-04-16 18:50:58 +0000 |
324 | +++ include/server/mir/scene/surface_creation_parameters.h 2015-04-28 16:01:14 +0000 |
325 | @@ -28,6 +28,7 @@ |
326 | #include "mir/frontend/surface_id.h" |
327 | #include "mir/input/input_reception_mode.h" |
328 | #include "mir/optional_value.h" |
329 | +#include "mir/shell/surface_specification.h" |
330 | |
331 | #include <memory> |
332 | #include <string> |
333 | @@ -94,6 +95,10 @@ |
334 | optional_value<geometry::Height> min_height; |
335 | optional_value<geometry::Width> max_width; |
336 | optional_value<geometry::Height> max_height; |
337 | + mir::optional_value<geometry::DeltaX> width_inc; |
338 | + mir::optional_value<geometry::DeltaY> height_inc; |
339 | + mir::optional_value<shell::SurfaceAspectRatio> min_aspect; |
340 | + mir::optional_value<shell::SurfaceAspectRatio> max_aspect; |
341 | }; |
342 | |
343 | bool operator==(const SurfaceCreationParameters& lhs, const SurfaceCreationParameters& rhs); |
344 | |
345 | === modified file 'include/server/mir/shell/surface_specification.h' |
346 | --- include/server/mir/shell/surface_specification.h 2015-04-10 08:20:39 +0000 |
347 | +++ include/server/mir/shell/surface_specification.h 2015-04-28 16:01:14 +0000 |
348 | @@ -33,6 +33,8 @@ |
349 | { |
350 | namespace shell |
351 | { |
352 | +struct SurfaceAspectRatio { unsigned width; unsigned height; }; |
353 | + |
354 | /// Specification of surface properties requested by client |
355 | struct SurfaceSpecification |
356 | { |
357 | @@ -52,6 +54,10 @@ |
358 | optional_value<geometry::Height> min_height; |
359 | optional_value<geometry::Width> max_width; |
360 | optional_value<geometry::Height> max_height; |
361 | + mir::optional_value<geometry::DeltaX> width_inc; |
362 | + mir::optional_value<geometry::DeltaY> height_inc; |
363 | + mir::optional_value<SurfaceAspectRatio> min_aspect; |
364 | + mir::optional_value<SurfaceAspectRatio> max_aspect; |
365 | |
366 | // TODO scene::SurfaceCreationParameters overlaps this content but has additional fields: |
367 | // geometry::Point top_left; |
368 | |
369 | === modified file 'src/client/mir_surface.cpp' |
370 | --- src/client/mir_surface.cpp 2015-04-23 07:11:04 +0000 |
371 | +++ src/client/mir_surface.cpp 2015-04-28 16:01:14 +0000 |
372 | @@ -96,6 +96,10 @@ |
373 | SERIALIZE_OPTION_IF_SET(min_height, message); |
374 | SERIALIZE_OPTION_IF_SET(max_width, message); |
375 | SERIALIZE_OPTION_IF_SET(max_height, message); |
376 | + SERIALIZE_OPTION_IF_SET(width_inc, message); |
377 | + SERIALIZE_OPTION_IF_SET(height_inc, message); |
378 | + // min_aspect is a special case (below) |
379 | + // max_aspect is a special case (below) |
380 | |
381 | if (parent.is_set() && parent.value() != nullptr) |
382 | message.set_parent_id(parent.value()->id()); |
383 | @@ -108,6 +112,18 @@ |
384 | message.mutable_aux_rect()->set_height(aux_rect.value().height); |
385 | } |
386 | |
387 | + if (min_aspect.is_set()) |
388 | + { |
389 | + message.mutable_min_aspect()->set_width(min_aspect.value().width); |
390 | + message.mutable_min_aspect()->set_height(min_aspect.value().height); |
391 | + } |
392 | + |
393 | + if (max_aspect.is_set()) |
394 | + { |
395 | + message.mutable_max_aspect()->set_width(max_aspect.value().width); |
396 | + message.mutable_max_aspect()->set_height(max_aspect.value().height); |
397 | + } |
398 | + |
399 | return message; |
400 | } |
401 | |
402 | @@ -563,6 +579,10 @@ |
403 | COPY_IF_SET(min_height); |
404 | COPY_IF_SET(max_width); |
405 | COPY_IF_SET(max_height); |
406 | + COPY_IF_SET(width_inc); |
407 | + COPY_IF_SET(height_inc); |
408 | + // min_aspect is a special case (below) |
409 | + // max_aspect is a special case (below) |
410 | #undef COPY_IF_SET |
411 | |
412 | if (spec.surface_name.is_set()) |
413 | @@ -584,6 +604,20 @@ |
414 | rect->set_height(value.height); |
415 | } |
416 | |
417 | + if (spec.min_aspect.is_set()) |
418 | + { |
419 | + auto const aspect = surface_specification->mutable_min_aspect(); |
420 | + aspect->set_width(spec.min_aspect.value().width); |
421 | + aspect->set_height(spec.min_aspect.value().height); |
422 | + } |
423 | + |
424 | + if (spec.max_aspect.is_set()) |
425 | + { |
426 | + auto const aspect = surface_specification->mutable_max_aspect(); |
427 | + aspect->set_width(spec.max_aspect.value().width); |
428 | + aspect->set_height(spec.max_aspect.value().height); |
429 | + } |
430 | + |
431 | modify_wait_handle.expect_result(); |
432 | server->modify_surface(0, &mods, &modify_result, |
433 | google::protobuf::NewCallback(this, &MirSurface::on_modified)); |
434 | |
435 | === modified file 'src/client/mir_surface.h' |
436 | --- src/client/mir_surface.h 2015-04-09 15:59:52 +0000 |
437 | +++ src/client/mir_surface.h 2015-04-28 16:01:14 +0000 |
438 | @@ -67,6 +67,8 @@ |
439 | |
440 | mir::protobuf::SurfaceParameters serialize() const; |
441 | |
442 | + struct AspectRatio { unsigned width; unsigned height; }; |
443 | + |
444 | // Required parameters |
445 | MirConnection* connection{nullptr}; |
446 | |
447 | @@ -91,6 +93,10 @@ |
448 | mir::optional_value<int> min_height; |
449 | mir::optional_value<int> max_width; |
450 | mir::optional_value<int> max_height; |
451 | + mir::optional_value<int> width_inc; |
452 | + mir::optional_value<int> height_inc; |
453 | + mir::optional_value<AspectRatio> min_aspect; |
454 | + mir::optional_value<AspectRatio> max_aspect; |
455 | }; |
456 | |
457 | struct MirSurface |
458 | |
459 | === modified file 'src/client/mir_surface_api.cpp' |
460 | --- src/client/mir_surface_api.cpp 2015-04-17 14:33:09 +0000 |
461 | +++ src/client/mir_surface_api.cpp 2015-04-28 16:01:14 +0000 |
462 | @@ -632,3 +632,51 @@ |
463 | MIR_LOG_UNCAUGHT_EXCEPTION(ex); |
464 | // Keep calm and carry on |
465 | } |
466 | + |
467 | +bool mir_surface_spec_set_width_increment(MirSurfaceSpec *spec, unsigned width_inc) |
468 | +try |
469 | +{ |
470 | + spec->width_inc = width_inc; |
471 | + return true; |
472 | +} |
473 | +catch (std::exception const& ex) |
474 | +{ |
475 | + MIR_LOG_UNCAUGHT_EXCEPTION(ex); |
476 | + return false; |
477 | +} |
478 | + |
479 | +bool mir_surface_spec_set_height_increment(MirSurfaceSpec *spec, unsigned height_inc) |
480 | +try |
481 | +{ |
482 | + spec->height_inc = height_inc; |
483 | + return true; |
484 | +} |
485 | +catch (std::exception const& ex) |
486 | +{ |
487 | + MIR_LOG_UNCAUGHT_EXCEPTION(ex); |
488 | + return false; |
489 | +} |
490 | + |
491 | +bool mir_surface_spec_set_min_aspect_ratio(MirSurfaceSpec* spec, unsigned width, unsigned height) |
492 | +try |
493 | +{ |
494 | + spec->min_aspect = {width, height}; |
495 | + return true; |
496 | +} |
497 | +catch (std::exception const& ex) |
498 | +{ |
499 | + MIR_LOG_UNCAUGHT_EXCEPTION(ex); |
500 | + return false; |
501 | +} |
502 | + |
503 | +bool mir_surface_spec_set_max_aspect_ratio(MirSurfaceSpec* spec, unsigned width, unsigned height) |
504 | +try |
505 | +{ |
506 | + spec->max_aspect = {width, height}; |
507 | + return true; |
508 | +} |
509 | +catch (std::exception const& ex) |
510 | +{ |
511 | + MIR_LOG_UNCAUGHT_EXCEPTION(ex); |
512 | + return false; |
513 | +} |
514 | |
515 | === modified file 'src/client/symbols.map' |
516 | --- src/client/symbols.map 2015-04-23 07:11:04 +0000 |
517 | +++ src/client/symbols.map 2015-04-28 16:01:14 +0000 |
518 | @@ -163,10 +163,14 @@ |
519 | mir_surface_event_get_attribute; |
520 | mir_surface_event_get_attribute_value; |
521 | mir_surface_set_swapinterval; |
522 | + mir_surface_spec_set_height_increment; |
523 | + mir_surface_spec_set_max_aspect_ratio; |
524 | + mir_surface_spec_set_max_width; |
525 | + mir_surface_spec_set_max_height; |
526 | + mir_surface_spec_set_min_aspect_ratio; |
527 | mir_surface_spec_set_min_width; |
528 | mir_surface_spec_set_min_height; |
529 | - mir_surface_spec_set_max_width; |
530 | - mir_surface_spec_set_max_height; |
531 | + mir_surface_spec_set_width_increment; |
532 | mir_surface_swap_buffers; |
533 | mir_surface_swap_buffers_sync; |
534 | mir_touch_event_modifiers; |
535 | |
536 | === modified file 'src/protobuf/mir_protobuf.proto' |
537 | --- src/protobuf/mir_protobuf.proto 2015-04-23 07:11:04 +0000 |
538 | +++ src/protobuf/mir_protobuf.proto 2015-04-28 16:01:14 +0000 |
539 | @@ -31,6 +31,16 @@ |
540 | optional int32 min_height = 14; |
541 | optional int32 max_width = 15; |
542 | optional int32 max_height = 16; |
543 | + optional int32 width_inc = 17; |
544 | + optional int32 height_inc = 18; |
545 | + optional SurfaceAspectRatio min_aspect = 19; |
546 | + optional SurfaceAspectRatio max_aspect = 20; |
547 | +} |
548 | + |
549 | +message SurfaceAspectRatio |
550 | +{ |
551 | + required uint32 width = 1; |
552 | + required uint32 height = 2; |
553 | } |
554 | |
555 | // If and when we break our protocol backward-compatibility, this could be |
556 | @@ -53,6 +63,10 @@ |
557 | optional int32 min_height = 14; |
558 | optional int32 max_width = 15; |
559 | optional int32 max_height = 16; |
560 | + optional int32 width_inc = 17; |
561 | + optional int32 height_inc = 18; |
562 | + optional SurfaceAspectRatio min_aspect = 19; |
563 | + optional SurfaceAspectRatio max_aspect = 20; |
564 | } |
565 | |
566 | |
567 | |
568 | === modified file 'src/server/frontend/session_mediator.cpp' |
569 | --- src/server/frontend/session_mediator.cpp 2015-04-28 10:29:48 +0000 |
570 | +++ src/server/frontend/session_mediator.cpp 2015-04-28 16:01:14 +0000 |
571 | @@ -238,9 +238,17 @@ |
572 | COPY_IF_SET(min_height); |
573 | COPY_IF_SET(max_width); |
574 | COPY_IF_SET(max_height); |
575 | + COPY_IF_SET(width_inc); |
576 | + COPY_IF_SET(height_inc); |
577 | |
578 | #undef COPY_IF_SET |
579 | |
580 | + if (request->has_min_aspect()) |
581 | + params.min_aspect = { request->min_aspect().width(), request->min_aspect().height()}; |
582 | + |
583 | + if (request->has_max_aspect()) |
584 | + params.max_aspect = { request->max_aspect().width(), request->max_aspect().height()}; |
585 | + |
586 | auto const surf_id = shell->create_surface(session, params); |
587 | |
588 | auto surface = session->get_surface(surf_id); |
589 | @@ -461,6 +469,10 @@ |
590 | COPY_IF_SET(min_height); |
591 | COPY_IF_SET(max_width); |
592 | COPY_IF_SET(max_height); |
593 | + COPY_IF_SET(width_inc); |
594 | + COPY_IF_SET(height_inc); |
595 | + // min_aspect is a special case (below) |
596 | + // max_aspect is a special case (below) |
597 | |
598 | #undef COPY_IF_SET |
599 | |
600 | @@ -470,6 +482,20 @@ |
601 | mods.aux_rect = {{rect.left(), rect.top()}, {rect.width(), rect.height()}}; |
602 | } |
603 | |
604 | + if (surface_specification.has_min_aspect()) |
605 | + mods.min_aspect = |
606 | + { |
607 | + surface_specification.min_aspect().width(), |
608 | + surface_specification.min_aspect().height() |
609 | + }; |
610 | + |
611 | + if (surface_specification.has_max_aspect()) |
612 | + mods.max_aspect = |
613 | + { |
614 | + surface_specification.max_aspect().width(), |
615 | + surface_specification.max_aspect().height() |
616 | + }; |
617 | + |
618 | auto const id = mf::SurfaceId(request->surface_id().value()); |
619 | |
620 | shell->modify_surface(session, id, mods); |
621 | |
622 | === modified file 'src/server/shell/canonical_window_manager.cpp' |
623 | --- src/server/shell/canonical_window_manager.cpp 2015-04-23 07:11:04 +0000 |
624 | +++ src/server/shell/canonical_window_manager.cpp 2015-04-28 16:01:14 +0000 |
625 | @@ -27,6 +27,8 @@ |
626 | #include <linux/input.h> |
627 | #include <csignal> |
628 | |
629 | +#include <climits> |
630 | + |
631 | namespace msh = mir::shell; |
632 | namespace ms = mir::scene; |
633 | using namespace mir::geometry; |
634 | @@ -43,11 +45,15 @@ |
635 | state{mir_surface_state_restored}, |
636 | restore_rect{surface->top_left(), surface->size()}, |
637 | session{session}, |
638 | - parent{surface->parent()}, |
639 | + parent{params.parent}, |
640 | min_width{params.min_width}, |
641 | min_height{params.min_height}, |
642 | max_width{params.max_width}, |
643 | - max_height{params.max_height} |
644 | + max_height{params.max_height}, |
645 | + width_inc{params.width_inc}, |
646 | + height_inc{params.height_inc}, |
647 | + min_aspect{params.min_aspect}, |
648 | + max_aspect{params.max_aspect} |
649 | { |
650 | } |
651 | |
652 | @@ -256,21 +262,25 @@ |
653 | { |
654 | auto& surface_info = tools->info_for(surface); |
655 | |
656 | + #define COPY_IF_SET(field)\ |
657 | + if (modifications.field.is_set())\ |
658 | + surface_info.field = modifications.field |
659 | + |
660 | + COPY_IF_SET(min_width); |
661 | + COPY_IF_SET(min_height); |
662 | + COPY_IF_SET(max_width); |
663 | + COPY_IF_SET(max_height); |
664 | + COPY_IF_SET(min_width); |
665 | + COPY_IF_SET(width_inc); |
666 | + COPY_IF_SET(height_inc); |
667 | + COPY_IF_SET(min_aspect); |
668 | + COPY_IF_SET(max_aspect); |
669 | + |
670 | + #undef COPY_IF_SET |
671 | + |
672 | if (modifications.name.is_set()) |
673 | surface->rename(modifications.name.value()); |
674 | |
675 | - if (modifications.min_width.is_set()) |
676 | - surface_info.min_width = modifications.min_width; |
677 | - |
678 | - if (modifications.min_height.is_set()) |
679 | - surface_info.min_height = modifications.min_height; |
680 | - |
681 | - if (modifications.max_width.is_set()) |
682 | - surface_info.max_width = modifications.max_width; |
683 | - |
684 | - if (modifications.max_height.is_set()) |
685 | - surface_info.max_height = modifications.max_height; |
686 | - |
687 | if (modifications.width.is_set() || modifications.height.is_set()) |
688 | { |
689 | auto new_size = surface->size(); |
690 | @@ -623,53 +633,103 @@ |
691 | |
692 | bool msh::CanonicalWindowManagerPolicy::constrained_resize( |
693 | std::shared_ptr<ms::Surface> const& surface, |
694 | - Point new_pos, |
695 | - Size new_size, |
696 | + Point const& requested_pos, |
697 | + Size const& requested_size, |
698 | bool const left_resize, |
699 | bool const top_resize, |
700 | Rectangle const& bounds) |
701 | { |
702 | auto const& surface_info = tools->info_for(surface); |
703 | |
704 | - if (surface_info.min_width.is_set() && surface_info.min_width.value() > new_size.width) |
705 | - { |
706 | - if (left_resize) |
707 | - { |
708 | - new_pos.x += surface_info.min_width.value() - new_size.width; |
709 | - } |
710 | - |
711 | - new_size.width = surface_info.min_width.value(); |
712 | - } |
713 | - |
714 | - if (surface_info.min_height.is_set() && surface_info.min_height.value() > new_size.height) |
715 | - { |
716 | - if (top_resize) |
717 | - { |
718 | - new_pos.y += surface_info.min_height.value() - new_size.height; |
719 | - } |
720 | - |
721 | - new_size.height = surface_info.min_height.value(); |
722 | - } |
723 | - |
724 | - if (surface_info.max_width.is_set() && surface_info.max_width.value() < new_size.width) |
725 | - { |
726 | - if (left_resize) |
727 | - { |
728 | - new_pos.x += surface_info.max_width.value() - new_size.width; |
729 | - } |
730 | - |
731 | - new_size.width = surface_info.max_width.value(); |
732 | - } |
733 | - |
734 | - if (surface_info.max_height.is_set() && surface_info.max_height.value() < new_size.height) |
735 | - { |
736 | - if (top_resize) |
737 | - { |
738 | - new_pos.y += surface_info.max_height.value() - new_size.height; |
739 | - } |
740 | - |
741 | - new_size.height = surface_info.max_height.value(); |
742 | - } |
743 | + auto const min_width = surface_info.min_width.is_set() ? surface_info.min_width.value() : Width{}; |
744 | + auto const min_height = surface_info.min_height.is_set() ? surface_info.min_height.value() : Height{}; |
745 | + auto const max_width = surface_info.max_width.is_set() ? |
746 | + surface_info.max_width.value() : Width{std::numeric_limits<int>::max()}; |
747 | + auto const max_height = surface_info.max_height.is_set() ? |
748 | + surface_info.max_height.value() : Height{std::numeric_limits<int>::max()}; |
749 | + |
750 | + Point new_pos = requested_pos; |
751 | + Size new_size = requested_size; |
752 | + |
753 | + if (surface_info.min_aspect.is_set()) |
754 | + { |
755 | + auto const ar = surface_info.min_aspect.value(); |
756 | + |
757 | + auto const error = new_size.height.as_int()*long(ar.width) - new_size.width.as_int()*long(ar.height); |
758 | + |
759 | + if (error > 0) |
760 | + { |
761 | + // Add (denominator-1) to numerator to ensure rounding up |
762 | + auto const width_correction = (error+(ar.height-1))/ar.height; |
763 | + auto const height_correction = (error+(ar.width-1))/ar.width; |
764 | + |
765 | + if (width_correction < height_correction) |
766 | + { |
767 | + new_size.width = new_size.width + DeltaX(width_correction); |
768 | + } |
769 | + else |
770 | + { |
771 | + new_size.height = new_size.height - DeltaY(height_correction); |
772 | + } |
773 | + } |
774 | + } |
775 | + |
776 | + if (surface_info.max_aspect.is_set()) |
777 | + { |
778 | + auto const ar = surface_info.max_aspect.value(); |
779 | + |
780 | + auto const error = new_size.width.as_int()*long(ar.height) - new_size.height.as_int()*long(ar.width); |
781 | + |
782 | + if (error > 0) |
783 | + { |
784 | + // Add (denominator-1) to numerator to ensure rounding up |
785 | + auto const height_correction = (error+(ar.width-1))/ar.width; |
786 | + auto const width_correction = (error+(ar.height-1))/ar.height; |
787 | + |
788 | + if (width_correction < height_correction) |
789 | + { |
790 | + new_size.width = new_size.width - DeltaX(width_correction); |
791 | + } |
792 | + else |
793 | + { |
794 | + new_size.height = new_size.height + DeltaY(height_correction); |
795 | + } |
796 | + } |
797 | + } |
798 | + |
799 | + if (min_width > new_size.width) |
800 | + new_size.width = min_width; |
801 | + |
802 | + if (min_height > new_size.height) |
803 | + new_size.height = min_height; |
804 | + |
805 | + if (max_width < new_size.width) |
806 | + new_size.width = max_width; |
807 | + |
808 | + if (max_height < new_size.height) |
809 | + new_size.height = max_height; |
810 | + |
811 | + if (surface_info.width_inc.is_set()) |
812 | + { |
813 | + auto const width = new_size.width.as_int() - min_width.as_int(); |
814 | + auto inc = surface_info.width_inc.value().as_int(); |
815 | + if (width % inc) |
816 | + new_size.width = min_width + DeltaX{inc*(((2L*width + inc)/2)/inc)}; |
817 | + } |
818 | + |
819 | + if (surface_info.height_inc.is_set()) |
820 | + { |
821 | + auto const height = new_size.height.as_int() - min_height.as_int(); |
822 | + auto inc = surface_info.height_inc.value().as_int(); |
823 | + if (height % inc) |
824 | + new_size.height = min_height + DeltaY{inc*(((2L*height + inc)/2)/inc)}; |
825 | + } |
826 | + |
827 | + if (left_resize) |
828 | + new_pos.x += new_size.width - requested_size.width; |
829 | + |
830 | + if (top_resize) |
831 | + new_pos.y += new_size.height - requested_size.height; |
832 | |
833 | if (left_resize) |
834 | { |
835 | |
836 | === modified file 'src/server/shell/canonical_window_manager.h' |
837 | --- src/server/shell/canonical_window_manager.h 2015-04-16 18:50:58 +0000 |
838 | +++ src/server/shell/canonical_window_manager.h 2015-04-28 16:01:14 +0000 |
839 | @@ -50,6 +50,10 @@ |
840 | optional_value<geometry::Height> min_height; |
841 | optional_value<geometry::Width> max_width; |
842 | optional_value<geometry::Height> max_height; |
843 | + mir::optional_value<geometry::DeltaX> width_inc; |
844 | + mir::optional_value<geometry::DeltaY> height_inc; |
845 | + mir::optional_value<SurfaceAspectRatio> min_aspect; |
846 | + mir::optional_value<SurfaceAspectRatio> max_aspect; |
847 | }; |
848 | |
849 | // standard window management algorithm: |
850 | @@ -126,8 +130,8 @@ |
851 | void raise_tree(std::shared_ptr<scene::Surface> const& root) const; |
852 | bool constrained_resize( |
853 | std::shared_ptr<scene::Surface> const& surface, |
854 | - geometry::Point new_pos, |
855 | - geometry::Size new_size, |
856 | + geometry::Point const& requested_pos, |
857 | + geometry::Size const& requested_size, |
858 | const bool left_resize, |
859 | const bool top_resize, |
860 | geometry::Rectangle const& bounds); |
861 | |
862 | === modified file 'tests/acceptance-tests/test_surface_modifications.cpp' |
863 | --- tests/acceptance-tests/test_surface_modifications.cpp 2015-04-23 07:11:04 +0000 |
864 | +++ tests/acceptance-tests/test_surface_modifications.cpp 2015-04-28 16:01:14 +0000 |
865 | @@ -137,15 +137,25 @@ |
866 | EXPECT_CALL(surface_observer, renamed(StrEq(new_title))). |
867 | WillOnce(InvokeWithoutArgs([&]{ server_ready.raise(); })); |
868 | |
869 | - auto const spec = mir_connection_create_spec_for_changes(connection); |
870 | - |
871 | - mir_surface_spec_set_name(spec, new_title); |
872 | - mir_surface_apply_spec(surface, spec); |
873 | - mir_surface_spec_release(spec); |
874 | + apply_changes([&](MirSurfaceSpec* spec) |
875 | + { |
876 | + mir_surface_spec_set_name(spec, new_title); |
877 | + }); |
878 | |
879 | server_ready.wait(); |
880 | } |
881 | |
882 | + template<typename Specifier> |
883 | + void apply_changes(Specifier const& specifier) const |
884 | + { |
885 | + auto const spec = mir_connection_create_spec_for_changes(connection); |
886 | + |
887 | + specifier(spec); |
888 | + |
889 | + mir_surface_apply_spec(surface, spec); |
890 | + mir_surface_spec_release(spec); |
891 | + } |
892 | + |
893 | MockSurfaceObserver surface_observer; |
894 | std::weak_ptr<ms::Surface> shell_surface; |
895 | }; |
896 | @@ -168,11 +178,10 @@ |
897 | |
898 | EXPECT_CALL(surface_observer, renamed(StrEq(new_title))); |
899 | |
900 | - auto const spec = mir_connection_create_spec_for_changes(connection); |
901 | - |
902 | - mir_surface_spec_set_name(spec, new_title); |
903 | - mir_surface_apply_spec(surface, spec); |
904 | - mir_surface_spec_release(spec); |
905 | + apply_changes([&](MirSurfaceSpec* spec) |
906 | + { |
907 | + mir_surface_spec_set_name(spec, new_title); |
908 | + }); |
909 | } |
910 | |
911 | TEST_F(SurfaceModifications, surface_spec_resize_is_notified) |
912 | @@ -182,13 +191,11 @@ |
913 | |
914 | EXPECT_CALL(surface_observer, resized_to(Size{new_width, new_height})); |
915 | |
916 | - auto const spec = mir_connection_create_spec_for_changes(connection); |
917 | - |
918 | - mir_surface_spec_set_width(spec, new_width); |
919 | - mir_surface_spec_set_height(spec, new_height); |
920 | - |
921 | - mir_surface_apply_spec(surface, spec); |
922 | - mir_surface_spec_release(spec); |
923 | + apply_changes([&](MirSurfaceSpec* spec) |
924 | + { |
925 | + mir_surface_spec_set_width(spec, new_width); |
926 | + mir_surface_spec_set_height(spec, new_height); |
927 | + }); |
928 | } |
929 | |
930 | TEST_F(SurfaceModifications, surface_spec_change_width_is_notified) |
931 | @@ -197,12 +204,10 @@ |
932 | |
933 | EXPECT_CALL(surface_observer, resized_to(WidthEq(new_width))); |
934 | |
935 | - auto const spec = mir_connection_create_spec_for_changes(connection); |
936 | - |
937 | - mir_surface_spec_set_width(spec, new_width); |
938 | - |
939 | - mir_surface_apply_spec(surface, spec); |
940 | - mir_surface_spec_release(spec); |
941 | + apply_changes([&](MirSurfaceSpec* spec) |
942 | + { |
943 | + mir_surface_spec_set_width(spec, new_width); |
944 | + }); |
945 | } |
946 | |
947 | TEST_F(SurfaceModifications, surface_spec_change_height_is_notified) |
948 | @@ -211,24 +216,20 @@ |
949 | |
950 | EXPECT_CALL(surface_observer, resized_to(HeightEq(new_height))); |
951 | |
952 | - auto const spec = mir_connection_create_spec_for_changes(connection); |
953 | - |
954 | - mir_surface_spec_set_height(spec, new_height); |
955 | - |
956 | - mir_surface_apply_spec(surface, spec); |
957 | - mir_surface_spec_release(spec); |
958 | + apply_changes([&](MirSurfaceSpec* spec) |
959 | + { |
960 | + mir_surface_spec_set_height(spec, new_height); |
961 | + }); |
962 | } |
963 | |
964 | TEST_F(SurfaceModifications, surface_spec_min_width_is_respected) |
965 | { |
966 | auto const min_width = 17; |
967 | |
968 | - { |
969 | - auto const spec = mir_connection_create_spec_for_changes(connection); |
970 | - mir_surface_spec_set_min_width(spec, min_width); |
971 | - mir_surface_apply_spec(surface, spec); |
972 | - mir_surface_spec_release(spec); |
973 | - } |
974 | + apply_changes([&](MirSurfaceSpec* spec) |
975 | + { |
976 | + mir_surface_spec_set_min_width(spec, min_width); |
977 | + }); |
978 | |
979 | ensure_server_has_processed_setup(); |
980 | |
981 | @@ -245,12 +246,10 @@ |
982 | { |
983 | auto const min_height = 19; |
984 | |
985 | - { |
986 | - auto const spec = mir_connection_create_spec_for_changes(connection); |
987 | - mir_surface_spec_set_min_height(spec, min_height); |
988 | - mir_surface_apply_spec(surface, spec); |
989 | - mir_surface_spec_release(spec); |
990 | - } |
991 | + apply_changes([&](MirSurfaceSpec* spec) |
992 | + { |
993 | + mir_surface_spec_set_min_height(spec, min_height); |
994 | + }); |
995 | |
996 | ensure_server_has_processed_setup(); |
997 | |
998 | @@ -267,12 +266,10 @@ |
999 | { |
1000 | auto const max_width = 23; |
1001 | |
1002 | - { |
1003 | - auto const spec = mir_connection_create_spec_for_changes(connection); |
1004 | - mir_surface_spec_set_max_width(spec, max_width); |
1005 | - mir_surface_apply_spec(surface, spec); |
1006 | - mir_surface_spec_release(spec); |
1007 | - } |
1008 | + apply_changes([&](MirSurfaceSpec* spec) |
1009 | + { |
1010 | + mir_surface_spec_set_max_width(spec, max_width); |
1011 | + }); |
1012 | |
1013 | ensure_server_has_processed_setup(); |
1014 | |
1015 | @@ -289,12 +286,10 @@ |
1016 | { |
1017 | auto const max_height = 29; |
1018 | |
1019 | - { |
1020 | - auto const spec = mir_connection_create_spec_for_changes(connection); |
1021 | - mir_surface_spec_set_max_height(spec, max_height); |
1022 | - mir_surface_apply_spec(surface, spec); |
1023 | - mir_surface_spec_release(spec); |
1024 | - } |
1025 | + apply_changes([&](MirSurfaceSpec* spec) |
1026 | + { |
1027 | + mir_surface_spec_set_max_height(spec, max_height); |
1028 | + }); |
1029 | |
1030 | ensure_server_has_processed_setup(); |
1031 | |
1032 | @@ -306,3 +301,149 @@ |
1033 | generate_alt_click_at(bottom_right); |
1034 | generate_alt_move_to(bottom_right + DeltaY(max_height)); |
1035 | } |
1036 | + |
1037 | +TEST_F(SurfaceModifications, surface_spec_width_inc_is_respected) |
1038 | +{ |
1039 | + auto const width_inc = 13; |
1040 | + |
1041 | + apply_changes([&](MirSurfaceSpec* spec) |
1042 | + { |
1043 | + mir_surface_spec_set_width_increment(spec, width_inc); |
1044 | + }); |
1045 | + |
1046 | + ensure_server_has_processed_setup(); |
1047 | + |
1048 | + auto const shell_surface = this->shell_surface.lock(); |
1049 | + auto const bottom_right = shell_surface->input_bounds().bottom_right() - Displacement{1,1}; |
1050 | + |
1051 | + Size actual; |
1052 | + EXPECT_CALL(surface_observer, resized_to(_)).WillOnce(SaveArg<0>(&actual)); |
1053 | + |
1054 | + generate_alt_click_at(bottom_right); |
1055 | + generate_alt_move_to(bottom_right + DeltaX(16)); |
1056 | + |
1057 | + EXPECT_TRUE(actual.width.as_int() % width_inc == 0); |
1058 | +} |
1059 | + |
1060 | +TEST_F(SurfaceModifications, surface_spec_with_min_width_and_width_inc_is_respected) |
1061 | +{ |
1062 | + auto const width_inc = 13; |
1063 | + auto const min_width = 7; |
1064 | + |
1065 | + apply_changes([&](MirSurfaceSpec* spec) |
1066 | + { |
1067 | + mir_surface_spec_set_width_increment(spec, width_inc); |
1068 | + mir_surface_spec_set_min_width(spec, min_width); |
1069 | + }); |
1070 | + |
1071 | + ensure_server_has_processed_setup(); |
1072 | + |
1073 | + auto const shell_surface = this->shell_surface.lock(); |
1074 | + auto const bottom_right = shell_surface->input_bounds().bottom_right() - Displacement{1,1}; |
1075 | + |
1076 | + Size actual; |
1077 | + EXPECT_CALL(surface_observer, resized_to(_)).WillOnce(SaveArg<0>(&actual)); |
1078 | + |
1079 | + generate_alt_click_at(bottom_right); |
1080 | + generate_alt_move_to(bottom_right + DeltaX(16)); |
1081 | + |
1082 | + EXPECT_TRUE((actual.width.as_int() - min_width) % width_inc == 0); |
1083 | +} |
1084 | + |
1085 | +TEST_F(SurfaceModifications, surface_spec_height_inc_is_respected) |
1086 | +{ |
1087 | + auto const height_inc = 13; |
1088 | + |
1089 | + apply_changes([&](MirSurfaceSpec* spec) |
1090 | + { |
1091 | + mir_surface_spec_set_height_increment(spec, height_inc); |
1092 | + }); |
1093 | + |
1094 | + ensure_server_has_processed_setup(); |
1095 | + |
1096 | + auto const shell_surface = this->shell_surface.lock(); |
1097 | + auto const bottom_right = shell_surface->input_bounds().bottom_right() - Displacement{1,1}; |
1098 | + |
1099 | + Size actual; |
1100 | + EXPECT_CALL(surface_observer, resized_to(_)).WillOnce(SaveArg<0>(&actual)); |
1101 | + |
1102 | + generate_alt_click_at(bottom_right); |
1103 | + generate_alt_move_to(bottom_right + DeltaY(16)); |
1104 | + |
1105 | + EXPECT_TRUE(actual.height.as_int() % height_inc == 0); |
1106 | +} |
1107 | + |
1108 | +TEST_F(SurfaceModifications, surface_spec_with_min_height_and_height_inc_is_respected) |
1109 | +{ |
1110 | + auto const height_inc = 13; |
1111 | + auto const min_height = 7; |
1112 | + |
1113 | + apply_changes([&](MirSurfaceSpec* spec) |
1114 | + { |
1115 | + mir_surface_spec_set_height_increment(spec, height_inc); |
1116 | + mir_surface_spec_set_min_height(spec, min_height); |
1117 | + }); |
1118 | + |
1119 | + ensure_server_has_processed_setup(); |
1120 | + |
1121 | + auto const shell_surface = this->shell_surface.lock(); |
1122 | + auto const bottom_right = shell_surface->input_bounds().bottom_right() - Displacement{1,1}; |
1123 | + |
1124 | + Size actual; |
1125 | + EXPECT_CALL(surface_observer, resized_to(_)).WillOnce(SaveArg<0>(&actual)); |
1126 | + |
1127 | + generate_alt_click_at(bottom_right); |
1128 | + generate_alt_move_to(bottom_right + DeltaY(16)); |
1129 | + |
1130 | + EXPECT_TRUE((actual.height.as_int() - min_height) % height_inc == 0); |
1131 | +} |
1132 | + |
1133 | +TEST_F(SurfaceModifications, surface_spec_with_min_aspect_ratio_is_respected) |
1134 | +{ |
1135 | + auto const aspect_width = 11; |
1136 | + auto const aspect_height = 7; |
1137 | + |
1138 | + apply_changes([&](MirSurfaceSpec* spec) |
1139 | + { |
1140 | + mir_surface_spec_set_min_aspect_ratio(spec, aspect_width, aspect_height); |
1141 | + }); |
1142 | + |
1143 | + ensure_server_has_processed_setup(); |
1144 | + |
1145 | + auto const shell_surface = this->shell_surface.lock(); |
1146 | + auto const bottom_right = shell_surface->input_bounds().bottom_right() - Displacement{1,1}; |
1147 | + auto const bottom_left = shell_surface->input_bounds().bottom_left() + Displacement{1,-1}; |
1148 | + |
1149 | + Size actual; |
1150 | + EXPECT_CALL(surface_observer, resized_to(_)).WillOnce(SaveArg<0>(&actual)); |
1151 | + |
1152 | + generate_alt_click_at(bottom_right); |
1153 | + generate_alt_move_to(bottom_left); |
1154 | + |
1155 | + EXPECT_THAT(actual.width.as_float()/actual.height.as_float(), Ge(float(aspect_width)/aspect_height)); |
1156 | +} |
1157 | + |
1158 | +TEST_F(SurfaceModifications, surface_spec_with_max_aspect_ratio_is_respected) |
1159 | +{ |
1160 | + auto const aspect_width = 7; |
1161 | + auto const aspect_height = 11; |
1162 | + |
1163 | + apply_changes([&](MirSurfaceSpec* spec) |
1164 | + { |
1165 | + mir_surface_spec_set_max_aspect_ratio(spec, aspect_width, aspect_height); |
1166 | + }); |
1167 | + |
1168 | + ensure_server_has_processed_setup(); |
1169 | + |
1170 | + auto const shell_surface = this->shell_surface.lock(); |
1171 | + auto const bottom_right = shell_surface->input_bounds().bottom_right() - Displacement{1,1}; |
1172 | + auto const top_right = shell_surface->input_bounds().top_right() - Displacement{1,-1}; |
1173 | + |
1174 | + Size actual; |
1175 | + EXPECT_CALL(surface_observer, resized_to(_)).WillOnce(SaveArg<0>(&actual)); |
1176 | + |
1177 | + generate_alt_click_at(bottom_right); |
1178 | + generate_alt_move_to(top_right); |
1179 | + |
1180 | + EXPECT_THAT(actual.width.as_float()/actual.height.as_float(), Le(float(aspect_width)/aspect_height)); |
1181 | +} |
PASSED: Continuous integration, rev:2513 jenkins. qa.ubuntu. com/job/ mir-ci/ 3576/ jenkins. qa.ubuntu. com/job/ mir-android- vivid-i386- build/2131 jenkins. qa.ubuntu. com/job/ mir-clang- vivid-amd64- build/2130 jenkins. qa.ubuntu. com/job/ mir-mediumtests -vivid- touch/2081 jenkins. qa.ubuntu. com/job/ mir-vivid- amd64-ci/ 1573 jenkins. qa.ubuntu. com/job/ mir-vivid- amd64-ci/ 1573/artifact/ work/output/ *zip*/output. zip jenkins. qa.ubuntu. com/job/ mir-mediumtests -builder- vivid-armhf/ 2081 jenkins. qa.ubuntu. com/job/ mir-mediumtests -builder- vivid-armhf/ 2081/artifact/ work/output/ *zip*/output. zip jenkins. qa.ubuntu. com/job/ mir-mediumtests -runner- mako/5011 s-jenkins. ubuntu- ci:8080/ job/touch- flash-device/ 19824
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/mir- ci/3576/ rebuild
http://