<div class="gmail_quote">On Fri, Jun 1, 2012 at 1:30 AM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="im">On Thu, 31 May 2012 14:02:48 -0600 Scott Moreau <<a href="mailto:oreaus@gmail.com">oreaus@gmail.com</a>> wrote: > * Drop user input detection since cursor position may change when pasting text, > for example. > * Use fixed type in protocol. </div>Hi Scott, some inline comments I below. <div><div class="h5"> > --- > > Changed int to fixed in protocol/text-cursor-position.xml > > clients/window.c | 15 +++++---------- > clients/window.h | 2 +- > protocol/text-cursor-position.xml | 4 ++-- > src/compositor.c | 11 ++++++----- > src/compositor.h | 6 ++++-- > src/text-cursor-position.c | 2 +- > 6 files changed, 19 insertions(+), 21 deletions(-) > > diff --git a/clients/window.c b/clients/window.c > index 3ef648e..74fc5c1 100644 > --- a/clients/window.c > +++ b/clients/window.c > @@ -151,7 +151,6 @@ struct window { > int resize_needed; > int type; > int transparent; > - int send_cursor_position; > struct input *keyboard_device; > enum window_buffer_type buffer_type; > > @@ -1826,9 +1825,6 @@ keyboard_handle_key(void *data, struct wl_keyboard *keyboard, > if (!window || window->keyboard_device != input) > return; > > - if (state) > - window->send_cursor_position = 1; > - > num_syms = xkb_key_get_syms(d->xkb.state, code, &syms); > xkb_state_update_key(d->xkb.state, code, > state ? XKB_KEY_DOWN : XKB_KEY_UP); > @@ -2616,18 +2612,18 @@ window_get_title(struct window *window) > } > > void > -window_set_text_cursor_position(struct window *window, int32_t x, int32_t y) > +window_set_text_cursor_position(struct window *window, uint32_t x, uint32_t y) </div></div>Why the change in argument types? I think we have all integer coordinates always as signed, regardless of valid domain. </blockquote><div> Ok thanks for pointing this out. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div><div class="h5"> > { > struct text_cursor_position *text_cursor_position = > window->display->text_cursor_position; > > - if (!window->send_cursor_position || !text_cursor_position) > + if (!text_cursor_position) > return; > > text_cursor_position_notify(text_cursor_position, > - window->surface, x, y); > - > - window->send_cursor_position = 0; > + window->surface, > + wl_fixed_from_int(x), > + wl_fixed_from_int(y)); > } > > void > @@ -2716,7 +2712,6 @@ window_create_internal(struct display *display, struct window *parent) > window->allocation.height = 0; > window->saved_allocation = window->allocation; > window->transparent = 1; > - window->send_cursor_position = 0; > window->type = TYPE_NONE; > window->input_region = NULL; > window->opaque_region = NULL; > diff --git a/clients/window.h b/clients/window.h > index a8537b3..09e4dc8 100644 > --- a/clients/window.h > +++ b/clients/window.h > @@ -308,7 +308,7 @@ const char * > window_get_title(struct window *window); > > void > -window_set_text_cursor_position(struct window *window, int32_t x, int32_t y); > +window_set_text_cursor_position(struct window *window, uint32_t x, uint32_t y); > > int > widget_set_tooltip(struct widget *parent, char *entry, float x, float y); > diff --git a/protocol/text-cursor-position.xml b/protocol/text-cursor-position.xml > index dbeda72..0fbc54e 100644 > --- a/protocol/text-cursor-position.xml > +++ b/protocol/text-cursor-position.xml > @@ -3,8 +3,8 @@ > <interface name="text_cursor_position" version="1"> > <request name="notify"> > <arg name="surface" type="object" interface="wl_surface"/> > - <arg name="x" type="uint"/> > - <arg name="y" type="uint"/> > + <arg name="x" type="fixed"/> > + <arg name="y" type="fixed"/> > </request> > </interface> > > diff --git a/src/compositor.c b/src/compositor.c > index a36ccd5..fbfab6e 100644 > --- a/src/compositor.c > +++ b/src/compositor.c > @@ -2446,14 +2446,15 @@ weston_output_destroy(struct weston_output *output) > > WL_EXPORT void > weston_text_cursor_position_notify(struct weston_surface *surface, > - int32_t cur_pos_x, > - int32_t cur_pos_y) > + wl_fixed_t cur_pos_fx, > + wl_fixed_t cur_pos_fy) > { > struct weston_output *output; > - int32_t global_x, global_y; > + wl_fixed_t global_x, global_y; > > - weston_surface_to_global(surface, cur_pos_x, cur_pos_y, > - &global_x, &global_y); > + weston_surface_to_global(surface, wl_fixed_to_int(cur_pos_fx), > + wl_fixed_to_int(cur_pos_fy), > + &global_x, &global_y); </div></div>That is wrong, because global_x is wl_fixed_t and weston_surface_to_global() expects an int32_t. Therefore the result will be off by a factor of 256. If this works for you, then there probably is another similar confusion between int and fixed elsewhere. </blockquote><div> Yes, I don't really understand all this wl_fixed_t stuff and it's somewhat annoying. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> You should use weston_surface_to_global_fixed() instead, dropping the wl_fixed_to_int() calls, which throw away precision - the reason why wl_fixed_t exists in the first place. </blockquote><div> Ok thanks, I have to rework this patch anyway against latest commits. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> This would have been cought by the compiler, if wl_fixed_t just was a type-safe type and not something that implicitly casts to any integer... </blockquote><div> Yes again, annoying. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div><div class="h5"> > > wl_list_for_each(output, &surface->compositor->output_list, link) > if (output->zoom.active && > diff --git a/src/compositor.h b/src/compositor.h > index 09cd215..63ad3a6 100644 > --- a/src/compositor.h > +++ b/src/compositor.h > @@ -567,9 +567,11 @@ void > weston_compositor_shutdown(struct weston_compositor *ec); > void > weston_output_update_zoom(struct weston_output *output, > - int x, int y, uint32_t type); > + wl_fixed_t x, wl_fixed_t y, > + uint32_t type); > void > -weston_text_cursor_position_notify(struct weston_surface *surface, int x, int y); > +weston_text_cursor_position_notify(struct weston_surface *surface, > + wl_fixed_t x, wl_fixed_t y); > void > weston_output_update_matrix(struct weston_output *output); > void > diff --git a/src/text-cursor-position.c b/src/text-cursor-position.c > index 6f46636..ef1085a 100644 > --- a/src/text-cursor-position.c > +++ b/src/text-cursor-position.c > @@ -37,7 +37,7 @@ static void > text_cursor_position_notify(struct wl_client *client, > struct wl_resource *resource, > struct wl_resource *surface_resource, > - uint32_t x, uint32_t y) > + wl_fixed_t x, wl_fixed_t y) > { > weston_text_cursor_position_notify((struct weston_surface *) surface_resource, x, y); > } </div></div>Thanks, pq </blockquote><div> Thanks, Scott </div></div>