<div class="gmail_quote">2012/2/21 Kristian Høgsberg <<a href="mailto:krh@bitplanet.net">krh@bitplanet.net</a>> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="im">On Mon, Feb 20, 2012 at 11:56 PM, Scott Moreau <<a href="mailto:oreaus@gmail.com">oreaus@gmail.com</a>> wrote: > Implement a camera/modelview matrix for transforms to simulate camera movement. > Ideally, we would want to use <modifier>+Scroll but that will have to wait > for axis events. For now we use keyboard grabs. Zoom in/out with Super+Up/Down. > Zoom area follows mouse pointer. </div>Very cool! I like it, but there's a few comments below. thanks, Kristian <div><div class="h5"> > --- > src/compositor.c | 41 +++++++++++++++++++++++- > src/compositor.h | 12 +++++++ > src/shell.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 143 insertions(+), 1 deletions(-) > > diff --git a/src/compositor.c b/src/compositor.c > index 8339e6c..ccbccd4 100644 > --- a/src/compositor.c > +++ b/src/compositor.c > @@ -925,6 +925,7 @@ weston_output_repaint(struct weston_output *output, int msecs) > struct weston_surface *es; > struct weston_animation *animation, *next; > struct weston_frame_callback *cb, *cnext; > + struct weston_matrix modelview; > pixman_region32_t opaque, new_damage, total_damage, > overlap, surface_overlap; > int32_t width, height; > @@ -937,6 +938,12 @@ weston_output_repaint(struct weston_output *output, int msecs) > output->border.top + output->border.bottom; > glViewport(0, 0, width, height); > > + weston_matrix_init(&output->camera_matrix); > + weston_matrix_translate(&output->camera_matrix, > + output->zoom.trans_x, > + output->zoom.trans_y, 0); > + weston_matrix_invert(&modelview, &output->camera_matrix); > + </div></div>I think we want to move the output->matrix computation out in a weston_output_update_matrix() functions and then just roll the zoom math into that. Then in weston_output_move() or the zoom functions, when we update the output transform, we just mark it dirty and recompute it in weston_output_repaint(). </blockquote><div> Yes, this sounds like it would be more versatile. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div class="im"> > pixman_region32_init(&new_damage); > pixman_region32_init(&opaque); > pixman_region32_init(&overlap); > @@ -953,6 +960,9 @@ weston_output_repaint(struct weston_output *output, int msecs) > pixman_region32_fini(&surface_overlap); > pixman_region32_union(&overlap, &overlap, > &es->transform.boundingbox); > + glUniform1f(es->shader->zoom_uniform, output->zoom.level); > + glUniformMatrix4fv(es->shader->modelview_uniform, > + 1, GL_FALSE, modelview.d); </div>Need to do this in weston_surface_draw so we update the right shader uniform, but if we roll the zoom transform into output->matrix as described above that will happen automatically. </blockquote><div> Noted. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div class="im"> > } > > weston_output_set_cursor(output, ec->input_device); > @@ -1322,6 +1332,9 @@ notify_motion(struct wl_input_device *device, uint32_t time, int x, int y) > max_x = output->x + output->current->width; > if (output->y + output->current->height > max_y) > max_y = output->y + output->current->height; > + if (output->zoom.active && > + pixman_region32_contains_point(&output->region, x, y, NULL)) > + zoom_update(output, x, y); </div>There's an edge case here (literally!) since if the pointer is on the screen edge, the zoom position doesn't update. If I move along the edge, the view doesn't change. Only when I move it into the screen, away from the edge does the position update. </blockquote><div> Admittedly, I haven't tested it outside of X. I will have a look into it. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div class="im"> > } > > if (!x_valid) { > @@ -1850,12 +1863,14 @@ bind_output(struct wl_client *client, > > static const char vertex_shader[] = > "uniform mat4 proj;\n" > + "uniform mat4 modelview;\n" > + "uniform float zoom;\n" > "attribute vec2 position;\n" > "attribute vec2 texcoord;\n" > "varying vec2 v_texcoord;\n" > "void main()\n" > "{\n" > - " gl_Position = proj * vec4(position, 0.0, 1.0);\n" > + " gl_Position = proj * (modelview * vec4(position, 0.0, zoom));\n" > " v_texcoord = texcoord;\n" > "}\n"; </div>Again, if we roll the zoom into the output matrix we don't need this. </blockquote><div> Ok. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div class="im"> > @@ -1929,6 +1944,8 @@ weston_shader_init(struct weston_shader *shader, > } > > shader->proj_uniform = glGetUniformLocation(shader->program, "proj"); > + shader->modelview_uniform = glGetUniformLocation(shader->program, "modelview"); > + shader->zoom_uniform = glGetUniformLocation(shader->program, "zoom"); > shader->tex_uniform = glGetUniformLocation(shader->program, "tex"); > shader->alpha_uniform = glGetUniformLocation(shader->program, "alpha"); > shader->color_uniform = glGetUniformLocation(shader->program, "color"); > @@ -1946,6 +1963,21 @@ weston_output_destroy(struct weston_output *output) > } > > WL_EXPORT void > +zoom_update(struct weston_output *output, int x, int y) > +{ > + if (output->zoom.level <= 0) > + return; > + > + float ratio = (1 / output->zoom.level) - 1; > + > + output->zoom.trans_x = (output->mm_width * ratio) * > + ((float)x / output->mm_width); > + output->zoom.trans_y = (output->mm_height * ratio) * > + ((float)y / output->mm_height); > + weston_output_damage(output); > +} </div>output->mm_width is the physical width of the display (in millimeter), not the number of pixels. Use output->current->width.</blockquote><div> Ah, I did not realize this </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> And this should be in the weston_output_matrix_update() function mentioned above. </blockquote><div> Right. </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_EXPORT void > weston_output_move(struct weston_output *output, int x, int y) > { > int flip; > @@ -1985,6 +2017,13 @@ weston_output_init(struct weston_output *output, struct weston_compositor *c, > output->mm_width = width; > output->mm_height = height; > > + output->zoom.active = 0; > + output->zoom.level = 1.0; > + output->zoom.trans_x = 0.0; > + output->zoom.trans_y = 0.0; > + > + weston_matrix_init(&output->camera_matrix); > + > output->flags = flags; > weston_output_move(output, x, y); > > diff --git a/src/compositor.h b/src/compositor.h > index 4c82e79..66ec688 100644 > --- a/src/compositor.h > +++ b/src/compositor.h > @@ -54,10 +54,17 @@ struct weston_border { > int32_t left, right, top, bottom; > }; > > +struct weston_output_zoom { > + int active; > + float level; > + int trans_x, trans_y; > +}; > + > struct weston_output { > struct wl_list link; > struct weston_compositor *compositor; > struct weston_matrix matrix; > + struct weston_matrix camera_matrix; > struct wl_list frame_callback_list; > int32_t x, y, mm_width, mm_height; > struct weston_border border; > @@ -66,6 +73,7 @@ struct weston_output { > uint32_t flags; > int repaint_needed; > int repaint_scheduled; > + struct weston_output_zoom zoom; > > char *make, *model; > uint32_t subpixel; > @@ -100,6 +108,8 @@ struct weston_shader { > GLuint program; > GLuint vertex_shader, fragment_shader; > GLint proj_uniform; > + GLint modelview_uniform; > + GLint zoom_uniform; > GLint tex_uniform; > GLint alpha_uniform; > GLint color_uniform; > @@ -414,6 +424,8 @@ weston_compositor_init(struct weston_compositor *ec, struct wl_display *display) > void > weston_compositor_shutdown(struct weston_compositor *ec); > void > +zoom_update(struct weston_output *output, int x, int y); > +void > weston_output_move(struct weston_output *output, int x, int y); > void > weston_output_init(struct weston_output *output, struct weston_compositor *c, > diff --git a/src/shell.c b/src/shell.c > index fa165e2..ee406f0 100644 > --- a/src/shell.c > +++ b/src/shell.c > @@ -113,6 +113,11 @@ struct shell_surface { > struct wl_list link; > }; > > +struct weston_zoom_grab { > + struct wl_keyboard_grab grab; > + uint32_t key; > +}; > + > struct weston_move_grab { > struct wl_pointer_grab grab; > struct weston_surface *surface; > @@ -1035,6 +1040,88 @@ resize_binding(struct wl_input_device *device, uint32_t time, > } > > static void > +zoom_grab_key(struct wl_keyboard_grab *grab, > + uint32_t time, uint32_t key, int32_t state) > +{ > + struct weston_zoom_grab *zoom; > + zoom = container_of(grab, struct weston_zoom_grab, grab); > + > + if (state == 0) { > + wl_input_device_end_keyboard_grab(grab->input_device, time); > + free(zoom); > + } > +} > + > +static const struct wl_keyboard_grab_interface zoom_grab_interface = { > + zoom_grab_key, > +}; > + > +static void > +zoom_in_binding(struct wl_input_device *device, uint32_t time, > + uint32_t key, uint32_t button, uint32_t state, void *data) > +{ > + struct weston_input_device *wd = (struct weston_input_device *) device; > + struct weston_compositor *compositor = wd->compositor; > + struct weston_output *output; > + struct weston_zoom_grab *zoom; > + > + zoom = malloc(sizeof *zoom); > + if (!zoom) > + return; > + > + zoom->grab.interface = &zoom_grab_interface; > + > + wl_input_device_start_keyboard_grab(&wd->input_device, &zoom->grab, time); </div></div>Why do you use a key grab here? Oh, it's to avoid delivering the event right? I think we just need to make bindings swallow the event (key press and release for the key in question, for buttons too).</blockquote><div> Indeed. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> The grab method you're using breaks if somebody presses and then releases another key or the super key first. </blockquote><div> Yes, I realize this. I just wanted to make sure there was no chance of not freeing allocated memory. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Oh, as a feature request, could we make shift+super do integer scaling? As it is, I have to press 10 times to get to double size, triple size isn't possible, and 10 more times to do 4 times zoom.</blockquote><div> Absolutely. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> We may also want GL_LINEAR filtering when we're zoomed to a non-integer multiple. </blockquote><div> Yes, indeed. </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, &compositor->output_list, link) { > + if (pixman_region32_contains_point(&output->region, > + device->x, device->y, NULL)) { > + output->zoom.active = 1; > + output->zoom.level -= 0.05; > + if (output->zoom.level > 1.0) > + output->zoom.level = 1.0; > + if (output->zoom.level < 0.0) > + output->zoom.level = 0.05; > + > + zoom_update(output, device->x, device->y); > + } > + } > +} > + > +static void > +zoom_out_binding(struct wl_input_device *device, uint32_t time, > + uint32_t key, uint32_t button, uint32_t state, void *data) > +{ > + struct weston_input_device *wd = (struct weston_input_device *) device; > + struct weston_compositor *compositor = wd->compositor; > + struct weston_output *output; > + struct weston_zoom_grab *zoom; > + > + zoom = malloc(sizeof *zoom); > + if (!zoom) > + return; > + > + zoom->grab.interface = &zoom_grab_interface; > + > + wl_input_device_start_keyboard_grab(&wd->input_device, &zoom->grab, time); > + > + wl_list_for_each(output, &compositor->output_list, link) { > + if (pixman_region32_contains_point(&output->region, > + device->x, device->y, NULL)) { > + output->zoom.level += 0.05; > + if (output->zoom.level > 1.0) > + output->zoom.level = 1.0; > + if (output->zoom.level < 0.0) { > + output->zoom.active = 0; > + output->zoom.level = 0.0; > + } > + > + zoom_update(output, device->x, device->y); > + } > + } > +} > + > +static void > terminate_binding(struct wl_input_device *device, uint32_t time, > uint32_t key, uint32_t button, uint32_t state, void *data) > { > @@ -1827,6 +1914,10 @@ shell_init(struct weston_compositor *ec) > move_binding, shell); > weston_compositor_add_binding(ec, 0, BTN_MIDDLE, MODIFIER_SUPER, > resize_binding, shell); > + weston_compositor_add_binding(ec, KEY_UP, 0, MODIFIER_SUPER, > + zoom_in_binding, shell); > + weston_compositor_add_binding(ec, KEY_DOWN, 0, MODIFIER_SUPER, > + zoom_out_binding, shell); > weston_compositor_add_binding(ec, KEY_BACKSPACE, 0, > MODIFIER_CTRL | MODIFIER_ALT, > terminate_binding, ec); > -- > 1.7.4.1 > </div></div>> _______________________________________________ > wayland-devel mailing list > <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </blockquote></div> Thanks for your input. I will work on a v3 patch now. Cheers, Scott