[RFC] compositor: Introduce composite zoom. v1
Scott Moreau
oreaus at gmail.com
Tue Feb 14 08:31:04 PST 2012
On Tue, Feb 14, 2012 at 12:53 AM, Pekka Paalanen <ppaalanen at gmail.com>wrote:
> On Mon, 13 Feb 2012 15:26:16 -0700
> Scott Moreau <oreaus at gmail.com> wrote:
>
> > Implements a 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 to come. For now we double check for the binding
> > in notify_key as a quick-n-dirty hack to avoid sending unwanted events.
> > Zoom in/out with Super+Up/Down. Zoom area follows mouse pointer.
> >
> > ---
> > src/compositor.c | 43 +++++++++++++++++++++++++++++++++++++++++--
> > src/compositor.h | 15 +++++++++++++++
> > src/shell.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 99 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/compositor.c b/src/compositor.c
> > index ab90ded..de4f9f4 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -307,6 +307,21 @@ surface_compute_bbox(struct weston_surface
> *surface, int32_t sx, int32_t sy,
> > ceilf(max_x) - int_x, ceilf(max_y) -
> int_y);
> > }
> >
> > +WL_EXPORT void
> > +weston_update_zoom(struct weston_output *output, int x, int y)
>
> We already have the name "zoom" in util.c for per-surface scaling
> effect. Is there a danger to confuse these?
>
Probably. Which is why I think it may be time to start thinking about how
to separate functionalities better.
>
> Following the naming conventions I were taught here, this function
> should at least be called weston_output_update_zoom(), since it is a
> weston_output method.
>
Noted.
>
> > +{
> > + if (output->zoom.level <= 0.0)
> > + return;
> > +
> > + GLfloat 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);
>
> That indentation looks funny.
>
Maybe it can be output->zoom.trans_x =\n ...
>
> > + weston_output_damage(output);
> > +}
> > +
> > static void
> > weston_surface_update_transform_disable(struct weston_surface *surface)
> > {
> > @@ -774,7 +789,16 @@ weston_surface_draw(struct weston_surface *es,
> struct weston_output *output)
> > glUseProgram(es->shader->program);
> > ec->current_shader = es->shader;
> > }
> > + glUniform1f(es->shader->zoom_uniform, output->zoom.level);
> >
> > + struct weston_matrix modelview_inverse;
> > + weston_matrix_init(&output->modelview_matrix);
> > + weston_matrix_translate(&output->modelview_matrix,
> > +
> output->zoom.trans_x,
> > +
> output->zoom.trans_y, 0);
> > + weston_matrix_invert(&modelview_inverse,
> &output->modelview_matrix);
>
> Couldn't these computations be moved to some per-output function?
> Repeating the same computation per surface is unnecessary.
>
Didn't notice that, thanks.
>
> Also, do you really need to call weston_matrix_invert() for inverting a
> pure translation matrix? ;-)
>
For simple translation, it could be made much more compact but I wanted to
simulate a camera matrix.
>
> > + glUniformMatrix4fv(es->shader->modelview_uniform,
> > + 1, GL_FALSE, modelview_inverse.d);
>
> The inverse matrix is going into modelview uniform, one of them needs a
> corretion in the name.
>
> > glUniformMatrix4fv(es->shader->proj_uniform,
> > 1, GL_FALSE, output->matrix.d);
> > glUniform1i(es->shader->tex_uniform, 0);
> > @@ -1349,6 +1373,8 @@ 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)
> > + weston_update_zoom(output, x, y);
> > }
> >
> > if (!x_valid) {
> > @@ -1466,6 +1492,9 @@ notify_key(struct wl_input_device *device,
> >
> > weston_compositor_run_binding(compositor, wd, time, key, 0, state);
> >
> > + if (wd->modifier_state & MODIFIER_SUPER && (key == KEY_UP || key
> == KEY_DOWN))
> > + return;
>
> This does look a bit suspicious. So the reason for this is, that if we
> have keyboard-only bindings, using them will send the key events also to
> the client?
>
Yes.
>
> Hm, maybe we would need a keyboard grab for the time the binding is
> held down, to easily avoid sending both the key press and release, and
> getting clients out of sync. Waiting for proper keyboard grabs to be
> implemented...
>
Yes, I've been looking into how to do proper keyboard grabs.
>
> > +
> > update_modifier_state(wd, key, state);
> > end = device->keys.data + device->keys.size;
> > for (k = device->keys.data; k < end; k++) {
> > @@ -1815,12 +1844,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";
> >
> > @@ -1894,6 +1925,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");
> > @@ -1950,6 +1983,12 @@ 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.increment = 0.05;
> > + output->zoom.level = 1.0;
> > + output->zoom.trans_x = 0.0;
> > + output->zoom.trans_y = 0.0;
>
> AFAICT, you are only using trans_x,trans_y for creating an equivalent
> translation matrix. You could store the translation matrix here instead
> and change the x,y elements.
>
Yes, it certainly could be much simpler strictly for zoom. As you said on
irc though, this could potentially be used for screen rotation and possibly
other things. I wanted to take steps to show the function without
obfuscating much, while trying to make it versatile.
>
> > +
> > output->flags = flags;
> > weston_output_move(output, x, y);
> >
> > diff --git a/src/compositor.h b/src/compositor.h
> > index 966d3f4..0f5827e 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -54,10 +54,19 @@ struct weston_border {
> > int32_t left, right, top, bottom;
> > };
> >
> > +struct weston_composite_zoom {
> > + int active;
> > + /* 1.0 is no zoom, 0.0 is 100% zoomed in */
> > + GLfloat level;
> > + float increment;
> > + int trans_x, trans_y;
> > +};
> > +
> > struct weston_output {
> > struct wl_list link;
> > struct weston_compositor *compositor;
> > struct weston_matrix matrix;
> > + struct weston_matrix modelview_matrix;
> > struct wl_list frame_callback_list;
> > int32_t x, y, mm_width, mm_height;
> > struct weston_border border;
> > @@ -66,6 +75,7 @@ struct weston_output {
> > uint32_t flags;
> > int repaint_needed;
> > int repaint_scheduled;
> > + struct weston_composite_zoom zoom;
> >
> > char *make, *model;
> > uint32_t subpixel;
> > @@ -105,6 +115,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;
> > @@ -325,6 +337,9 @@ void
> > weston_surface_draw(struct weston_surface *es, struct weston_output
> *output);
> >
> > void
> > +weston_update_zoom(struct weston_output *output, int x, int y);
> > +
> > +void
> > notify_motion(struct wl_input_device *device,
> > uint32_t time, int x, int y);
> > void
> > diff --git a/src/shell.c b/src/shell.c
> > index 66c4f01..6b2ffc0 100644
> > --- a/src/shell.c
> > +++ b/src/shell.c
> > @@ -964,6 +964,45 @@ resize_binding(struct wl_input_device *device,
> uint32_t time,
> > }
> >
> > 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;
> > + output = container_of(compositor->output_list.next,
> > + struct weston_output, link);
>
> You always zoom only the first output? How about an output where the
> pointer is in, or lacking a pointer, the output of the surface which
> has the keyboard focus?
>
Ah, should fix that. Thanks.
>
> From X you can easily test multiple outputs.
>
Yes, I was wondering why it only zoomed one input regardless.
>
> > +
> > + output->zoom.active = 1;
> > + output->zoom.level -= output->zoom.increment;
> > + if (output->zoom.level > 1.0)
> > + output->zoom.level = 1.0;
> > + if (output->zoom.level <= output->zoom.increment)
> > + output->zoom.level = output->zoom.increment;
> > +
> > + weston_update_zoom(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;
> > + output = container_of(compositor->output_list.next,
> > + struct weston_output, link);
> > +
> > + output->zoom.level += output->zoom.increment;
> > + if (output->zoom.level > 1.0) {
> > + output->zoom.active = 0;
> > + output->zoom.level = 1.0;
> > + }
> > +
> > + weston_update_zoom(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)
> > {
> > @@ -1618,6 +1657,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);
>
> All my comments were just on the details.
>
> I'll leave commenting on the feature as a whole for krh.
>
> Thanks for your input.
Cheers,
Scott
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20120214/0a6f76a6/attachment-0001.htm>
More information about the wayland-devel
mailing list