Re: [PATCH v3 wayland-protocols 4/4] tablet: Add pad support to the tablet protocol
On Tue, May 17, 2016 at 02:07:39PM -0700, Jason Gerecke wrote: > On 05/17/2016 03:02 AM, Carlos Garnacho wrote: > > Hey :), > > > > On Tue, May 17, 2016 at 2:50 AM, Peter Hutterer > > wrote: > >> On Tue, May 17, 2016 at 01:30:03AM +0200, Carlos Garnacho wrote: > >>> Hey :), > >>> > >>> On Wed, May 11, 2016 at 4:51 AM, Peter Hutterer > >>> wrote: > From: Carlos Garnacho > > The pad's interface is similar to the tool interface, a client is > notified of > the pad after the tablet_added event. > > The pad has three functionalities: buttons, rings and strips. > Buttons are fairly straightforward, rings and strips are separate > interfaces > with a pointer-axis-like source/value/frame events. > The two interfaces are effectively identical but for the actual value > they > send (degrees vs normalized position). > > Buttons are sequentially indexed starting with zero, unlike other > protocols > where a linux/input.h-style semantic event code is used. Since we expect > all > buttons to have client-specific functionality, an additional event tells > the > client when a given button index is not available, usually because the > compositor assignes some function to it (e.g. mode switching, see below). > > Specific to the pad device is the set_feedback request which enables a > client > to set a user-defined string to display for an OSD on the current > mappings. > This request is available for buttons, rings and strips. > > Finally, the pad supports "modes", effectively sets of button/ring/strip > configurations. > > Signed-off-by: Carlos Garnacho > Signed-off-by: Peter Hutterer > --- > Changes to v2: > - change to v2 of the protocol > - various comments and suggestions for improved wording incorporated > - ring is in wl_fixed degrees now (was int, in 0.01 of a degree) > - button events changed to sequential indices > - new buttons_reserved event > > unstable/tablet/tablet-unstable-v2.xml | 436 > + > 1 file changed, 436 insertions(+) > > >> > >> .. > >> > + > + > + Sent on wp_tablet_pad initialization and/or at a later time to > notify the > + client of reserved buttons. > + > + Compositors may consume some buttons for global actions, those > + global actions will not trigger wl_tablet_pad.button events (e.g. > + the button to switch between modes, if any). This event notifies > the > + client that some button indices are reserved by the compositor > and > + will not generate events visible to the client. Button indices > start > + at 0. > + > + This event may is sent in the initial burst of events before the > + wp_tablet_pad.done event and/or at any later time when the > + compositor changes the list of reserved buttons. This event is > only > + sent when the compositor reserves at least one button. > + > + > + > + > + > >>> > >>> After some hands-on experience, I see this API in libwacom: > >>> > >>> int libwacom_get_ring_num_modes(const WacomDevice *device); > >>> int libwacom_get_ring2_num_modes(const WacomDevice *device); > >>> int libwacom_get_strips_num_modes(const WacomDevice *device); > >>> > >>> This makes me think, are there devices with more than one of these > >>> modes? In that case I guess would be better to move .modes and .mode > >>> to wp_tablet_pad_ring/strip than exposing the NxM combinations here. I > >>> guess it also means renouncing to making these modes affect anything > >>> else than the ring/strip. > >>> > >>> If we move these events there, I wonder if we better add .done events > >>> there too, or it suffices with wp_tablet_pad.done. I'd expect all > >>> device characteristics to be announced before wp_tablet_pad.done > >>> anyway. > >> > >> the only device that had two rings and modes was the 24HD and both had 4 > >> modes iirc. that device isn't on sale anymore and given the current gen of > >> wacom tablets I doubt it comes back. > >> > > 3 modes on each side, actually :) > > >> however, the 22hd has two touch strips at the back and two mode switch > >> buttons. I don't think there's much of a use-case for having different > >> numbers of modes but having the modes switch independently is certainly > >> something we should support. > >> > > The chances of a device having multiple mode switches with a different > number of modes is pretty low. The reason has to do with why some > devices have a second set of buttons/rings/switches in the first place: > left-handed users. While it's trivial enough to rotate an Intuos around > so the pad controls are under the users's non-dominan
Re: [PATCH wayland-protocols v2 7/7] xdg-shell: Introduce xdg_positioner
On Tue, May 17, 2016 at 03:41:37PM -0500, Yong Bakos wrote: > On May 11, 2016, at 12:50 AM, Jonas Ådahl wrote: > > > > xdg_positioner is a method for declarative positioning of child surfaces > > (currently only xdg_popup surfaces). A client creates a description of a > > positioning logic using the xdg_positioner interface. The xdg_positioner > > object is then used when creating a xdg_popup for describing how the > > child surface should be positioned in relation to the parent surface. > > > > Signed-off-by: Jonas Ådahl > > Signed-off-by: Mike Blumenkrantz > > Hi, > A few errors corrected inline below. > > yong > > > > --- > > > > Changes since v1: > > > > - Clarify that the rules of xdg_positioner is copied when used, and itself > > doesn't get "attached" to any surface. > > - Make it clear errors are raised on invalid input and when used incorrectly > > - Added xdg_positioner.set_size, in order to not relying on a possibly > > incorrect xdg_surface.set_window_geometry. > > - Added the concept of a "complete" positioner, i.e. one with a set size and > > anchor rect. > > - Fixed bitfield enum field. > > - Made the slide_x/y semantics less undefined. > > > > > > Jonas > > > > unstable/xdg-shell/xdg-shell-unstable-v6.xml | 248 > > ++- > > 1 file changed, 246 insertions(+), 2 deletions(-) > > > > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > index 2edc341..dfd7e84 100644 > > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > @@ -45,6 +45,8 @@ > > summary="the client specified an invalid popup parent surface"/> > >> summary="the client provided an invalid surface state"/> > > + > +summary="the client provided an invalid positioner"/> > > > > > > > > @@ -57,6 +59,15 @@ > > > > > > > > + > > + > > + Create a positioner object. A positioner object is used to position > > + surfaces relative to some parent surface. See the interface description > > + and xdg_surface.get_popup for details. > > + > > + > > + > > + > > > > > > This creates an xdg_surface for the given surface. While xdg_surface > > @@ -96,6 +107,223 @@ > > > > > > > > + > > + > > + The xdg_positioner provides an interface for constructing positioning > > + rules used for positioning a child surface relative to another > > surface > > Perhaps "its parent surface" rather than "another surface." Yea, maybe thats better. Or "a parent surface", since at this point that relationship is not set up yet. > > > > + in a certain way. It allows methods for defining a rule that will > > make > > + the child surface stay within the border of the visible area of the > > + screen, with different ways in which the child surface should change > > + its position, including sliding along an axis, or flipping around a > > + rectangle. > > + > > + See the various requests for details about possible rules. > > + > > + Semantically, an xdg_positioner is a collection of positioning > > rules. When > > + used for positioning a surface, for example when passed as an > > argument to > > + xdg_surface.get_popup, the compositor copies the rules that were set > > up at > > + the time of the request. Making any changes or destroying the object > > after > > + it was used has no effect on previous usages. > > + > > + For a xdg_positioner object to be considered complete, it must have a > > an xdg_positioner > > > > + non-zero size set by set_size, and a non-zero anchor rectangle set by > > + set_anchor_rect. Passing an incomplete xdg_positioner object when > > + positioning a surface raises an error. > > + > > + > > + > > + > > + > > + > > + > > + > > + Notify the compositor that the xdg_positioner will no longer be used. > > + > > + > > + > > + > > + > > to-be-positioned > or > of the rectangle to be positioned > > And should that be surface instead of rectangle? See the first line of the > description, here: > > > + Set the size of the surface that is to be positioned with the positioner > > + object. The size is in surface-local coordinates and corresponds to the > > + window geometry width. See xdg_surface.set_window_geometry. This should be rectangle. We are not setting future surface sizes, but the "window geometry" i.e. part of the popup without shadow etc. > > window geometry. (omit width?) or change it to size rather, since it is equivalent to the size (width/height) part of the window geometry. > > > > + > > + If a zero or negative size is set the invalid_input error is raised. > > + > > + > > + > > Maybe just "width of" instead of "set width of". Same for height. Sure. > > > > + > > +
[PATCH wayland] wayland-server: Clarify included header dependencies
From: Yong Bakos wayland-server.c directly depends on wayland-util.h, and will include wayland-server-protocol.h via wayland-server.h. Explicitly include wayland-util.h, making this dependency clear. Remove the redundant inclusion of wayland-server-protocol.h. Signed-off-by: Yong Bakos --- src/wayland-server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wayland-server.c b/src/wayland-server.c index f745e62..0fe532c 100644 --- a/src/wayland-server.c +++ b/src/wayland-server.c @@ -43,9 +43,9 @@ #include #include +#include "wayland-util.h" #include "wayland-private.h" #include "wayland-server.h" -#include "wayland-server-protocol.h" #include "wayland-os.h" /* This is the size of the char array in struct sock_addr_un. -- 2.7.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland-protocols v2 1/7] xdg-shell: Turn xdg_surface into a generic base interface
On Tue, May 17, 2016 at 01:35:43PM -0500, Yong Bakos wrote: > On May 11, 2016, at 12:49 AM, Jonas Ådahl wrote: > > > > Split out toplevel window like requests and events into a new interface > > called xdg_toplevel, and turn xdg_surface into a generic base interface > > which others extends. > > > > xdg_popup is changed to extend the xdg_surface. > > > > The configure event in xdg_surface was split up making > > xdg_surface.configure an event only carrying the serial number, while a > > new xdg_toplevel.configure event carries the other data previously sent > > via xdg_surface.configure. xdg_toplevel.configure is made to extend, > > via the latch-state mechanism, xdg_surface.configure and depends on > > that event to synchronize state. > > > > Other future xdg_surface based extensions are meant to also extend > > xdg_surface.configure for relevant window type dependend state > > synchronization. > > > > Signed-off-by: Jonas Ådahl > > Signed-off-by: Mike Blumenkrantz > > Hi Jonas and Mike, > Some feedback inline below. Forgive my ignorance, but is the goal > of the xdg-shell protocol extension to extract/remove the wl_shell > and wl_shell_surface stuff from the core Wayland protocol? wl_shell/wl_shell_surface should be considered deprecated, except we don't have anything stable to replace it with. It was just a proof of concept of how to actually get something on the screen. xdg_shell aims to be that replacement. I'm not sure if we can actually remove wl_shell and friends later, but we might at least be able to mark it as deprecated (if we add such a meaning to the XML). > > I realize my confusions noted below may be due to my lack of > experience here. > > Thank you, > yong > > > > --- > > > > Changes since v1: > > > > - moved xdg_surface based role semantics into xdg_surface > > - reworded xdg_toplevel description a bit > > - various minor doc changes > > > > > > Jonas > > > > > > unstable/xdg-shell/xdg-shell-unstable-v6.xml | 271 > > --- > > 1 file changed, 161 insertions(+), 110 deletions(-) > > > > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > index ce57153..4080119 100644 > > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > > @@ -54,11 +54,9 @@ > > > > > > > > - This creates an xdg_surface for the given surface and gives it the > > - xdg_surface role. A wl_surface can only be given an xdg_surface role > > - once. If get_xdg_surface is called with a wl_surface that already has > > - an active xdg_surface associated with it, or if it had any other role, > > - an error is raised. > > + This creates an xdg_surface for the given surface. While xdg_surface > > + itself is not a role, the corresponding surface may only be assigned > > + a role extending xdg_surface, such as xdg_toplevel or xdg_popup. > > > > See the documentation of xdg_surface for more details about what an > > xdg_surface is and how it is used. > > @@ -67,29 +65,6 @@ > > > > > > > > - > > - > > - This creates an xdg_popup for the given surface and gives it the > > - xdg_popup role. A wl_surface can only be given an xdg_popup role > > - once. If get_xdg_popup is called with a wl_surface that already has > > - an active xdg_popup associated with it, or if it had any other role, > > - an error is raised. > > - > > - This request must be used in response to some sort of user action > > - like a button press, key press, or touch down event. > > - > > - See the documentation of xdg_popup for more details about what an > > - xdg_popup is and how it is used. > > - > > - > > - > > - > > - > > - > > - > > - > > - > > - > > > > > > The ping event asks the client if it's still alive. Pass the > > @@ -117,13 +92,23 @@ > > > > > > > > - > > + > > An interface that may be implemented by a wl_surface, for > > implementations that provide a desktop-style user interface. > > > > - It provides requests to treat surfaces like windows, allowing to set > > - properties like maximized, fullscreen, minimized, and to move and > > resize > > - them, and associate metadata like title and app id. > > + It provides a base set of functionality required to construct user > > + interface elements requiring management by the compositor, such as > > + toplevel windows, menus, etc. The types of functionality are split > > into > > + xdg_surface roles. > > + > > + Creating an xdg_surface does not set the role for a wl_surface. In > > order > > + to map an xdg_surface, create a role-specific object using, e.g., > > + get_toplevel, get_popup. The wl_surface for any given xdg_surface can > > + have at most one role, and may not be assigned any role not based on > > + xdg_
Re: [PATCH v3 wayland-protocols 4/4] tablet: Add pad support to the tablet protocol
On 05/17/2016 03:02 AM, Carlos Garnacho wrote: > Hey :), > > On Tue, May 17, 2016 at 2:50 AM, Peter Hutterer > wrote: >> On Tue, May 17, 2016 at 01:30:03AM +0200, Carlos Garnacho wrote: >>> Hey :), >>> >>> On Wed, May 11, 2016 at 4:51 AM, Peter Hutterer >>> wrote: From: Carlos Garnacho The pad's interface is similar to the tool interface, a client is notified of the pad after the tablet_added event. The pad has three functionalities: buttons, rings and strips. Buttons are fairly straightforward, rings and strips are separate interfaces with a pointer-axis-like source/value/frame events. The two interfaces are effectively identical but for the actual value they send (degrees vs normalized position). Buttons are sequentially indexed starting with zero, unlike other protocols where a linux/input.h-style semantic event code is used. Since we expect all buttons to have client-specific functionality, an additional event tells the client when a given button index is not available, usually because the compositor assignes some function to it (e.g. mode switching, see below). Specific to the pad device is the set_feedback request which enables a client to set a user-defined string to display for an OSD on the current mappings. This request is available for buttons, rings and strips. Finally, the pad supports "modes", effectively sets of button/ring/strip configurations. Signed-off-by: Carlos Garnacho Signed-off-by: Peter Hutterer --- Changes to v2: - change to v2 of the protocol - various comments and suggestions for improved wording incorporated - ring is in wl_fixed degrees now (was int, in 0.01 of a degree) - button events changed to sequential indices - new buttons_reserved event unstable/tablet/tablet-unstable-v2.xml | 436 + 1 file changed, 436 insertions(+) >> >> .. >> + + + Sent on wp_tablet_pad initialization and/or at a later time to notify the + client of reserved buttons. + + Compositors may consume some buttons for global actions, those + global actions will not trigger wl_tablet_pad.button events (e.g. + the button to switch between modes, if any). This event notifies the + client that some button indices are reserved by the compositor and + will not generate events visible to the client. Button indices start + at 0. + + This event may is sent in the initial burst of events before the + wp_tablet_pad.done event and/or at any later time when the + compositor changes the list of reserved buttons. This event is only + sent when the compositor reserves at least one button. + + + + + >>> >>> After some hands-on experience, I see this API in libwacom: >>> >>> int libwacom_get_ring_num_modes(const WacomDevice *device); >>> int libwacom_get_ring2_num_modes(const WacomDevice *device); >>> int libwacom_get_strips_num_modes(const WacomDevice *device); >>> >>> This makes me think, are there devices with more than one of these >>> modes? In that case I guess would be better to move .modes and .mode >>> to wp_tablet_pad_ring/strip than exposing the NxM combinations here. I >>> guess it also means renouncing to making these modes affect anything >>> else than the ring/strip. >>> >>> If we move these events there, I wonder if we better add .done events >>> there too, or it suffices with wp_tablet_pad.done. I'd expect all >>> device characteristics to be announced before wp_tablet_pad.done >>> anyway. >> >> the only device that had two rings and modes was the 24HD and both had 4 >> modes iirc. that device isn't on sale anymore and given the current gen of >> wacom tablets I doubt it comes back. >> 3 modes on each side, actually :) >> however, the 22hd has two touch strips at the back and two mode switch >> buttons. I don't think there's much of a use-case for having different >> numbers of modes but having the modes switch independently is certainly >> something we should support. >> The chances of a device having multiple mode switches with a different number of modes is pretty low. The reason has to do with why some devices have a second set of buttons/rings/switches in the first place: left-handed users. While it's trivial enough to rotate an Intuos around so the pad controls are under the users's non-dominant hand, that's not the case with the Cintiqs. To address this, a mirror-image set of pad controls is present to make things equally accessible to either hand. There's basically zero chance that a tablet would have a different number of modes on its two mirrored halves. More likely is a device that doesn't have a mirrored layout but
Re: [PATCH wayland-protocols v2 7/7] xdg-shell: Introduce xdg_positioner
On May 11, 2016, at 12:50 AM, Jonas Ådahl wrote: > > xdg_positioner is a method for declarative positioning of child surfaces > (currently only xdg_popup surfaces). A client creates a description of a > positioning logic using the xdg_positioner interface. The xdg_positioner > object is then used when creating a xdg_popup for describing how the > child surface should be positioned in relation to the parent surface. > > Signed-off-by: Jonas Ådahl > Signed-off-by: Mike Blumenkrantz Hi, A few errors corrected inline below. yong > --- > > Changes since v1: > > - Clarify that the rules of xdg_positioner is copied when used, and itself > doesn't get "attached" to any surface. > - Make it clear errors are raised on invalid input and when used incorrectly > - Added xdg_positioner.set_size, in order to not relying on a possibly > incorrect xdg_surface.set_window_geometry. > - Added the concept of a "complete" positioner, i.e. one with a set size and > anchor rect. > - Fixed bitfield enum field. > - Made the slide_x/y semantics less undefined. > > > Jonas > > unstable/xdg-shell/xdg-shell-unstable-v6.xml | 248 ++- > 1 file changed, 246 insertions(+), 2 deletions(-) > > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > index 2edc341..dfd7e84 100644 > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > @@ -45,6 +45,8 @@ >summary="the client specified an invalid popup parent surface"/> > summary="the client provided an invalid surface state"/> > + + summary="the client provided an invalid positioner"/> > > > > @@ -57,6 +59,15 @@ > > > > + > + > + Create a positioner object. A positioner object is used to position > + surfaces relative to some parent surface. See the interface description > + and xdg_surface.get_popup for details. > + > + > + > + > > > This creates an xdg_surface for the given surface. While xdg_surface > @@ -96,6 +107,223 @@ > > > > + > + > + The xdg_positioner provides an interface for constructing positioning > + rules used for positioning a child surface relative to another surface Perhaps "its parent surface" rather than "another surface." > + in a certain way. It allows methods for defining a rule that will make > + the child surface stay within the border of the visible area of the > + screen, with different ways in which the child surface should change > + its position, including sliding along an axis, or flipping around a > + rectangle. > + > + See the various requests for details about possible rules. > + > + Semantically, an xdg_positioner is a collection of positioning rules. > When > + used for positioning a surface, for example when passed as an argument > to > + xdg_surface.get_popup, the compositor copies the rules that were set > up at > + the time of the request. Making any changes or destroying the object > after > + it was used has no effect on previous usages. > + > + For a xdg_positioner object to be considered complete, it must have a an xdg_positioner > + non-zero size set by set_size, and a non-zero anchor rectangle set by > + set_anchor_rect. Passing an incomplete xdg_positioner object when > + positioning a surface raises an error. > + > + > + > + > + > + > + > + > + Notify the compositor that the xdg_positioner will no longer be used. > + > + > + > + > + to-be-positioned or of the rectangle to be positioned And should that be surface instead of rectangle? See the first line of the description, here: > + Set the size of the surface that is to be positioned with the positioner > + object. The size is in surface-local coordinates and corresponds to the > + window geometry width. See xdg_surface.set_window_geometry. window geometry. (omit width?) > + > + If a zero or negative size is set the invalid_input error is raised. > + > + > + Maybe just "width of" instead of "set width of". Same for height. > + > + > + > + > + Specify the anchor rectangle within the parent surface that the child > + surface will be placed relative to. The rectangle is relative to the > + window geometry as defined by xdg_surface.set_window_geometry of the > + parent surface. The rectangle must be at least 1x1 large. > + > + When the xdg_positioner object is used to position a child surface, the > + anchor rectangle may not extend outside the window geometry of the > + positioned child's parent surface. Will exceeding those bounds result in an error, or just clipping? > + > + If a zero or negative size is set the invalid_input error is raised. > + > + > + Perha
Re: [PATCH wayland-protocols v2 6/7] xdg-shell: Make xdg_popup non-grabbing by default
On May 11, 2016, at 12:50 AM, Jonas Ådahl wrote: > > Turn xdg_popup into plain temporary child surfaces without any grabbing > or mapping order requirements by default. > > In order to create grabbing popup chains, a new request 'grab' is > introduced which enables more or less the same semantics and > requirements as xdg_popup previously had related to grabbing, stacking > and mapping order. > > This enables using xdg_popup for creating tooltips and other user > interface elements that does not want to take an explicit grab. > > Signed-off-by: Jonas Ådahl > Signed-off-by: Mike Blumenkrantz All seems pretty clear to me and is Reviewed-by: Yong Bakos yong > --- > > Changes since v1: > > - various grammatical fixes > > > Jonas > > unstable/xdg-shell/xdg-shell-unstable-v6.xml | 110 +-- > 1 file changed, 68 insertions(+), 42 deletions(-) > > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > index 90cf7c2..2edc341 100644 > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > @@ -158,16 +158,11 @@ > This creates an xdg_popup object for the given xdg_surface and gives the > associated wl_surface the xdg_popup role. > > - This request must be used in response to some sort of user action like a > - button press, key press, or touch down event. > - > See the documentation of xdg_popup for more details about what an > xdg_popup is and how it is used. > > > > - > - > > > > @@ -666,46 +661,30 @@ > > > > - A popup surface is a short-lived, temporary surface that can be > - used to implement menus. It takes an explicit grab on the surface > - that will be dismissed when the user dismisses the popup. This can > - be done by the user clicking outside the surface, using the keyboard, > - or even locking the screen through closing the lid or a timeout. > - > - When the popup is dismissed, a popup_done event will be sent out, > - and at the same time the surface will be unmapped. The xdg_popup > - object is now inert and cannot be reactivated, so clients should > - destroy it. Explicitly destroying the xdg_popup object will also > - dismiss the popup and unmap the surface. > - > - Clients will receive events for all their surfaces during this > - grab (which is an "owner-events" grab in X11 parlance). This is > - done so that users can navigate through submenus and other > - "nested" popup windows without having to dismiss the topmost > - popup. > - > - Clients that want to dismiss the popup when another surface of > - their own is clicked should dismiss the popup using the destroy > + A popup surface is a short-lived, temporary surface. It can be used to > + implement for example menus, popovers, tooltips and other similar user > + interface concepts. > + > + A popup can be made to take an explicit grab. See xdg_popup.grab for > + details. > + > + When the popup is dismissed, a popup_done event will be sent out, and > at > + the same time the surface will be unmapped. See the > xdg_popup.popup_done > + event for details. > + > + Explicitly destroying the xdg_popup object will also dismiss the popup > and > + unmap the surface. Clients that want to dismiss the popup when another > + surface of their own is clicked should dismiss the popup using the > destroy > request. > > The parent surface must have either the xdg_toplevel or xdg_popup > surface > role. > > - Specifying an xdg_popup for the parent means that the popups are > - nested, with this popup now being the topmost popup. Nested > - popups must be destroyed in the reverse order they were created > - in, e.g. the only popup you are allowed to destroy at all times > - is the topmost one. > - > - If there is an existing popup when creating a new popup, the > - parent must be the current topmost popup. > - > - A parent surface must be mapped before the new popup is mapped. > + A newly created xdg_popup will be stacked on top of all previously > created > + xdg_popup surfaces associated with the same xdg_toplevel. > > - When compositors choose to dismiss a popup, they will likely > - dismiss every nested popup as well. When a compositor dismisses > - popups, it will follow the same dismissing order as required > - from the client. > + The parent of an xdg_popup must be mapped (see the xdg_surface > + description) before the xdg_popup itself. > > The x and y arguments passed when creating the popup object specify > where the top left of the popup should be placed, relative to the > @@ -714,9 +693,6 @@ > > The client must call wl_surface.commit on the corresponding wl
Re: [PATCH wayland-protocols v2 5/7] xdg-shell: Make get_popup take a xdg_surface instead of wl_surface
On May 11, 2016, at 12:50 AM, Jonas Ådahl wrote: > > The reason for using wl_surface before was that xdg_popup and > xdg_surface (now xdg_toplevel) had no common interface other than > wl_surface, but since xdg_surface is now the base interface, lets use > that. > > Signed-off-by: Jonas Ådahl > Reviewed-by: Mike Blumenkrantz Reviewed-by: Yong Bakos yong > --- > unstable/xdg-shell/xdg-shell-unstable-v6.xml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > index d539c1f..90cf7c2 100644 > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > @@ -165,7 +165,7 @@ > xdg_popup is and how it is used. > > > - > + > > > > -- > 2.5.5 > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland-protocols v2 4/7] xdg-shell: Improve error enum formatting some
On May 11, 2016, at 12:50 AM, Jonas Ådahl wrote: > > The long lines stood out, break them by putting the summary on its own > line. > > Signed-off-by: Jonas Ådahl > Reviewed-by: Yong Bakos > Reviewed-by: Mike Blumenkrantz This v2 is again Reviewed-by: Yong Bakos At some point I want to gently propose that all protocol docs place the summary attributes on their own line, for a) consistency and b) to decouple documentation changes from other parameter attributes in the commit history. yong > --- > unstable/xdg-shell/xdg-shell-unstable-v6.xml | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > index 0d31ca5..d539c1f 100644 > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > @@ -37,10 +37,14 @@ > > > > - > - > - > - > + + summary="xdg_shell was destroyed before children"/> > + + summary="the client tried to map or destroy a non-topmost popup"/> > + + summary="the client specified an invalid popup parent surface"/> > + + summary="the client provided an invalid surface state"/> > > > > -- > 2.5.5 > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland-protocols v2 3/7] xdg-shell: Add error codes for invalid surface state
On May 11, 2016, at 12:50 AM, Jonas Ådahl wrote: > > Signed-off-by: Jonas Ådahl > Reviewed-by: Mike Blumenkrantz Makes sense, Reviewed-by: Yong Bakos yong > --- > unstable/xdg-shell/xdg-shell-unstable-v6.xml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > index 2a30af5..0d31ca5 100644 > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > @@ -40,6 +40,7 @@ > > > > + > > > > @@ -127,6 +128,7 @@ > > > > + > > > > -- > 2.5.5 > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland-protocols v2 1/7] xdg-shell: Turn xdg_surface into a generic base interface
On May 11, 2016, at 12:49 AM, Jonas Ådahl wrote: > > Split out toplevel window like requests and events into a new interface > called xdg_toplevel, and turn xdg_surface into a generic base interface > which others extends. > > xdg_popup is changed to extend the xdg_surface. > > The configure event in xdg_surface was split up making > xdg_surface.configure an event only carrying the serial number, while a > new xdg_toplevel.configure event carries the other data previously sent > via xdg_surface.configure. xdg_toplevel.configure is made to extend, > via the latch-state mechanism, xdg_surface.configure and depends on > that event to synchronize state. > > Other future xdg_surface based extensions are meant to also extend > xdg_surface.configure for relevant window type dependend state > synchronization. > > Signed-off-by: Jonas Ådahl > Signed-off-by: Mike Blumenkrantz Hi Jonas and Mike, Some feedback inline below. Forgive my ignorance, but is the goal of the xdg-shell protocol extension to extract/remove the wl_shell and wl_shell_surface stuff from the core Wayland protocol? I realize my confusions noted below may be due to my lack of experience here. Thank you, yong > --- > > Changes since v1: > > - moved xdg_surface based role semantics into xdg_surface > - reworded xdg_toplevel description a bit > - various minor doc changes > > > Jonas > > > unstable/xdg-shell/xdg-shell-unstable-v6.xml | 271 --- > 1 file changed, 161 insertions(+), 110 deletions(-) > > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > index ce57153..4080119 100644 > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml > @@ -54,11 +54,9 @@ > > > > - This creates an xdg_surface for the given surface and gives it the > - xdg_surface role. A wl_surface can only be given an xdg_surface role > - once. If get_xdg_surface is called with a wl_surface that already has > - an active xdg_surface associated with it, or if it had any other role, > - an error is raised. > + This creates an xdg_surface for the given surface. While xdg_surface > + itself is not a role, the corresponding surface may only be assigned > + a role extending xdg_surface, such as xdg_toplevel or xdg_popup. > > See the documentation of xdg_surface for more details about what an > xdg_surface is and how it is used. > @@ -67,29 +65,6 @@ > > > > - > - > - This creates an xdg_popup for the given surface and gives it the > - xdg_popup role. A wl_surface can only be given an xdg_popup role > - once. If get_xdg_popup is called with a wl_surface that already has > - an active xdg_popup associated with it, or if it had any other role, > - an error is raised. > - > - This request must be used in response to some sort of user action > - like a button press, key press, or touch down event. > - > - See the documentation of xdg_popup for more details about what an > - xdg_popup is and how it is used. > - > - > - > - > - > - > - > - > - > - > > > The ping event asks the client if it's still alive. Pass the > @@ -117,13 +92,23 @@ > > > > - > + > An interface that may be implemented by a wl_surface, for > implementations that provide a desktop-style user interface. > > - It provides requests to treat surfaces like windows, allowing to set > - properties like maximized, fullscreen, minimized, and to move and > resize > - them, and associate metadata like title and app id. > + It provides a base set of functionality required to construct user > + interface elements requiring management by the compositor, such as > + toplevel windows, menus, etc. The types of functionality are split into > + xdg_surface roles. > + > + Creating an xdg_surface does not set the role for a wl_surface. In > order > + to map an xdg_surface, create a role-specific object using, e.g., > + get_toplevel, get_popup. The wl_surface for any given xdg_surface can > + have at most one role, and may not be assigned any role not based on > + xdg_surface. > + > + A role must be assigned before any other requests are made to the > + xdg_surface object. > > The client must call wl_surface.commit on the corresponding wl_surface > for the xdg_surface state to take effect. > @@ -133,12 +118,142 @@ > manipulate a buffer prior to the first xdg_surface.configure call must > also be treated as errors. > > - For a surface to be mapped by the compositor the client must have > - committed both an xdg_surface state and a buffer. > + For a surface to be mapped by the compositor the client must have > assigned > + one of the xdg_surface based roles, it must have commi
Re: [PATCH] ivi-layout: Initialize surface source rectange to 1x1
On May 17, 2016, at 3:58 AM, mateuszx.potr...@intel.com wrote: > > From: Mateusz Polrola > > If surface will be set to visible before its source rectangle will > be defined it will be displayed in its orginal size. > This is because initial setting of destination rectangle to 1x1 is > not causing surface resize and because source rectangle is 0x0 > appropiate transformation matrix is not calculated > > Signed-off-by: Mateusz Polrola Hi Mateusz, Minor nit: would be nice to see [PATCH weston] in the subject line. See: https://lists.freedesktop.org/archives/wayland-devel/2016-March/027722.html Regarding the patch, > --- > ivi-shell/ivi-layout.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c > index 1601787..ef7ee02 100644 > --- a/ivi-shell/ivi-layout.c > +++ b/ivi-shell/ivi-layout.c > @@ -250,6 +250,9 @@ init_surface_properties(struct > ivi_layout_surface_properties *prop) >*/ > prop->dest_width = 1; > prop->dest_height = 1; > + > + prop->source_width = 1; > + prop->source_height = 1; > } Minor nit: I wouldn't use a blank line there. Should there be a condition around this to only set the source width/height to 1 if they're both 0? yong > > /** > -- > 2.1.0 > > Intel Deutschland GmbH > Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany > Tel: +49 89 99 8853-0, www.intel.de > Managing Directors: Christin Eisenschmid, Christian Lamprechter > Chairperson of the Supervisory Board: Nicole Lau > Registered Office: Munich > Commercial Register: Amtsgericht Muenchen HRB 186928 > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] weston: Don't exit just because tty is in gfx mode already
On May 16, 2016, at 5:25 PM, Florian Hänel wrote:
>
> From: =?UTF-8?q?Florian=20H=C3=A4nel?=
>
> If weston crashed (or during development) it can leave the tty
> in graphics mode, which would prevent it from ever coming up again.
>
> Another compositor running should be handled by upstart/systemd
> and the tty in graphics mode does not prevent us from using it.
>
> A informational log message for debugging purposes should be enough.
Hi Florian,
Some quick feedback about the sending of the patch. See:
https://cgit.freedesktop.org/wayland/wayland/tree/doc/Contributing
Ideally, the subject line should be:
[PATCH weston] launcher-util: Don't...
And the commit message should include an "Sob" line (use the -s option with
git commit). And, are you using a non-default encoding config with git
send-email? I'm noticing that your name isn't being formatted correctly.
Regards,
yong
> ---
> src/launcher-util.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/src/launcher-util.c b/src/launcher-util.c
> index e89710b..a98ff74 100644
> --- a/src/launcher-util.c
> +++ b/src/launcher-util.c
> @@ -319,7 +319,6 @@ setup_tty(struct weston_launcher *launcher, int tty)
> if (kd_mode != KD_TEXT) {
> weston_log("%s is already in graphics mode, "
>"is another display server running?\n", tty_device);
> -goto err_close;
> }
>
> ioctl(launcher->tty, VT_ACTIVATE, minor(buf.st_rdev));
> --
> 2.7.4
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 4/8] compositor-drm: Prevent a crash in the pixman renderer
On Tue, 3 May 2016 15:08:37 +0200 Quentin Glidic wrote: > On 02/05/2016 23:40, Emmanuel Gil Peyrot wrote: > > When pixman is used and no connector could be found (or any other > > error), drm_backend_create() tried to destroy a gbm_device that would > > only be created in init_egl(), resulting in a segfault. > > > > Signed-off-by: Emmanuel Gil Peyrot > > --- > > src/compositor-drm.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/src/compositor-drm.c b/src/compositor-drm.c > > index c11562f..f9a997b 100644 > > --- a/src/compositor-drm.c > > +++ b/src/compositor-drm.c > > @@ -3227,7 +3227,8 @@ err_drm_source: > > err_udev_input: > > udev_input_destroy(&b->input); > > err_sprite: > > - gbm_device_destroy(b->gbm); > > + if (b->gbm) > > + gbm_device_destroy(b->gbm); > > destroy_sprites(b); > > err_udev_dev: > > udev_device_unref(drm_device); > > > > > Reviewed-by: Quentin Glidic > > Should land on its own. > Hi, patches 1, 2 and 4 pushed: 130ae6e..b8347e3 master -> master Patch 2 with the long line fix. The rest we might want to defer for after 1.11. Thanks, pq pgppsA0hPnljD.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 3/8] compositor-drm: Don’t try to set the cursor anymore once we know it’s broken
On Mon, 2 May 2016 22:40:12 +0100 Emmanuel Gil Peyrot wrote: > Signed-off-by: Emmanuel Gil Peyrot > --- > src/compositor-drm.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/compositor-drm.c b/src/compositor-drm.c > index ea118fa..c11562f 100644 > --- a/src/compositor-drm.c > +++ b/src/compositor-drm.c > @@ -671,7 +671,8 @@ drm_output_repaint(struct weston_output *output_base, > > output->page_flip_pending = 1; > > - drm_output_set_cursor(output); > + if (!backend->cursors_are_broken) > + drm_output_set_cursor(output); > > /* >* Now, update all the sprite surfaces Hi, we talked about this in irc. This is an easy-looking change, but there are subtle things hiding behind that raised my eyebrow a bit. drm_output_prepare_cursor_view() should already be bailing out, for cursors_are_broken if nothing else. If that happens, then this patch would only avoid unsetting the cursor on every frame. I can very well understand the desire to not repeatedly unset the cursor, however you told me unsetting was not the case. Btw. even the unsetting case might be prone to a couple corner-cases: - VT-switching in, you better hammer in all KMS state again. Even if cursors are broken for us, maybe something else managed to set one. We don't want to leave a stale cursor on screen. - According to [1], some virtual hardware might be sometimes able to handle cursors and other times not. Or, maybe there are some other unknown limitations on when cursor can be used or not, but anyway, I'd like to guarantee there won't be a stale cursor on screen. Funnily enough, the DRM-backend is already never going back from cursors_are_broken when once set. The current intended behaviour in Weston is to try to use the HW cursor, and set cursors_are_broken in drm_output_set_cursor() the first time it fails. That makes prepare_cursor_view() always bail out, so no cursor is attempted again, though the cursor is unset (mostly redundantly) on every frame. Because it is unclear whether this patch is actually necessary, and we could not figure it out in irc, and there is some remote possibility for a rare regression, I'd leave this out for now. Thanks, pq [1] https://lists.x.org/archives/xorg-devel/2016-March/048960.html pgpmrjnCgImWG.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/2] compositor: fix comments about weston_compositor::surface_list
On Tue, 17 May 2016 12:17:45 +0300 Pekka Paalanen wrote: > On Mon, 16 May 2016 16:29:18 +0200 > Armin Krezović wrote: > > > On 10.05.2016 16:10, Pekka Paalanen wrote: > > > From: Pekka Paalanen > > > > > > a7af70436b7dccfacd736626d6719b3e751fd985 converted the surface list into > > > a view list. > > > > > > It looks like weston_surface::output's comment about surface list does > > > not apply to view list. Still, many places assume weston_surface::output > > > is not NULL when processing "visible" surfaces, e.g. those reachable via > > > the view list. > > > > > > The comment on weston_view::output is updated, but I could not figure > > > out the actual relationship between that and being on the view list. > > > > > > weston_view::link is documented to be in weston_compositor::view_list, > > > and weston_compositor::view_list is documented to contain weston_views. > > > > > > > It is always nice to see someone documenting the code. I've been banging > > my head lately to understand the codebase, and the currently present > > comments really help there. > > > > With the issue noted below fixed, this patch is: > > > > Reviewed-by: Armin Krezović > > > > > Signed-off-by: Pekka Paalanen > > > --- > > > src/compositor.h | 7 +++ > > > 1 file changed, 3 insertions(+), 4 deletions(-) > IOW, did not find a connection between link and output there. > > > I think I'll tweak the comments in these two patches a bit, and maybe > push then. Comments tweaked a bit, and both pushed: 901ac32..130ae6e master -> master Thanks, pq pgpUkwVQulLiB.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v3 wayland-protocols 4/4] tablet: Add pad support to the tablet protocol
Hey :), On Tue, May 17, 2016 at 2:50 AM, Peter Hutterer wrote: > On Tue, May 17, 2016 at 01:30:03AM +0200, Carlos Garnacho wrote: >> Hey :), >> >> On Wed, May 11, 2016 at 4:51 AM, Peter Hutterer >> wrote: >> > From: Carlos Garnacho >> > >> > The pad's interface is similar to the tool interface, a client is notified >> > of >> > the pad after the tablet_added event. >> > >> > The pad has three functionalities: buttons, rings and strips. >> > Buttons are fairly straightforward, rings and strips are separate >> > interfaces >> > with a pointer-axis-like source/value/frame events. >> > The two interfaces are effectively identical but for the actual value they >> > send (degrees vs normalized position). >> > >> > Buttons are sequentially indexed starting with zero, unlike other protocols >> > where a linux/input.h-style semantic event code is used. Since we expect >> > all >> > buttons to have client-specific functionality, an additional event tells >> > the >> > client when a given button index is not available, usually because the >> > compositor assignes some function to it (e.g. mode switching, see below). >> > >> > Specific to the pad device is the set_feedback request which enables a >> > client >> > to set a user-defined string to display for an OSD on the current mappings. >> > This request is available for buttons, rings and strips. >> > >> > Finally, the pad supports "modes", effectively sets of button/ring/strip >> > configurations. >> > >> > Signed-off-by: Carlos Garnacho >> > Signed-off-by: Peter Hutterer >> > --- >> > Changes to v2: >> > - change to v2 of the protocol >> > - various comments and suggestions for improved wording incorporated >> > - ring is in wl_fixed degrees now (was int, in 0.01 of a degree) >> > - button events changed to sequential indices >> > - new buttons_reserved event >> > >> > unstable/tablet/tablet-unstable-v2.xml | 436 >> > + >> > 1 file changed, 436 insertions(+) >> > > > .. > >> > + >> > + >> > + Sent on wp_tablet_pad initialization and/or at a later time to >> > notify the >> > + client of reserved buttons. >> > + >> > + Compositors may consume some buttons for global actions, those >> > + global actions will not trigger wl_tablet_pad.button events (e.g. >> > + the button to switch between modes, if any). This event notifies >> > the >> > + client that some button indices are reserved by the compositor and >> > + will not generate events visible to the client. Button indices >> > start >> > + at 0. >> > + >> > + This event may is sent in the initial burst of events before the >> > + wp_tablet_pad.done event and/or at any later time when the >> > + compositor changes the list of reserved buttons. This event is only >> > + sent when the compositor reserves at least one button. >> > + >> > + >> > + >> > + >> > + >> >> After some hands-on experience, I see this API in libwacom: >> >> int libwacom_get_ring_num_modes(const WacomDevice *device); >> int libwacom_get_ring2_num_modes(const WacomDevice *device); >> int libwacom_get_strips_num_modes(const WacomDevice *device); >> >> This makes me think, are there devices with more than one of these >> modes? In that case I guess would be better to move .modes and .mode >> to wp_tablet_pad_ring/strip than exposing the NxM combinations here. I >> guess it also means renouncing to making these modes affect anything >> else than the ring/strip. >> >> If we move these events there, I wonder if we better add .done events >> there too, or it suffices with wp_tablet_pad.done. I'd expect all >> device characteristics to be announced before wp_tablet_pad.done >> anyway. > > the only device that had two rings and modes was the 24HD and both had 4 > modes iirc. that device isn't on sale anymore and given the current gen of > wacom tablets I doubt it comes back. > > however, the 22hd has two touch strips at the back and two mode switch > buttons. I don't think there's much of a use-case for having different > numbers of modes but having the modes switch independently is certainly > something we should support. > > the next step is then the buttons of course: on the 22hd I'd expect only the > right set of buttons to switch mode when the center button is pressed, but > the strip could/should be paird with that right set of buttons. > so we effectively need something like button groups within the pad, and the > strip/ring (and later LEDs) could be part of that button group. > > The two options we have here is to either nest them within the tablet_pad > with most of the current pad events/requests move to the button group: > wp_tablet_manager > wp_tablet_seat > wp_tablet_pad > wp_tablet_pad_button_group > wp_tablet_pad_ring > wp_tablet_pad_strip Hmm, with this interface hierarchy, it seems to me that you will always get the same set of button groups with a same pad dev
Re: [PATCH weston 1/2] compositor: fix comments about weston_compositor::surface_list
On Mon, 16 May 2016 16:29:18 +0200
Armin Krezović wrote:
> On 10.05.2016 16:10, Pekka Paalanen wrote:
> > From: Pekka Paalanen
> >
> > a7af70436b7dccfacd736626d6719b3e751fd985 converted the surface list into
> > a view list.
> >
> > It looks like weston_surface::output's comment about surface list does
> > not apply to view list. Still, many places assume weston_surface::output
> > is not NULL when processing "visible" surfaces, e.g. those reachable via
> > the view list.
> >
> > The comment on weston_view::output is updated, but I could not figure
> > out the actual relationship between that and being on the view list.
> >
> > weston_view::link is documented to be in weston_compositor::view_list,
> > and weston_compositor::view_list is documented to contain weston_views.
> >
>
> It is always nice to see someone documenting the code. I've been banging
> my head lately to understand the codebase, and the currently present
> comments really help there.
>
> With the issue noted below fixed, this patch is:
>
> Reviewed-by: Armin Krezović
>
> > Signed-off-by: Pekka Paalanen
> > ---
> > src/compositor.h | 7 +++
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/compositor.h b/src/compositor.h
> > index a95f05d..7851000 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -754,7 +754,7 @@ struct weston_compositor {
> > struct wl_list output_list;
> > struct wl_list seat_list;
> > struct wl_list layer_list;
> > - struct wl_list view_list;
> > + struct wl_list view_list; /* struct weston_view::link */
> > struct wl_list plane_list;
> > struct wl_list key_binding_list;
> > struct wl_list modifier_binding_list;
> > @@ -890,7 +890,7 @@ struct weston_view {
> > struct wl_list surface_link;
> > struct wl_signal destroy_signal;
> >
> > - struct wl_list link;
> > + struct wl_list link; /* weston_compositor::view_list */
> > struct weston_layer_entry layer_link; /* part of geometry */
> > struct weston_plane *plane;
> >
> > @@ -951,7 +951,7 @@ struct weston_view {
> > /*
> > * Which output to vsync this surface to.
> > * Used to determine, whether to send or queue frame events.
> > -* Must be NULL, if 'link' is not in weston_compositor::surface_list.
> > +* Must be NULL, if 'link' is not in weston_compositor::view_list.
>
> I couldn't find if anything checks for this, and view->output can only be null
> if it got NULL from an output list. So I think the comment is redundant.
Btw. you cannot get a NULL from an output list, unless you first
explicitly check that the list is empty and bail out. Code that simply
does wl_container_of(output_list.next, output, link) or similar will
silently get a bad pointer and cause havoc later. The 'next' of an
empty list points to the list head itself, it's not NULL. And list head
is almost always not a valid list member.
Cool. I'm re-checking just to be sure. Marking weston_view::link as
deprecated, the compiler points me to the following cases:
weston_view_unmap():
Bails out via weston_view_is_mapped() if view->output == NULL.
Otherwise removes *and* initializes 'link'. So this is a
potential issue, but the init hints that 'link' is intended to
be always removable, which would void the issue.
weston_view_create() unconditionally inits 'link', and
weston_view_destroy() unconditionally removes 'link', so these confirm
that 'link' is always removable.
view_list_add_subsurface_view():
Bails out if surface->output is NULL. (Not view->output!)
Otherwise it does an unconditional wl_list_insert(view->link),
which means the list 'link' was previously in, is now corrupt
because it did not remove first.
view_list_add():
Also does an unconditional wl_list_insert(view->link),
which means the list 'link' was previously in, is now corrupt.
However, the corruption created there does not hurt, because those
functions are called from weston_compositor_build_view_list(), which
does wl_list_init() on the view_list, leaving all member of the list
orphaned and with corrupted 'link' to begin with.
But, there may be a danger there. If some views are not put back in the
view list during that weston_compositor_build_view_list() call, they
will be left with a corrupted 'link'. If you then remove 'link', it may
corrupt others. This path could use more investigation, and maybe we
should just fix it to keep all 'link' members always valid anyway by
doing the proper removes. (You can also remove a list head, it means
the elements remain in a headless list you cannot safely traverse, but
remove will work just fine.)
weston_plane_release():
Traverses the view_list to reset plane pointers to NULL since
the plane is being destroyed... looks like it might miss any
views not in view_list though, which might cause problems if
such view taken into use
[PATCH] ivi-layout: Initialize surface source rectange to 1x1
From: Mateusz Polrola If surface will be set to visible before its source rectangle will be defined it will be displayed in its orginal size. This is because initial setting of destination rectangle to 1x1 is not causing surface resize and because source rectangle is 0x0 appropiate transformation matrix is not calculated Signed-off-by: Mateusz Polrola --- ivi-shell/ivi-layout.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c index 1601787..ef7ee02 100644 --- a/ivi-shell/ivi-layout.c +++ b/ivi-shell/ivi-layout.c @@ -250,6 +250,9 @@ init_surface_properties(struct ivi_layout_surface_properties *prop) */ prop->dest_width = 1; prop->dest_height = 1; + + prop->source_width = 1; + prop->source_height = 1; } /** -- 2.1.0 Intel Deutschland GmbH Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Christian Lamprechter Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/2] compositor: surface and view output comment fixes
On Mon, 16 May 2016 23:02:42 +0800
Jonas Ådahl wrote:
> On Mon, May 16, 2016 at 04:45:05PM +0200, Armin Krezović wrote:
> > On 10.05.2016 16:10, Pekka Paalanen wrote:
> > > From: Pekka Paalanen
> > >
> > > weston_surface::output and weston_view::output as used for different
> > > purposes. Only the surface output is used for frame callbacks.
> > >
> > > The uses of the view output are much more vague and hard to describe.
> > >
> > > Also fix a comment mistake in weston_surface_assign_output().
> > >
> >
> > All the changes look fine to me. I have one comment below, as I'm not sure
> > if it's a mistake or not.
> >
> > In case it isn't a mistake (or with the mistake fixed, in case it is one),
> > this patch is also:
> >
> > Reviewed-by: Armin Krezović
> >
> > > Signed-off-by: Pekka Paalanen
> > > ---
> > > src/compositor.c | 8 +++-
> > > src/compositor.h | 5 +++--
> > > 2 files changed, 6 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/src/compositor.c b/src/compositor.c
> > > index ee47a82..40d8baf 100644
> > > --- a/src/compositor.c
> > > +++ b/src/compositor.c
> > > @@ -1082,16 +1082,15 @@ weston_surface_update_output_mask(struct
> > > weston_surface *es, uint32_t mask)
> > > }
> > > }
> > >
> > > -
> > > /** Recalculate which output(s) the surface has views displayed on
> > > *
> > > * \param es The surface to remap to outputs
> > > *
> > > * Finds the output that is showing the largest amount of one
> > > * of the surface's various views. This output becomes the
> > > - * surface's primary output for vsync and frame event purposes.
> > > + * surface's primary output for vsync and frame callback purposes.
> > > *
> > > - * Also notes the primary outputs of all of the surface's views
> > > + * Also notes all outputs of all of the surface's views
> > > * in the output_mask for the surface.
> > > */
> > > static void
> > > @@ -1136,8 +1135,7 @@ weston_surface_assign_output(struct weston_surface
> > > *es)
> > > *
> > > * Identifies the set of outputs that the view is visible on,
> > > * noting them into the output_mask. The output that the view
> > > - * is most visible on is set as the view's primary output for
> > > - * vsync and frame event purposes.
> > > + * is most visible on is set as the view's primary output.
> > > *
> > > * Also does the same for the view's surface. See
> > > * weston_surface_assign_output().
> > > diff --git a/src/compositor.h b/src/compositor.h
> > > index 7851000..0801f20 100644
> > > --- a/src/compositor.h
> > > +++ b/src/compositor.h
> > > @@ -949,8 +949,9 @@ struct weston_view {
> > > } transform;
> > >
> > > /*
> > > - * Which output to vsync this surface to.
> > > - * Used to determine, whether to send or queue frame events.
> > > + * The primary output for this view.
> > > + * Used for picking the output for driving view animations, inheriting
> >
> > Is the correct term "driving animations" or "drawing animations" ?
>
> It is actually "driving" here. One surface may be drawn on multiple
> outputs, but only one output will "drive" the animation.
>
> Driving the animation means to respond to frame callbacks, and since
> outputs may be rendered independently, we tie the animation of a surface
> to only one output.
>
> For example: a client rendering an animation will attach and commit a
> new buffer of a frame of its animation, and ask for a frame callback.
> Then it will wait for the frame callback until it draws, attaches and
> commits the buffer of the next frame of its animation.
>
> In order to get as good results as possible, meaning content drawn by
> the client should end up on the output as quickly as possible, we
> calculate the output it has the largest overlapping region on, and
> whenever that output was painted, we reply to the frame callback.
>
> If we for example would wait until all outputs the surface is one is
> drawn on until replying, we would potentially delay the frame callback,
> making the latency between the client drawing and the content being
> displayed on the output, unnecessarily long, since we would effectively
> let the output drawn last, no matter how large portion of the surface is
> drawn on it, always "drive" the animation.
That is all true for client-drawn animations.
However, weston also has an internal animation framework, see
src/animation.c, which primarily does view animations: changing view
parameters over time, like size, position, or opacity. Also this
animation framework is driven by a single chosen output of a view, for
the reasons Jonas explained.
To recap, client-drawn animations are driven by weston_surface::output
repaint cycle, and server-side view animations are driven by
weston_view::output repaint cycle.
Clients only "know" about weston_surface (wl_surface), they are
completely unaware of how many weston_views it might have. Therefore it
makes sense to drive the client repaint cycle with the client
perspective, and serv
