[Spice-devel] [vdagent-win PATCH] handle_mouse_event: fix off-by-one when normalizing mouse coordinates

[Uri Lublin]

On 11/23/2015 01:40 AM, Fabiano Fidêncio wrote:
> On Sun, Nov 22, 2015 at 4:54 PM, Uri Lublin <uril at redhat.com> wrote:
>> Mouse coordinates can be 0..(width-1) and 0..(height-1)
>>
>> Normalization of mouse coordinates (for SendInput) was done
>> against w,h and thus right-most and down-most coordinates
>> could never be reached.
>
> So, you're avoiding a division by zero, right? This is the main point
> of this patch AFAIU.
> IMHO, it should be a bit more explicit in the log.

Hi Fabiano,

The main point of this patch is to normalize the input value sent
to Windows with SendInput using w-1 and h-1.
The normalized right-most point would be 0xFFFF (65536).
Since the possible values are 0..(w-1) with the new code we
get 0xFFFF * (w-1)/(w-1)
instead of what was before 0xFFFF * (w-1)/w .
For example if w=800, 0xFFFF * (w-1)/w = 65453


I can replace "protection" below with an explicit sentence.

Thanks,
     Uri

>
>>
>> Fix it by normalizing against (w-1) and (h-1).
>>
>> Also added protection against a case of 0 or 1 (for any of w,h)
>> Fixes rhbz#1032037
>> ---
>>   vdagent/vdagent.cpp | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp
>> index c976665..595db85 100644
>> --- a/vdagent/vdagent.cpp
>> +++ b/vdagent/vdagent.cpp
>> @@ -567,13 +567,15 @@ bool VDAgent::handle_mouse_event(VDAgentMouseState* state)
>>       ZeroMemory(&_input, sizeof(INPUT));
>>       _input.type = INPUT_MOUSE;
>>       if (state->x != _mouse_x || state->y != _mouse_y) {
>> +        DWORD w = _desktop_layout->get_total_width();
>> +        DWORD h = _desktop_layout->get_total_height();
>> +        w = (w > 1) ? w-1 : 1; /* coordinates are 0..w-1, protect w==0 */
>> +        h = (h > 1) ? h-1 : 1; /* coordinates are 0..h-1, protect h==0 */
>>           _mouse_x = state->x;
>>           _mouse_y = state->y;
>>           mouse_move = MOUSEEVENTF_MOVE;
>> -        _input.mi.dx = (mode->get_pos_x() + _mouse_x) * 0xffff /
>> -                       _desktop_layout->get_total_width();
>> -        _input.mi.dy = (mode->get_pos_y() + _mouse_y) * 0xffff /
>> -                       _desktop_layout->get_total_height();
>> +        _input.mi.dx = (mode->get_pos_x() + _mouse_x) * 0xffff / w;
>> +        _input.mi.dy = (mode->get_pos_y() + _mouse_y) * 0xffff / h;
>>       }
>>       if (state->buttons != _buttons_state) {
>>           buttons_change = get_buttons_change(_buttons_state, state->buttons, VD_AGENT_LBUTTON_MASK,
>> --
>> 2.5.0
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
> ACK (feel free to ignore my comment if you think the commit message is
> clear enough).
>