Merge lp://qastaging/~mc-return/compiz/compiz.merge-fix1167933.2-KEY_MOVE_INC-is-hardcoded into lp://qastaging/compiz/0.9.10

Proposed by MC Return
Status: Merged
Approved by: Sam Spilsbury
Approved revision: 3656
Merged at revision: 3660
Proposed branch: lp://qastaging/~mc-return/compiz/compiz.merge-fix1167933.2-KEY_MOVE_INC-is-hardcoded
Merge into: lp://qastaging/compiz/0.9.10
Diff against target: 83 lines (+12/-9)
3 files modified
plugins/move/move.xml.in (+8/-3)
plugins/move/src/move.cpp (+4/-4)
plugins/move/src/move.h (+0/-2)
To merge this branch: bzr merge lp://qastaging/~mc-return/compiz/compiz.merge-fix1167933.2-KEY_MOVE_INC-is-hardcoded
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Sam Spilsbury Approve
MC Return Needs Resubmitting
Review via email: mp+158767@code.qastaging.launchpad.net

Commit message

*Move xml:

Slightly improved long description of the move plugin.
Implemented option to change the key move increment size.
Removed <precision> settings as they just work for floats.

*Move code:

Removed hardcoded KEY_MOVE_INC variable.
Introduced int moveIncrement to not have to execute
optionGetKeyMoveInc () to get the value twice.

(LP: #1167933, part 2)

To post a comment you must log in.
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

38 + mKeys[i].dx * optionGetKeyMoveInc (),
39 + mKeys[i].dy * optionGetKeyMoveInc ());

What I'd suggest doing is making a temporary instead of calling optionGetMoveInc () twice. Eg:

int moveIncrement = optionGetKeyMoveInc ();

...

mKeys[i].dx * moveIncrement;
mKeys[i].dy * moveIncrement;

Revision history for this message
MC Return (mc-return) wrote :

> 38 + mKeys[i].dx *
> optionGetKeyMoveInc (),
> 39 + mKeys[i].dy *
> optionGetKeyMoveInc ());
>
> What I'd suggest doing is making a temporary instead of calling
> optionGetMoveInc () twice. Eg:
>
> int moveIncrement = optionGetKeyMoveInc ();
>
> ...
>
> mKeys[i].dx * moveIncrement;
> mKeys[i].dy * moveIncrement;

I tried that (because I knew you would say that ;)), but unfortunately
testing showed that it stopped being interactive - changing the value
was somehow not immediately applied then and the old value was still
used...
Not sure why though...

3656. By MC Return

Move xml:
Slightly improved long description of the move plugin
Removed <precision> settings as they just work for floats

Move code:
Introduced int moveIncrement to not have to execute
optionGetKeyMoveInc () to get the value twice

Revision history for this message
MC Return (mc-return) wrote :

> 38 + mKeys[i].dx *
> optionGetKeyMoveInc (),
> 39 + mKeys[i].dy *
> optionGetKeyMoveInc ());
>
> What I'd suggest doing is making a temporary instead of calling
> optionGetMoveInc () twice. Eg:
>
> int moveIncrement = optionGetKeyMoveInc ();
>
> ...
>
> mKeys[i].dx * moveIncrement;
> mKeys[i].dy * moveIncrement;

Done in r3656. Further testing showed that once it gets declared
inside the for loop, it works correctly.

Revision history for this message
MC Return (mc-return) :
review: Needs Resubmitting
Revision history for this message
Sam Spilsbury (smspillaz) :
review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: