Re: [xkbcommon] Use an integer type for modifiers bit mask.
On quinta-feira, 3 de outubro de 2013 23:03:44, Wander Lairson Costa wrote: > If we combine two enum values, the result is not a valid enum value > anymore, so it cannot be attributed to an enum variable. > > C++ compilers will complain if such an assigment is done. This is done all the time in C++. The enum must be backed by an integer with at least as many bits as the enum possesses. With the ABI that GCC uses, it's always at least 4 bytes. The only thing is that you need to cast it from integer back to the enum type. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC DRAFT] graphics tablet protocol extension
On Thu, Oct 03, 2013 at 03:44:51PM +0200, David Herrmann wrote: [...] > >> > > >> > @@ -1306,6 +1307,19 @@ > >> > > >> > > >> > > >> > + > >> > + > >> > + > >> > +The ID provided will be initialized to the wl_tablet_manager > >> > +interface for this seat. This can then be used to retrieve the > >> > +objects representing the actual tablet devices. > >> > + > >> > +This request only takes effect if the seat has the tablets > >> > +capability. > >> > + > >> > + > >> > + > >> > + > >> > > >> > > >> > > >> > @@ -1617,6 +1631,223 @@ > >> > > >> > > >> > > >> > + > >> > + > >> > + A tablet manager object provides requests to access the graphics > >> > + tablets available on this system. > >> > + > >> > + > >> > + > >> > + > >> > +Describes the type of tablet. > >> > + > >> > + > >> > + > >> > + > >> > + > >> > + > >> > + > >> > + > >> > + > >> > + > >> > + > >> > + > >> > + > >> > >> So tablets are server-side created objects? I think we tried to avoid > >> that. Only wl_display uses this now, afaik. Even for globals we added > >> the wl_registry interface which just announces unique names which the > >> client then binds to via wl_registry.bind. So why not turn > >> wl_tablet_manager into wl_tablet_registry and provide the same > >> functions as the global registry? > > > > two things: > > having thought about this (and talked to krh and daniels at XDC) the tablet > > manager shouldn't be necessary anymore. It used to have the device_removed > > event, but now it's just an intermediate layer that can be removed by moving > > the device_added event up into the seat and sending it conditional on either > > get_tablets or on binding to the seat with the correct version. > > That sounds much better. I like the approach with advertising tablets > on the seat via "tabled_added" events. I don't even think it needs to > be conditional. Older clients just ignore it which seems fine. > > > > > as for moving tablets over to a global: my first attempt was to have > > wl_tablet_manager as a global. the server-side objects stay in that case, if > > you bind to the manager it sends you events about the tablets. so for three > > tablets and the client wanting tablet 2 it could look like that: > > > > wl_tablet_manager::bind > >event:device added(1) > >event:device added(2) > >event:device added(3) > >request:get_device(some client id, 2) > > > > at this point you still need some sort of ID to identify the tablet when the > > client request comes, so you might as well use server-allocated IDs and let > > the client discard what it doesn't want: > > > > wl_tablet_manager::bind > >event:device added(server id 1) > >event:device added(server id 2) > >event:device added(server id 3) > >request:release (server id 1) > >request:release (server id 3) > > > > so I don't know how to get rid of server-allocated IDs. > > Ouh, you always need server-allocated IDs, but not server-allocated > names. What wl_registry does is to provide its own ID namespace for > globals but they are unrelated to Wayland-protocol IDs and just > identify the global. So for tablets you could even reuse the > underlying input-device ID (or event evdev ID) if you wanted. A simple > counter would be enough, though. A client-side "get_tablet(tablet-id, > client-id)" call with the given tablet ID would then allow the client > to create the object itself. once again that was my first approach but then you have a internal ID mapping that is IMO unnecessary. if we need to assign an ID to send to the client so the client assigns another ID to init the device - why not allocate the final ID in the server. Plus, you then have to handle the race conditions where the device disappears before the client assigns an ID to it and you can't handle that through the object directly. > The advantage of your first example is that the client controls the > namespace (client side namespace is _much_ bigger than server-side). > Especially for hotpluggable devices we don't want to risk cluttering > the server-side ID namespace as we have no control over how many of > these may exist. (compared to wl_seat objects which are rather static > and thus could easily be server-side allocated). is ID allocation for a handful of devices really a problem? > > wl_global used to be a server-allocated API pre 1.0 then krh changed > it to wl_registry. I don't know his exact reasons but I think it's to > try to avoid server-side allocated global IDs at all. But obviously we > should ask him. This is just my understanding of it. > > > as said above, the tablet_manager doesn't really provide that much > > functionality at this point, so another alternative is to have wl_tablet > > directly as global, but I don'
Re: idle tasks starving in toytoolkit
On 27 September 2013 23:34, Daniel Stone wrote: > > Hi, > > On 27 September 2013 05:38, Neil Roberts wrote: > > Pekka Paalanen writes: > >> If not, is there not a possibility to break existing applications by > >> blocking too early? > > > > Yes, you're right, the patch is nonsense because it won't work when the > > application does wl_display_dispatch_pending because it might end up > > with some events still in the queue but the poll won't wake up to > > process them. > > Indeed, it doesn't solve the original problem at all, because you just > have to keep dispatching randomly and hope for the best. > > > It would be nice if the recommended main loop was more like this: > > > > [snip horrible unpleasantness] > > > > That way it doesn't matter if wl_display_dispatch_pending doesn't clear > > all of the events. > > Ugh. I really don't like the look of that; would be nice to have a > wl_display_dispatch_some_subset_of_pending(), which would return > failure / dispatched everything / still stuff left to dispatch. But I > worry this takes us into libdbus API design territory ... Or we can just document that causing a connection read from within an event handler can cause wl_display_dispatch to never exit, or even raise an error and exit if we detect a nested call to wl_display_dispatch_queue. It's maybe only demos what are drawing directly from the frame callback, but there's lots of them and it will confuse people. Regards, Tomeu ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [xkbcommon] Use an integer type for modifiers bit mask.
2013/10/4 Thiago Macieira : > On quinta-feira, 3 de outubro de 2013 23:03:44, Wander Lairson Costa wrote: >> If we combine two enum values, the result is not a valid enum value >> anymore, so it cannot be attributed to an enum variable. >> >> C++ compilers will complain if such an assigment is done. > > This is done all the time in C++. The enum must be backed by an integer with > at least as many bits as the enum possesses. > The issued raised when I took code from window.c in the weston clients: mask = xkb_state_serialize_mods(input->xkb.state, XKB_STATE_DEPRESSED | XKB_STATE_LATCHED); g++ gave me a build error because I was passing an integer to enum parameter. C++ is a bit more nit-picky than C regarding implicit conversions. Therefore I had to use a cast. > With the ABI that GCC uses, it's always at least 4 bytes. Personally, I don't like enum's in the ABI at all, because according to C/C++ standards, the compiler is free to choose whatever type it likes, and indeed I had problems with that in the past. C++11 fixed that [1]. But nevermind. > The only thing is that you need to cast it from integer back to the enum type. > That's what the patch is about: avoid casts. Whenever you use a cast, you are giving up the help the compiler may give to you regarding invalid type conversions. So, I always use the rule of thumb of avoiding casts whenever I can. IMO, this is not a harmful patch and will make the C++ programmers a little bit easier. Ofcourse the libxkbcommon maintainers may have another view and reject the patch. [1] http://www.cprogramming.com/c++11/c++11-nullptr-strongly-typed-enum-class.html -- Best Regards, Wander Lairson Costa ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [xkbcommon] Use an integer type for modifiers bit mask.
Hi, On 4 October 2013 13:09, Wander Lairson Costa wrote: > The issued raised when I took code from window.c in the weston clients: > > mask = xkb_state_serialize_mods(input->xkb.state, > XKB_STATE_DEPRESSED | > XKB_STATE_LATCHED); > > g++ gave me a build error because I was passing an integer to enum > parameter. C++ is a bit more nit-picky than C regarding implicit > conversions. Therefore I had to use a cast. One of so very many reasons I hate C++. But, unfortunately in the real world, we have to support people building with C++, so ... >> With the ABI that GCC uses, it's always at least 4 bytes. > > Personally, I don't like enum's in the ABI at all, because according > to C/C++ standards, the compiler is free to choose whatever type it > likes, and indeed I had problems with that in the past. C++11 fixed > that [1]. But nevermind. Well sure, but we took the added benefit of type safety (at least with C compilers) over the the small potential downside. I was fully aware of the technical point there, but don't see it as much of a risk at all. >> The only thing is that you need to cast it from integer back to the enum >> type. > > That's what the patch is about: avoid casts. Whenever you use a cast, > you are giving up the help the compiler may give to you regarding > invalid type conversions. So, I always use the rule of thumb of > avoiding casts whenever I can. IMO, this is not a harmful patch and > will make the C++ programmers a little bit easier. I can see where it's going, but on the other hand xkb_mod_mask_t is _so_ not the right type. That's a bitmask of actual modifiers (i.e. the return value), not a bitmask of components. So it avoids the cast, but also makes the API look extremely misleading, as we've documented exactly what xkb_mod_mask_t is. I really like the type safety aspect, so to be honest I'm just inclined to make C++ wear the cast for now. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [xkbcommon] Use an integer type for modifiers bit mask.
Hi On Fri, Oct 4, 2013 at 2:34 PM, Daniel Stone wrote: > On 4 October 2013 13:09, Wander Lairson Costa > wrote: >> That's what the patch is about: avoid casts. Whenever you use a cast, >> you are giving up the help the compiler may give to you regarding >> invalid type conversions. So, I always use the rule of thumb of >> avoiding casts whenever I can. IMO, this is not a harmful patch and >> will make the C++ programmers a little bit easier. > > I can see where it's going, but on the other hand xkb_mod_mask_t is > _so_ not the right type. That's a bitmask of actual modifiers (i.e. > the return value), not a bitmask of components. So it avoids the > cast, but also makes the API look extremely misleading, as we've > documented exactly what xkb_mod_mask_t is. > > I really like the type safety aspect, so to be honest I'm just > inclined to make C++ wear the cast for now. We had a similar discussion when I pointed out that "enum" may change the underlying type without notice when adding new enum-values. But I agree with Daniel, the GCC type-safety for enums in C is a very handy feature. So I'm also no big fan of the patch, sorry. Thanks David ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [xkbcommon] Use an integer type for modifiers bit mask.
Hi, On 4 October 2013 14:06, Wander Lairson Costa wrote: > 2013/10/4 Daniel Stone : >> Well sure, but we took the added benefit of type safety (at least with >> C compilers) over the the small potential downside. I was fully aware >> of the technical point there, but don't see it as much of a risk at >> all. > > The problem can arise when you compile the lib with one compiler, and > try to use it with another one. The was exactly the problem I had. I > was responsible for an USB library that, on Windows, I used to build > with Visual C++, but a client was using C++ Builder. Couple of hours > with the disassembly and I discovered that Visual C++ chose a 32 bits > types to represent the enum, but C++ Builder chose a 8 bits type... Yeah, I'm aware of the risk ... luckily we target systems with more coherent toolchains. ;) But still, we'll cross that bridge when we come to it. >> I really like the type safety aspect, so to be honest I'm just >> inclined to make C++ wear the cast for now. > > If you change your mind, I can resubmit the patch with the integer type fixed > :) Well, we'd just be creating integer types and using them everywhere. enum/integer conversions in C++ are not always cast-free either, and all it means is that we'd be throwing away the type safety we have today, and still not necessarily getting rid of the casts. The reason I went for this is that the names can be confusing, and I'd like to warn people very visibly and immediately when they get something wrong. Demoting everything to an integer removes that, so I'd like to see the users who are hitting this just deal with the cast. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] server: Add API to protect access to an SHM buffer
Neil,
While I think this patch will work in the single-threaded case, I'm not a
big fan of the api for two reasons: 1) without doing thread-local things it
is inherently single-threaded. (This is obviously not an issue for Weston,
but requiring all shm buffer reads to be on the Wayland fd thread could be
a big issue for others.) 2) it doesn't allow for integration with another
SIGBUS/SEGEV handler.
My knowledge of thread-local stuff isn't great, so (1) may be a non-issue
in the case where we're not fighting with another signal handler. However,
I do think it would be good to provide better integration with other
handlers. Perhaps a function that takes a "bad" pointer and checks to see
if it is in an active pool?
Thanks,
--Jason Ekstrand
On Sep 30, 2013 6:52 PM, "Neil Roberts" wrote:
> Linux will let you mmap a region of a file that is larger than the
> size of the file. If you then try to read from that region the process
> will get a SIGBUS signal. Currently the clients can use this to crash
> a compositor because it can create a pool and lie about the size of
> the file which will cause the compositor to try and read past the end
> of it. The compositor can't simply check the size of the file to
> verify that it is big enough because then there is a race condition
> where the client may truncate the file after the check is performed.
>
> This patch adds the following two public functions in the server API
> which can be used wrap access to an SHM buffer:
>
> void wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer);
> void wl_shm_buffer_end_access(struct wl_shm_buffer *buffer);
>
> In-between calls to these functions there will be a signal handler for
> SIGBUS. If the signal is caught then the buffer is remapped to an
> anonymous private buffer at the same address which allows the
> compositor to continue without crashing. The end_access function will
> then post an error to the buffer resource.
> ---
>
> This problem was pointed out earlier by wm4 on #wayland and it seems
> like quite a thorny issue. Here is a first attempt at a patch which
> does seem to work for Weston but I think there are plenty of issues to
> think about. It might not be such a good idea to be messing with
> signal handlers if the compositor itself also wants to handle SIGBUS.
> There are probably some threading concerns here too.
>
> I think you could do the patch without adding any extra API and just
> make it automatically work out which region to remap if you had a list
> of wl_shm_pools. The signal handler gets passed the address so it
> could check which pool contains it and just remap that one. However
> the end_access function seems like a nice place to post an event back
> to the client when it fails so I'm not sure which way is better. It's
> also probably a good idea to keep the signal handler as simple as
> possible.
>
> src/wayland-server.h | 6
> src/wayland-shm.c| 87
>
> 2 files changed, 93 insertions(+)
>
> diff --git a/src/wayland-server.h b/src/wayland-server.h
> index c93987d..b371c9e 100644
> --- a/src/wayland-server.h
> +++ b/src/wayland-server.h
> @@ -412,6 +412,12 @@ wl_resource_get_destroy_listener(struct wl_resource
> *resource,
>
> struct wl_shm_buffer;
>
> +void
> +wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer);
> +
> +void
> +wl_shm_buffer_end_access(struct wl_shm_buffer *buffer);
> +
> struct wl_shm_buffer *
> wl_shm_buffer_get(struct wl_resource *resource);
>
> diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> index eff29c3..95b1f33 100644
> --- a/src/wayland-shm.c
> +++ b/src/wayland-shm.c
> @@ -32,15 +32,22 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #include "wayland-private.h"
> #include "wayland-server.h"
>
> +static struct wl_shm_pool *wl_shm_current_pool;
> +static struct sigaction wl_shm_old_sigbus_action;
> +
> struct wl_shm_pool {
> struct wl_resource *resource;
> int refcount;
> char *data;
> int size;
> + int access_count;
> + int fallback_mapping_used;
> };
>
> struct wl_shm_buffer {
> @@ -219,6 +226,7 @@ shm_create_pool(struct wl_client *client, struct
> wl_resource *resource,
>
> pool->refcount = 1;
> pool->size = size;
> + pool->fallback_mapping_used = 0;
> pool->data = mmap(NULL, size,
> PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> if (pool->data == MAP_FAILED) {
> @@ -368,3 +376,82 @@ wl_shm_buffer_get_height(struct wl_shm_buffer *buffer)
> {
> return buffer->height;
> }
> +
> +static void
> +unset_sigbus_handler(void)
> +{
> + sigaction(SIGBUS, &wl_shm_old_sigbus_action, NULL);
> +}
> +
> +static void
> +sigbus_handler(int signum, siginfo_t *info, void *context)
> +{
> + struct wl_shm_pool *pool = wl_shm_current_pool;
> +
> + unset_sigbus_handler();
> +
> + /* If the offending address is outside the mapped space for
> +
Re: [PATCH] client: Add a way to get a pointer to the display's default queue
Hi,
I have find myself needing this in order to make sure that wl_buffers
are destroyed when the wl_surface is, but not before the compositor
releases them.
So, the client app would be calling wl_surface_destroy, but as the
front wl_buffer is still in use by the compositor, the EGL
implementation will need to defer the destruction until the
wl_buffer.release event comes. But the wl_buffers are using the EGL
implementation's queue and only the main queue is spinning right now,
so the release wouldn't be dispatched ever and the buffer would be
destroyed only after the client is disconnected.
Mesa doesn't seem to care about it and destroys the wl_buffers right
away, which also destroys the underlying images, but I guess with
most/all DRM drivers the buffers are refcounted and it doesn't matter.
With Neil's patch I can set the wl_buffer's queue to be the main one
when the client schedules the destroy, so I eventually get the
wl_buffer.release event.
Regards,
Tomeu
On 10 September 2013 15:47, Neil Roberts wrote:
> Adds a function called wl_display_get_main_queue which just returns a
> pointer to the default event queue. This will be useful in order to
> implement the EGL_WL_create_wayland_buffer_from_image extension. The
> buffers created within Mesa's Wayland platform are created using the
> the wl_drm object as a proxy factory which means they will be set to
> use Mesa's internal event queue. However, these buffers will be owned
> by the client application so they ideally need to use the default
> event loop. This function provides a way to set the proxy's event
> queue back to the default.
> ---
> src/wayland-client.c | 14 ++
> src/wayland-client.h | 1 +
> 2 files changed, 15 insertions(+)
>
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index 04d988b..48f06c7 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -194,6 +194,20 @@ wl_display_create_queue(struct wl_display *display)
> return queue;
> }
>
> +/** Get the main event queue for this display
> + *
> + * \param display The display context object
> + * \return A pointer to the main default event queue used with this
> + * display.
> + *
> + * \memberof wl_display
> + */
> +WL_EXPORT struct wl_event_queue *
> +wl_display_get_main_queue(struct wl_display *display)
> +{
> + return &display->queue;
> +}
> +
> /** Create a proxy object with a given interface
> *
> * \param factory Factory proxy object
> diff --git a/src/wayland-client.h b/src/wayland-client.h
> index cf92174..8cf6847 100644
> --- a/src/wayland-client.h
> +++ b/src/wayland-client.h
> @@ -157,6 +157,7 @@ int wl_display_get_error(struct wl_display *display);
> int wl_display_flush(struct wl_display *display);
> int wl_display_roundtrip(struct wl_display *display);
> struct wl_event_queue *wl_display_create_queue(struct wl_display *display);
> +struct wl_event_queue *wl_display_get_main_queue(struct wl_display *display);
>
> int wl_display_prepare_read_queue(struct wl_display *display,
> struct wl_event_queue *queue);
> --
> 1.8.3.1
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v2] Post buffer release events instead of queue when no frame callback
Neil Roberts writes: > I think that would mean you could cause tearing if you are using > eglSwapInterval(0) because you could write into the released buffer > while the GPU is actually still rendering the previous frame using the > buffer in a texture. > > I think this doesn't actually happen at the moment because as far as I > can tell the gl-renderer immediately queues the buffer release when you > attach a new one rather than waiting for the swap to actually > complete. Thanks to some explanation from Kristian and Pekka I now understand that this is the expected behaviour. The assumption is that it will be up to the GL driver or hardware to ensure that GL commands submitted by the client will be executed after those submitted by the compositor so the compositor is free to release straight after the eglSwapBuffer call. If the hardware doesn't just do this naturally then it would still be possible for a driver's buffer sharing implementation to also transfer fence objects so that it can ensure these semantics in the driver. Regards, - Neil - Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [xkbcommon] Use an integer type for modifiers bit mask.
2013/10/4 David Herrmann : > Hi > > On Fri, Oct 4, 2013 at 2:34 PM, Daniel Stone wrote: >> On 4 October 2013 13:09, Wander Lairson Costa >> wrote: >>> That's what the patch is about: avoid casts. Whenever you use a cast, >>> you are giving up the help the compiler may give to you regarding >>> invalid type conversions. So, I always use the rule of thumb of >>> avoiding casts whenever I can. IMO, this is not a harmful patch and >>> will make the C++ programmers a little bit easier. >> >> I can see where it's going, but on the other hand xkb_mod_mask_t is >> _so_ not the right type. That's a bitmask of actual modifiers (i.e. >> the return value), not a bitmask of components. So it avoids the >> cast, but also makes the API look extremely misleading, as we've >> documented exactly what xkb_mod_mask_t is. >> >> I really like the type safety aspect, so to be honest I'm just >> inclined to make C++ wear the cast for now. > > We had a similar discussion when I pointed out that "enum" may change > the underlying type without notice when adding new enum-values. But I > agree with Daniel, the GCC type-safety for enums in C is a very handy > feature. So I'm also no big fan of the patch, sorry. > We have conflictings pros and cons here, with different tradeoffs, I perfectly understand that. -- Best Regards, Wander Lairson Costa ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [xkbcommon] Use an integer type for modifiers bit mask.
On Fri, Oct 04, 2013 at 03:22:59PM +0200, David Herrmann wrote: > Hi > > On Fri, Oct 4, 2013 at 2:34 PM, Daniel Stone wrote: > > On 4 October 2013 13:09, Wander Lairson Costa > > wrote: > >> That's what the patch is about: avoid casts. Whenever you use a cast, > >> you are giving up the help the compiler may give to you regarding > >> invalid type conversions. So, I always use the rule of thumb of > >> avoiding casts whenever I can. IMO, this is not a harmful patch and > >> will make the C++ programmers a little bit easier. > > > > I can see where it's going, but on the other hand xkb_mod_mask_t is > > _so_ not the right type. That's a bitmask of actual modifiers (i.e. > > the return value), not a bitmask of components. So it avoids the > > cast, but also makes the API look extremely misleading, as we've > > documented exactly what xkb_mod_mask_t is. > > > > I really like the type safety aspect, so to be honest I'm just > > inclined to make C++ wear the cast for now. > > We had a similar discussion when I pointed out that "enum" may change > the underlying type without notice when adding new enum-values. Just adding a note that this problem can be mitigated by adding some dummy value to the enum to force it to be int-sized. Of course we can't do it in xkbcommon, but future APIs may want to do this, at least if they don't find it too ugly. Ran ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [xkbcommon] Use an integer type for modifiers bit mask.
On sexta-feira, 4 de outubro de 2013 09:09:32, Wander Lairson Costa wrote: > The issued raised when I took code from window.c in the weston clients: > > mask = xkb_state_serialize_mods(input->xkb.state, > XKB_STATE_DEPRESSED | > XKB_STATE_LATCHED); > > g++ gave me a build error because I was passing an integer to enum > parameter. C++ is a bit more nit-picky than C regarding implicit > conversions. Therefore I had to use a cast. Like you said, it's window.c. Why are you copy & pasting C code into a C++ file? > > With the ABI that GCC uses, it's always at least 4 bytes. > > Personally, I don't like enum's in the ABI at all, because according > to C/C++ standards, the compiler is free to choose whatever type it > likes, and indeed I had problems with that in the past. C++11 fixed > that [1]. But nevermind. We're not relying on the ABI. I said "The enum must be backed by an integer with at least as many bits as the enum possesses.". If you cast back from an int that contains one of the enum values or an OR combination of enum values, it *will* fit. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Buffer release events (was: Add support for eglSwapInterval)
Here is a quick summary of the discussion about the release event handling that has partially happened on this list and partially on IRC. Currently the release events are put into a queue so that they are only sent alongside another event sent by the compositor in order to avoid waking up the client more times than necessary. Usually this will end up being sent alongside the frame complete callback. The assumption is that most clients don't care about the buffer release events until they are about to draw something, which usually only happens when they get the frame callback so they don't need to wake up early. This assumption doesn't work if the client isn't installing a frame callback for example when doing eglSwapInterval(0). In that case the client may be blocked waiting for a buffer release event but the queue will never get flushed because no other events are being sent from the compositor. It could also happen if the compositor is not sending the release events until some time after the frame callback completes, for example if it waits until a pending swap buffer completes before releasing the buffer. Currently this doesn't happen for normal clients because the GL renderer immediately releases the buffer when a new one is attached but it may happen for fullscreen clients. One solution is to always post the release event instead of queuing whenever the client doesn't have a frame callback currently installed. This fixes the clients doing eglSwapInterval(0) without breaking all the normal clients. However this still isn't a perfect solution because it's still possible that a client would want to do eglSwapInterval(0) and render as fast as possible but still install its own frame callback, perhaps just to get information about how fast the frames are being displayed on the screen. Currently the only proposed complete solution is just to add a request to explicitly disable the queuing mechanism. This would have to be a request on the surface rather than being hidden away as part of wl_drm because the compositor needs to know about it. Mesa could implicitly send this request whenever eglSwapInterval is called. As far as I understand the explicit toggle is the current expected plan although it does feel a bit like a kludge so we may want to sit on it a bit in case we can think of something better. Regards, - Neil - Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [xkbcommon] Use an integer type for modifiers bit mask.
2013/10/4 Thiago Macieira : > On sexta-feira, 4 de outubro de 2013 09:09:32, Wander Lairson Costa wrote: >> The issued raised when I took code from window.c in the weston clients: >> >> mask = xkb_state_serialize_mods(input->xkb.state, >> XKB_STATE_DEPRESSED | >> XKB_STATE_LATCHED); >> >> g++ gave me a build error because I was passing an integer to enum >> parameter. C++ is a bit more nit-picky than C regarding implicit >> conversions. Therefore I had to use a cast. > > Like you said, it's window.c. Why are you copy & pasting C code into a C++ > file? > Well, weston clients are the only code reference I had in hand. Sometimes I go to qtwayland also, but it did not run away from the casts... >> > With the ABI that GCC uses, it's always at least 4 bytes. >> >> Personally, I don't like enum's in the ABI at all, because according >> to C/C++ standards, the compiler is free to choose whatever type it >> likes, and indeed I had problems with that in the past. C++11 fixed >> that [1]. But nevermind. > > We're not relying on the ABI. I said "The enum must be backed by an integer > with at least as many bits as the enum possesses.". If you cast back from an > int that contains one of the enum values or an OR combination of enum values, > it *will* fit. I think I caused more confusion than explained here. My original point was not if the value fits or not, but that once you do whatever math with the enum, the result is not an enum type anymore (yes, sounds nit-picky)... -- Best Regards, Wander Lairson Costa ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [xkbcommon] Use an integer type for modifiers bit mask.
#include
enum Enum {ZERO, A, B, C, D};
Enum operator|(Enum a, Enum b) { return Enum(int(a)|int(b)); }
int main() {
Enum a(A);
Enum b(B);
Enum c = a|b;
std::cout << a << ',' << b << ',' << c << std::endl;
}
Wander Lairson Costa wrote:
2013/10/4 Thiago Macieira :
On sexta-feira, 4 de outubro de 2013 09:09:32, Wander Lairson Costa wrote:
The issued raised when I took code from window.c in the weston clients:
mask = xkb_state_serialize_mods(input->xkb.state,
XKB_STATE_DEPRESSED |
XKB_STATE_LATCHED);
g++ gave me a build error because I was passing an integer to enum
parameter. C++ is a bit more nit-picky than C regarding implicit
conversions. Therefore I had to use a cast.
Like you said, it's window.c. Why are you copy & pasting C code into a C++
file?
Well, weston clients are the only code reference I had in hand.
Sometimes I go to qtwayland also, but it did not run away from the
casts...
With the ABI that GCC uses, it's always at least 4 bytes.
Personally, I don't like enum's in the ABI at all, because according
to C/C++ standards, the compiler is free to choose whatever type it
likes, and indeed I had problems with that in the past. C++11 fixed
that [1]. But nevermind.
We're not relying on the ABI. I said "The enum must be backed by an integer
with at least as many bits as the enum possesses.". If you cast back from an
int that contains one of the enum values or an OR combination of enum values,
it *will* fit.
I think I caused more confusion than explained here. My original point
was not if the value fits or not, but that once you do whatever math
with the enum, the result is not an enum type anymore (yes, sounds
nit-picky)...
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] input: set the focus to NULL when the focus's resource is destroyed
with the surface ref-count feature a surface may live on after its
resource was destroyed. so listen for the resource destroy signal
and set the focus to NULL.
---
src/compositor.c | 4
src/compositor.h | 3 +++
src/input.c | 52 +++-
3 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/src/compositor.c b/src/compositor.c
index 9757a75..6b38e38 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -1148,6 +1148,10 @@ destroy_surface(struct wl_resource *resource)
{
struct weston_surface *surface = wl_resource_get_user_data(resource);
+ /* Set the resource to NULL, since we don't want to leave a
+* dangling pointer if the surface was refcounted and survives
+* the weston_surface_destroy() call. */
+ surface->resource = NULL;
weston_surface_destroy(surface);
}
diff --git a/src/compositor.h b/src/compositor.h
index 5257356..1408110 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -312,6 +312,7 @@ struct weston_pointer {
struct wl_list resource_list;
struct wl_list focus_resource_list;
struct weston_surface *focus;
+ struct wl_listener focus_listener;
uint32_t focus_serial;
struct wl_signal focus_signal;
@@ -337,6 +338,7 @@ struct weston_touch {
struct wl_list resource_list;
struct wl_list focus_resource_list;
struct weston_surface *focus;
+ struct wl_listener focus_listener;
uint32_t focus_serial;
struct wl_signal focus_signal;
@@ -431,6 +433,7 @@ struct weston_keyboard {
struct wl_list resource_list;
struct wl_list focus_resource_list;
struct weston_surface *focus;
+ struct wl_listener focus_listener;
uint32_t focus_serial;
struct wl_signal focus_signal;
diff --git a/src/input.c b/src/input.c
index 1313b52..644f30a 100644
--- a/src/input.c
+++ b/src/input.c
@@ -71,6 +71,33 @@ weston_compositor_idle_release(struct weston_compositor
*compositor)
}
static void
+lose_pointer_focus(struct wl_listener *listener, void *data)
+{
+ struct weston_pointer *pointer =
+ container_of(listener, struct weston_pointer, focus_listener);
+
+ weston_pointer_set_focus(pointer, NULL, 0, 0);
+}
+
+static void
+lose_keyboard_focus(struct wl_listener *listener, void *data)
+{
+ struct weston_keyboard *keyboard =
+ container_of(listener, struct weston_keyboard, focus_listener);
+
+ weston_keyboard_set_focus(keyboard, NULL);
+}
+
+static void
+lose_touch_focus(struct wl_listener *listener, void *data)
+{
+ struct weston_touch *touch =
+ container_of(listener, struct weston_touch, focus_listener);
+
+ weston_touch_set_focus(touch->seat, NULL);
+}
+
+static void
move_resources(struct wl_list *destination, struct wl_list *source)
{
wl_list_insert_list(destination, source);
@@ -360,6 +387,8 @@ weston_pointer_create(void)
wl_list_init(&pointer->resource_list);
wl_list_init(&pointer->focus_resource_list);
+ wl_list_init(&pointer->focus_listener.link);
+ pointer->focus_listener.notify = lose_pointer_focus;
pointer->default_grab.interface = &default_pointer_grab_interface;
pointer->default_grab.pointer = pointer;
pointer->grab = &pointer->default_grab;
@@ -381,7 +410,7 @@ weston_pointer_destroy(struct weston_pointer *pointer)
pointer_unmap_sprite(pointer);
/* XXX: What about pointer->resource_list? */
-
+ wl_list_remove(&pointer->focus_listener.link);
free(pointer);
}
@@ -396,6 +425,8 @@ weston_keyboard_create(void)
wl_list_init(&keyboard->resource_list);
wl_list_init(&keyboard->focus_resource_list);
+ wl_list_init(&keyboard->focus_listener.link);
+ keyboard->focus_listener.notify = lose_keyboard_focus;
wl_array_init(&keyboard->keys);
keyboard->default_grab.interface = &default_keyboard_grab_interface;
keyboard->default_grab.keyboard = keyboard;
@@ -411,6 +442,7 @@ weston_keyboard_destroy(struct weston_keyboard *keyboard)
/* XXX: What about keyboard->resource_list? */
wl_array_release(&keyboard->keys);
+ wl_list_remove(&keyboard->focus_listener.link);
free(keyboard);
}
@@ -425,6 +457,8 @@ weston_touch_create(void)
wl_list_init(&touch->resource_list);
wl_list_init(&touch->focus_resource_list);
+ wl_list_init(&touch->focus_listener.link);
+ touch->focus_listener.notify = lose_touch_focus;
touch->default_grab.interface = &default_touch_grab_interface;
touch->default_grab.touch = touch;
touch->grab = &touch->default_grab;
@@ -438,6 +472,7 @@ weston_touch_destroy(struct weston_touch *touch)
{
/* XXX: What about touch->resource_list? */
+ wl_list_remove(&touch->focus_listener.link);
free(touch);
}
@@ -479,6 +514,8 @@ weston_po
