[PATCH 2/2 weston v4] desktop-shell: Implement panel window list.
This patch uses the special surface_data interface to send surface_data to the
shell. The shell then uses this information to render a window list in the
panel.
---
v4:
* Restructured function names and locations for better readability
* Set shell->surface_data_manager to NULL on unbind and check that it's valid
before sending any events to a surface_data object
* Rolled 3/3 of this set into this patch
clients/desktop-shell.c | 504 ++--
src/compositor.c| 2 +
src/compositor.h| 1 +
src/shell.c | 184 ++
4 files changed, 673 insertions(+), 18 deletions(-)
diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index 588dc1c..a63ebb3 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -44,6 +44,8 @@
#include "desktop-shell-client-protocol.h"
+#define MIN(x, y) (((x) < (y)) ? (x) : (y))
+
extern char **environ; /* defined by libc */
struct desktop {
@@ -52,36 +54,55 @@ struct desktop {
struct unlock_dialog *unlock_dialog;
struct task unlock_task;
struct wl_list outputs;
+ uint32_t output_count;
struct window *grab_window;
struct widget *grab_widget;
enum cursor_type grab_cursor;
+
+ struct surface_data_manager *surface_data_manager;
};
struct surface {
+ struct wl_list item_list;
+ struct surface_data *surface_data;
+ uint32_t output_mask;
+ char *title;
+};
+
+struct resize {
void (*configure)(void *data,
struct desktop_shell *desktop_shell,
uint32_t edges, struct window *window,
int32_t width, int32_t height);
};
+struct rgba {
+ float r, g, b, a;
+};
+
struct panel {
- struct surface base;
+ struct resize base;
struct window *window;
struct widget *widget;
struct wl_list launcher_list;
+ struct wl_list window_list;
+ struct rectangle window_list_rect;
+ uint32_t surface_count;
+ struct rgba focused_item;
struct panel_clock *clock;
};
struct background {
- struct surface base;
+ struct resize base;
struct window *window;
struct widget *widget;
};
struct output {
struct wl_output *output;
+ uint32_t id;
struct wl_list link;
struct panel *panel;
@@ -99,6 +120,16 @@ struct panel_launcher {
struct wl_array argv;
};
+struct list_item {
+ struct surface *surface;
+ struct widget *widget;
+ struct panel *panel;
+ cairo_surface_t *icon;
+ int focused, pressed;
+ struct wl_list link;
+ struct wl_list surface_link;
+};
+
struct panel_clock {
struct widget *widget;
struct panel *panel;
@@ -249,6 +280,15 @@ set_hex_color(cairo_t *cr, uint32_t color)
}
static void
+get_hex_color_rgba(uint32_t color, float *r, float *g, float *b, float *a)
+{
+ *r = ((color >> 16) & 0xff) / 255.0;
+ *g = ((color >> 8) & 0xff) / 255.0;
+ *b = ((color >> 0) & 0xff) / 255.0;
+ *a = ((color >> 24) & 0xff) / 255.0;
+}
+
+static void
panel_redraw_handler(struct widget *widget, void *data)
{
cairo_surface_t *surface;
@@ -337,7 +377,7 @@ panel_clock_redraw_handler(struct widget *widget, void
*data)
surface = window_get_surface(clock->panel->window);
cr = cairo_create(surface);
- cairo_select_font_face(cr, "sans",
+ cairo_select_font_face(cr, "helvetica",
CAIRO_FONT_SLANT_NORMAL,
CAIRO_FONT_WEIGHT_NORMAL);
cairo_set_font_size(cr, 14);
@@ -411,15 +451,49 @@ panel_button_handler(struct widget *widget,
}
static void
+panel_window_list_schedule_redraw(struct panel *panel)
+{
+ struct list_item *item;
+ float x, y, w, h;
+ float item_width, padding;
+
+ /* If there are no window list items, redraw the panel to clear it */
+ if (wl_list_empty(&panel->window_list)) {
+ widget_schedule_redraw(panel->widget);
+ return;
+ }
+
+ item_width = ((float) panel->window_list_rect.width /
+ panel->surface_count);
+ padding = MIN(item_width * 0.1f, 10.0f);
+
+ x = panel->window_list_rect.x + padding;
+ y = 16;
+ w = MIN(item_width - padding, 200);
+ h = 24;
+
+ wl_list_for_each(item, &panel->window_list, link) {
+ widget_set_allocation(item->widget, x, y - h / 2, w + 1, h + 1);
+ x += w + padding;
+ widget_schedule_redraw(item->widget);
+ }
+}
+
+static void
panel_resize_handler(struct widget *widget,
int32_t width, int32_t height, void *data)
{
struct panel_launcher *launcher;
+ struct rectangle launcher_rect;
+ struct rectangle clock_rect;
struct
Re: [PATCH weston 4/8] shell: Update bindings to conform to pointer axis protocol
Hi, On 29 September 2012 19:31, Jonas Ådahl wrote: > Ok, so what I'm trying to do is to enable what people call "smooth > scrolling" on an input level, meaning that scrolling is not based on > discrete arbitrary "steps" but on a more fluid motion. These types of > events makes most sense for certain types of step-less scroll wheels > and touchpads and I'll try to explain why. I agree it makes perfect sense, but note that axis events are already fixed-point, so you already get fairly fluid motion (motion in units of 1/256th of a wheel 'chunk', i.e. already subpixel). The only advantage of making it pixel-based is that clients no longer have to do the multiplication. But some applications don't want to smoothly scroll by pixels, and instead just want to, e.g, flip a page, every time the scroll motion passes a certain threshold. > When axis events are discrete steps, there is indeed little need to > relate to any kind of coordinate space except knowing what is "up" and > what is "down". A step can only be 1 or -1, thats it. This is how it > traditionally works in X11 (except XI2 I think supports non-discrete > axis events). XI2 follows exactly the same approach of offering scroll events in units of wheel chunks, except with a little more precision (using 16.16 fixed-point rather than 24.8). > If one wants to have axis events that more resemble smooth motions, > such as the ones emitted by those step-less scroll wheels or > touchpads, one needs to specify what the events actually mean, since > they are no longer only limited to 1 and -1. To do this, if we specify > an axis event to be a vector along an axis in a coordinate space > identical to motion events, we can create axis events that relate to > some measurement already known to both the compositor and client. A > step-less scroll wheel would transform its scroll events to a motion > vector measured in pixels and a touchpad would simply emit an axis > event as it would emit a motion event when scrolling. A client could > then read these events and can scroll its view by that amount of > pixels specified by the value parameter. I don't think this change makes much difference; the key is really just tuning the acceleration in the server (usually so it has a little more initial resistance and dampens low speeds, but accelerates dramatically at high speeds). I'm pretty ambivalent about this. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] evdev: Update axis notifications to follow protocol
Signed-off-by: Jonas Ådahl
---
src/evdev.c | 38 +-
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/src/evdev.c b/src/evdev.c
index 8848736..c8513c8 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -30,6 +30,8 @@
#include "compositor.h"
#include "evdev.h"
+#define DEFAULT_AXIS_STEP_DISTANCE wl_fixed_from_int(10)
+
void
evdev_led_update(struct evdev_device *device, enum weston_led leds)
{
@@ -161,17 +163,35 @@ evdev_process_relative(struct evdev_device *device,
device->pending_events |= EVDEV_RELATIVE_MOTION;
break;
case REL_WHEEL:
- notify_axis(device->seat,
- time,
- WL_POINTER_AXIS_VERTICAL_SCROLL,
- wl_fixed_from_int(e->value));
+ switch (e->value) {
+ case -1:
+ /* Scroll down */
+ case 1:
+ /* Scroll up */
+ notify_axis(device->seat,
+ time,
+ WL_POINTER_AXIS_VERTICAL_SCROLL,
+ -1 * e->value * DEFAULT_AXIS_STEP_DISTANCE);
+ break;
+ default:
+ break;
+ }
break;
case REL_HWHEEL:
- notify_axis(device->seat,
- time,
- WL_POINTER_AXIS_HORIZONTAL_SCROLL,
- wl_fixed_from_int(e->value));
- break;
+ switch (e->value) {
+ case -1:
+ /* Scroll left */
+ case 1:
+ /* Scroll right */
+ notify_axis(device->seat,
+ time,
+ WL_POINTER_AXIS_HORIZONTAL_SCROLL,
+ e->value * DEFAULT_AXIS_STEP_DISTANCE);
+ break;
+ default:
+ break;
+
+ }
}
}
--
1.7.9.5
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] evdev: Update axis notifications to follow protocol
Signed-off-by: Jonas Ådahl
---
Follow up from
http://lists.freedesktop.org/archives/wayland-devel/2012-September/005596.html
src/evdev.c | 38 +-
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/src/evdev.c b/src/evdev.c
index 8848736..648a6de 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -30,6 +30,8 @@
#include "compositor.h"
#include "evdev.h"
+#define DEFAULT_AXIS_STEP_DISTANCE wl_fixed_from_int(10)
+
void
evdev_led_update(struct evdev_device *device, enum weston_led leds)
{
@@ -161,17 +163,35 @@ evdev_process_relative(struct evdev_device *device,
device->pending_events |= EVDEV_RELATIVE_MOTION;
break;
case REL_WHEEL:
- notify_axis(device->seat,
- time,
- WL_POINTER_AXIS_VERTICAL_SCROLL,
- wl_fixed_from_int(e->value));
+ switch (e->value) {
+ case -1:
+ /* Scroll down */
+ case 1:
+ /* Scroll up */
+ notify_axis(device->seat,
+ time,
+ WL_POINTER_AXIS_VERTICAL_SCROLL,
+ -1 * e->value * DEFAULT_AXIS_STEP_DISTANCE);
+ break;
+ default:
+ break;
+ }
break;
case REL_HWHEEL:
- notify_axis(device->seat,
- time,
- WL_POINTER_AXIS_HORIZONTAL_SCROLL,
- wl_fixed_from_int(e->value));
- break;
+ switch (e->value) {
+ case -1:
+ /* Scroll left */
+ case 1:
+ /* Scroll right */
+ notify_axis(device->seat,
+ time,
+ WL_POINTER_AXIS_VERTICAL_SCROLL,
+ e->value * DEFAULT_AXIS_STEP_DISTANCE);
+ break;
+ default:
+ break;
+
+ }
}
}
--
1.7.9.5
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland v2] protocol: Clarify pointer axis event
Pointer axis events are in the same coordinate space as motion events. Signed-off-by: Jonas Ådahl --- protocol/wayland.xml | 15 +++ 1 file changed, 15 insertions(+) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index 5e56cb8..31243a5 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -864,6 +864,21 @@ Scroll and other axis notifications. + + For scroll events (vertical and horizontal scroll axes), the + value parameter is the length of a vector along the specified + axis in a coordinate space identical to those of motion events, + representing a relative movement along the specified axis. + + For devices that support movements non-parallel to axes multiple + axis events will be emitted. + + When applicable, for example for touch pads, the server can + choose to emit scroll events where the motion vector is + equivalent to a motion event vector. + + When applicable, clients can transform its view relative to the + scroll distance. -- 1.7.9.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] compositor-wayland: Create border after creating the OpenGL context.
--- src/compositor-wayland.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c index d665641..05f21c2 100644 --- a/src/compositor-wayland.c +++ b/src/compositor-wayland.c @@ -843,13 +843,14 @@ wayland_compositor_create(struct wl_display *display, c->base.destroy = wayland_destroy; c->base.restore = wayland_restore; - create_border(c); if (wayland_compositor_create_output(c, width, height) < 0) goto err_display; if (gles2_renderer_init(&c->base) < 0) goto err_display; + create_border(c); + loop = wl_display_get_event_loop(c->base.wl_display); fd = wl_display_get_fd(c->parent.wl_display, update_event_mask, c); -- 1.7.12.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] tests: Remove GLfloat usage.
---
tests/matrix-test.c | 3 +--
tests/surface-test.c | 2 +-
tests/test-client.c | 3 +--
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/tests/matrix-test.c b/tests/matrix-test.c
index 8e9d13f..cc78492 100644
--- a/tests/matrix-test.c
+++ b/tests/matrix-test.c
@@ -22,7 +22,6 @@
#include
#include
-#include
#include
#include
#include
@@ -54,7 +53,7 @@ read_timer(void)
}
static double
-det3x3(const GLfloat *c0, const GLfloat *c1, const GLfloat *c2)
+det3x3(const float *c0, const float *c1, const float *c2)
{
return (double)
c0[0] * c1[1] * c2[2] +
diff --git a/tests/surface-test.c b/tests/surface-test.c
index e8fcb9e..28243b1 100644
--- a/tests/surface-test.c
+++ b/tests/surface-test.c
@@ -30,7 +30,7 @@
TEST(surface_transform)
{
struct weston_surface *surface;
- GLfloat x, y;
+ float x, y;
surface = weston_surface_create(compositor);
weston_surface_configure(surface, 100, 100, 200, 200);
diff --git a/tests/test-client.c b/tests/test-client.c
index 0009a8e..8acbf89 100644
--- a/tests/test-client.c
+++ b/tests/test-client.c
@@ -27,7 +27,6 @@
#include
#include
#include
-#include /* needed for GLfloat */
#include
struct display {
@@ -41,7 +40,7 @@ struct input {
struct wl_seat *seat;
struct wl_pointer *pointer;
struct wl_keyboard *keyboard;
- GLfloat x, y;
+ float x, y;
uint32_t button_mask;
struct surface *pointer_focus;
struct surface *keyboard_focus;
--
1.7.12.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC Wayland 1/2] protocol: double-buffered state for wl_surface
On Wed, 3 Oct 2012 16:14:15 +0300 Pekka Paalanen wrote: > This change is breaks the protocol. Hi all, these two patches are my current plan, and after 2-3 days of careful specification writing, I curse the typo quoted above. ;-) I have implemented the wl_surface.attach double-buffering in weston and demo clients so far. All these protocol changes require some changes in all clients. I will also fix Weston to live up to these specifications. Now is the time to point out problems in the suggested protocol changes, bikeshed the names and terminology, and make sure the language is clear, understandable, and defines all relevant aspects. These changes should fix all the races we might have for a single wl_surface. I can see wl_surface.commit be used also for multi-buffer surfaces and what not. There will be further work to specify and fix shell protocols to be atomic, too, leveraging wl_surface.commit as much as possible. One open question I haven't really thought about yet is the frame callback, should it fall under wl_surface.commit too? Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[RFC Wayland 2/2] protocol: change input and opaque region interface
This change breaks the protocol. Now that set_input_region and set_opaque_region no longer have to describe the region atomically in one request, we can simplify the protocol by not using wl_region for them. Instead, the new requests mark_input and mark_opaque will take a simple rectangle, like damage already does. For describing complex forms clients will just send one rectangle at a time, and the server will accumulate the union of them. The wl_surface.commit request takes care, that no partial data is used. Since the action is now union instead of replace, a way to clear the regions is needed. This is provided by the implicit clear on the first mark_* request after a wl_surface.commit. As nothing uses wl_region interface anymore, it is removed from the core. Signed-off-by: Pekka Paalanen --- protocol/wayland.xml | 98 + 1 files changed, 34 insertions(+), 64 deletions(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index 8b34084..81f4812 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -126,13 +126,6 @@ - - - - Ask the compositor to create a new region. - - - @@ -704,9 +697,9 @@ - - - This request sets the region of the surface that contains + + + This request is used to describe the regions of the surface that have opaque content. The opaque region is an optimization hint for the compositor that lets it optimize out redrawing of content behind opaque regions. Setting an opaque region is not @@ -717,40 +710,52 @@ Opaque region is double-buffered state, see wl_surface.commit. - wl_surface.set_opaque_region changes the pending opaque region. - wl_surface.commit copies the pending region to the current region. + wl_surface.commit copies the pending opaque region to the current region. + The first wl_surface.mark_opaque request after a wl_surface.commit will + clear the pending opaque region before adding the given rectangle. + Further wl_surface.mark_opaque requests just add the given rectangle to + the pending opaque region, until the next wl_surface.commit. Otherwise the pending and current regions are never changed. - The initial value for opaque region is empty. Setting the pending - opaque region has copy semantics, and the wl_region object can be - destroyed immediately. A nil wl_region causes the pending opaque - region to be set to empty. + The initial value for opaque region is empty. The pending opaque + region can be set to empty by sending the first mark_opaque request + after a wl_surface.commit with a zero width and/or height. - + + + + - - - This request sets the region of the surface that can receive - pointer and touch events. Input events happening outside of - this region will try the next surface in the server surface - stack. The compositor ignores the parts of the input region that + + + This request is used to describe the regions of the surface that can + receive pointer and touch events. Input events initiating at + coordinates outside of this region will ignore this surface, and + input device enter/leave events obey the input region rather than the + surface. The compositor ignores the parts of the input region that fall outside of the surface. Input region is double-buffered state, see wl_surface.commit. - wl_surface.set_input_region changes the pending input region. - wl_surface.commit copies the pending region to the current region. + wl_surface.commit copies the pending input region to the current region. + The first wl_surface.mark_input request after a wl_surface.commit will + clear the pending input region before adding the given rectangle. + Further wl_surface.mark_input requests just add the given rectangle to + the pending input region, until the next wl_surface.commit. Otherwise the pending and current regions are never changed. The initial value for input region is infinite. That means the whole - surface will accept input. Setting the pending input region has copy - semantics, and the wl_region object can be destroyed immediately. A - nil wl_region causes the input region to be set to infinite. + surface will accept input. The pending input region can be set to + empty by sending the first mark_input request after a + wl_surface.commit with a zero width and/or height. - + + + + @@ -1143,39 +1148,4 @@ - - - Region. - - - - - Destroy the region. This will invalidate the object id. - - - - - -
[RFC Wayland 1/2] protocol: double-buffered state for wl_surface
This change is breaks the protocol. The current protocol is racy in that updates to surface content and surface state (e.g. damage, input and opaque regions) are not guaranteed to happen at the same time. Due to protocol buffering and handling practices, the issues are very hard to trigger. Committing damage to a surface at arbitrary times makes it hard to track when the wl_buffer is being read by the server, and when it is safe to overwrite (the case of wl_shm with a single buffer reused constantly). This protocol change introduces the concept of double-buffered state. Such state is accumulated and cached in the server, unused, until the final commit request. The surface will receive its new content and apply its new state atomically. A wl_surface.commit request is added to the protocol. This is thought to be more clear, than having wl_surface.attach committing implicitly, and then having another request to commit without attaching, as would be required for a GL app that wants to change e.g. input region without redrawing. When these changes are implemented, clients do not have to worry about ordering damage vs. input region vs. attach vs. ... anymore. Clients set the state in any order they want, and kick it all in with a commit. The interactions between wl_surface.attach, (wl_surface.commit,) wl_buffer.release, and wl_buffer.destroy have been undocumented. Only careful inspection of the compositor code has told when a wl_buffer is free for re-use, especially for wl_shm and wrt. wl_surface.damage. Try to clarify how it all should work, and what happens if the wl_buffer gets destroyed. An additional minor fix: allow NULL argument to wl_surface.set_opaque_region. The wording in the documentation already implied that a nil region is allowed. Signed-off-by: Pekka Paalanen --- protocol/wayland.xml | 121 - 1 files changed, 98 insertions(+), 23 deletions(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index e9f8034..8b34084 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -630,9 +630,37 @@ - Copy the contents of a buffer into this surface. The x and y - arguments specify the location of the new buffers upper left - corner, relative to the old buffers upper left corner. + Set the contents of a buffer into this surface. The x and y + arguments specify the location of the new pending buffer's upper + left corner, relative to the current buffer's upper left corner. In + other words, the x and y, and the width and height of the wl_buffer + together define in which directions the surface's size changes. + + Surface contents are double-buffered state, see wl_surface.commit. + + The initial surface contents are void; there is no content. + wl_surface.attach assigns the given wl_buffer as the pending wl_buffer. + wl_surface.commit applies the pending wl_buffer as the new + surface contents, and the size of the surface becomes the size of + the wl_buffer. The wl_buffer is also kept as pending, until + changed by wl_surface.attach or the wl_buffer is destroyed. + + Committing a pending wl_buffer allows the compositor to read the + pixels in the wl_buffer. The compositor may access the pixels at any + time after the wl_surface.commit request. When the compositor will + not access the pixels anymore, it will send the wl_buffer.release + event. Only after receiving wl_buffer.release, the client may re-use + the wl_buffer. + + Destroying the wl_buffer after wl_buffer.release does not change the + surface contents, even if the wl_buffer is still pending for the + next commit. In such case, the next commit does not change the + surface contents. However, if the client destroys the wl_buffer + before receiving wl_buffer.release, the surface contents become + undefined immediately. + + Only if wl_surface.attach is sent with a nil wl_buffer, the + following wl_surface.commit will remove the surface content. @@ -642,10 +670,21 @@ - After attaching a new buffer, this request is used to describe - the regions where the new buffer is different from the - previous buffer and needs to be repainted. Coordinates are - relative to the new buffer. + This request is used to describe the regions where the pending + buffer (or if pending buffer is none, the current buffer as updated + in-place) on the next wl_surface.commit will be different from the + current buffer, and needs to be repainted. The pending buffer can be + set by wl_surface.attach. The compositor ignores the parts of the + damage that fall outside of the surface. + + Damage is double-buffered state, see wl_surface.commit. + + The initial value for pending damage is empty: no damage. + wl_surfac
Re: [PATCH libxkbcommon v2 1/2] makekeys: use GNU gperf to generate perfect hashtables
Hi David,
On Tue, Oct 02, 2012 at 07:51:53PM +0200, David Herrmann wrote:
> Instead of using a home-brew hashtable generator, we should instead use
> the gperf program which is known to work.
>
> This removes the "makekeys" programs and instead replaces it by a file
> that can generate input files for gperf. Gperf then generates hashtables
> for all of these input files and writes them concatenated into
> ks_tables.h which then can be used from keysym.c
>
> Unfortunately, gperf does not support integer keys but only strings or
> binary data. Therefore, we have to make the keysym->name lookup
> little-endian to avoid errors during cross compilation.
>
> Signed-off-by: David Herrmann
> ---
The code is really nice, I have no comments on it.
I noticed though that it really blows up our binary size. Here's size -A
(only the relevant sections, CFLAGS=-O2) of current master:
sectionsize addr
.text11070810608
.rodata 95684 121344
.data.rel.ro 2192 237728
Total238568
Here it is after adding the third table to old makekeys, in the v1 patch
you sent:
sectionsize addr
.text11072410608
.rodata 122340 121376
.data.rel.ro 2192 264416
Total265240
And here it is with these patches:
sectionsize addr
.text11278828912
.rodata 716478 141728
.data.rel.ro 55824 879136
Total933614
So gperf is clearly doing... something here. In the gperf manual they
mention:
The size of the generate static keyword array can get extremely large
if the input keyword file is large or if the keywords are quite
similar. This tends to slow down the compilation of the generated C
code, and greatly inflates the object code size. If this situation
occurs, consider using the ‘-S’ option to reduce data size, potentially
increasing keyword recognition time a negligible amount. Since many C
compilers cannot correctly generate code for large switch statements it
is important to qualify the -S option with an appropriate numerical
argument that controls the number of switch statements generated.
To reduce the size I tried removing %compare-length from the name-to-key
tables (which helped a bit). Also tried using a few %switch numbers (and
thus let gcc create the lookup tables on its own), which reduced it to
about 55, but then the compilation takes about a minute :)
So to be honest, the hashing that gperf and makekeys do is nice, but I
don't see why we do it anyway, if it complicates thing. For example, I
just took 15 minutes to do it in the obvious way of creating simple
sorted {name, keysym} arrays and doing binary search on them, to replace
the current makekeys code (see attached patch - just a proof of concept
hacked up python script and a couple bsearch's). I don't see any
performance issues, and the size is:
.text11051648016
.rodata 66366 158560
.data.rel.ro 39568 241696
Total283860
With adding the third table it is:
.text11054867184
.rodata 80278 177760
.data.rel.ro 58768 278752
Total336180
So since makekeys is ugly and gperf is a bit excessive, maybe we should
just keep it simple, what do you think?
Ran
>From 8fb5efb045b7207b010c979cbeae8f8222759961 Mon Sep 17 00:00:00 2001
From: Ran Benita
Date: Wed, 3 Oct 2012 10:09:48 +0200
Subject: [PATCH libxkbcommon] Replace makekeys with python script + binary
search
Signed-off-by: Ran Benita
---
Makefile.am | 9 +-
configure.ac | 14 +--
makekeys.py | 23
makekeys/.gitignore | 1 -
makekeys/Makefile.am | 10 --
makekeys/makekeys.c | 302 ---
src/keysym.c | 97 ++---
7 files changed, 62 insertions(+), 394 deletions(-)
create mode 100644 makekeys.py
delete mode 100644 makekeys/.gitignore
delete mode 100644 makekeys/Makefile.am
delete mode 100644 makekeys/makekeys.c
diff --git a/Makefile.am b/Makefile.am
index 26646fb..dfbd8d9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,7 +1,5 @@
ACLOCAL_AMFLAGS = -I m4
-SUBDIRS = makekeys
-
pkgconfigdir = $(libdir)/pkgconfig
pkgconfig_DATA = xkbcommon.pc
@@ -92,11 +90,8 @@ src/xkbcomp/parser.c: $(top_builddir)/src/$(am__dirstamp)
$(top_builddir)/src/xk
src/xkbcomp/parser.h: $(top_builddir)/src/$(am__dirstamp)
$(top_builddir)/src/xkbcomp/$(am__dirstamp)
src/xkbcomp/scanner.c: $(top_builddir)/src/$(am__dirstamp)
$(top_builddir)/src/xkbcomp/$(am__dirstamp)
-src/ks_tables.h: $(top_builddir)/makekeys/makekeys$(EXEEXT)
- $(AM_V_GEN)$(top_builddir)/makekeys/makekeys
$(top_srcdir)/xkbcommon/xkbcommon-keysyms.h > $@
-
-$(top_builddir)/makekeys/makekeys$(EXEEXT): $(top_srcdir)/makekeys/makekeys.c
- $(MAKE) -C makekey
