Re: [PATCH v3] protocol: Extend wl_touch with touchpoint shape and orientation

2016-04-18 Thread Jonas Ådahl
On Tue, Apr 19, 2016 at 02:49:50PM +1000, Peter Hutterer wrote:
> On Mon, Apr 18, 2016 at 12:36:39PM +0800, Jonas Ådahl wrote:
> > On Fri, Apr 15, 2016 at 08:01:35AM -0700, Dennis Kempin wrote:
> > > This CL updates the wl_touch interface with a shape and
> > > orientation event.
> > > The shape/orientation of a touch point is not relevant for most UI
> > > applications, but allows a better experience in some cases
> > > such as drawing apps.
> > > 
> > > The events are used by the compositor to inform the client
> > > about changes in the shape and orientation of a touchpoint, which is
> > > approximated by an ellipse and it's angle to the y-axis.
> > > 
> > > The event is optional and only sent when compositor and the
> > > touch device support this type of information. The client is
> > > responsible for making a reasonable assumption about the
> > > touch shape if no shape is reported.
> > > 
> > > Signed-off-by: Dennis Kempin 
> > > ---
> > >  protocol/wayland.xml | 64 
> > > 
> > >  1 file changed, 59 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > > index 12a6efd..b8a9835 100644
> > > --- a/protocol/wayland.xml
> > > +++ b/protocol/wayland.xml
> > > @@ -1656,7 +1656,7 @@
> > >  
> > > 
> > > 
> > > -  
> > > +  
> > >  
> > >A seat is a group of keyboards, pointer and touch devices. This
> > >object is published as a global during start up, or when such a
> > > @@ -1765,7 +1765,7 @@
> > > 
> > >
> > > 
> > > -  
> > > +  
> > >  
> > >The wl_pointer interface represents one or more input devices,
> > >such as mice, which control the pointer location and pointer_focus
> > > @@ -2078,7 +2078,7 @@
> > >  
> > >
> > > 
> > > -  
> > > +  
> > >  
> > >The wl_keyboard interface represents one or more keyboards
> > >associated with a seat.
> > > @@ -2192,7 +2192,7 @@
> > >  
> > >
> > > 
> > > -  
> > > +  
> > >  
> > >The wl_touch interface represents a touchscreen
> > >associated with a seat.
> > > @@ -2242,7 +2242,12 @@
> > > 
> > >  
> > >
> > > - Indicates the end of a contact point list.
> > > + Indicates the end of a contact point list. The wayland protocol requires
> > > + touch point updates to be sent sequentially, however all events within a
> > > + frame should be considered one hardware event. A wl_touch.frame 
> > > terminates
> > > + at least one event but otherwise no guarantee is provided about the set 
> > > of
> > > + events within a frame. A client must assume that any state not updated 
> > > in a
> > > + frame is unchanged from the previously known state.
> > 
> > Seems to be some formatting error. This paragraph should be indented
> > with one tab, not one space. Same with the description in "shape" and
> > "orientation".
> > 
> > I think this part should be a separate patch. Also no need to point out
> > that this is the wayland protocol. I'd also suggest avoiding stating
> > that they are considered one hardware event. It'd probably be better to
> > follow what wl_pointer says here and say that they "logically belong
> > together". Check out wl_pointer.frame for some inspiration.
> 
> ftr, that's is the wording we used in the tablet protocol for the frame
> events:
> Marks the end of a series of axis and/or button updates from the
> tablet. The wayland protocol requires axis updates to be sent
> sequentially, however all events within a frame should be considered
> one hardware event.
> but I agree that the wl_pointer.frame wording is better here.
>  
> > >
> > >  
> > > 
> > > @@ -2262,6 +2267,55 @@
> > >  
> > >
> > >  
> > > +
> > > +
> > > +
> > > +
> > > +  
> > > + Sent when a touchpoint has changed its shape. If the touch position
> > > + or orientation changed at the same time, the wl_touch.motion,
> > > + wl_touch.orientation and wl_touch.shape are sent within the same
> > > + wl_touch.frame. Otherwise, only a wl_touch.shape is sent within this
> > > + wl_touch.frame. The protocol does not guarantee specific ordering of
> > > + wl_touch.orientation, wl_touch.shape and wl_touch.motion events.
> > 
> > I'd suggest changing this paragraph to be more clear that the
> > non-guarantees of what may be sent together, while avoiding the wording
> > "at the same time". Something like
> > 
> > Sent when a touchpoint has changed its shape.
> > 
> > This event event does not occur on its own. It is sent before a
> > wl_touch.frame event, and carries the new shape information for
> > the specific touch points for that frame.
> 
> I don't think you need the ',' here
> 
> > 
> > This event may be sent alone or together with other events
> > describing the touch point, such as wl_touch.motion and
> > wl_touch.orientation. The order of wl_touch.shape,
> > wl_

[PATCH v2 wayland-protocols] Add pad support to the tablet protocol

2016-04-18 Thread Peter Hutterer
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).

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 v1:
- typos fixed 

 unstable/tablet/tablet-unstable-v1.xml | 423 -
 1 file changed, 421 insertions(+), 2 deletions(-)

diff --git a/unstable/tablet/tablet-unstable-v1.xml 
b/unstable/tablet/tablet-unstable-v1.xml
index 907126c..36b9ae8 100644
--- a/unstable/tablet/tablet-unstable-v1.xml
+++ b/unstable/tablet/tablet-unstable-v1.xml
@@ -115,7 +115,7 @@
 interface version number is reset.
   
 
-  
+  
 
   An object that provides access to the graphics tablets available on this
   system. All tablets are associated with a seat, to get access to the
@@ -139,7 +139,7 @@
 
   
 
-  
+  
 
   An object that provides access to the graphics tablets available on this
   seat. After binding to this interface, the compositor sends a set of
@@ -172,6 +172,23 @@
   
   
 
+
+
+
+
+  
+   This event is sent whenever a new pad is known to the system. Typically,
+   pads are physically attached to tablets and a pad_added event is
+   sent immediately after the wp_tablet_seat.tablet_added.
+   However, some standalone pad devices logically attach to tablets at
+   runtime, the client must wait for wp_tablet_pad.enter to know the
+   tablet a pad is attached to.
+
+   This event only provides the object id of the pad, any further features
+   (buttons, strips, rings) is sent through the wp_tablet_pad interface.
+  
+  
+
   
 
   
@@ -638,4 +655,406 @@
 
   
 
+  
+
+  A circular interaction area.
+
+  Events on a ring are logically grouped by the wl_tablet_pad_ring.frame
+  event.
+
+
+
+  
+   Requests the compositor to use the provided feedback UTF-8 encoded
+   string associated with this ring.
+
+   Clients are encouraged to provide context-aware descriptions for
+   the actions associated with the ring, compositors may use this
+   information to offer visual feedback about the button layout
+   (eg. on-screen displays).
+
+   The string offered is considered user visible; general
+   internationalization rules apply.
+
+   This request should be issued immediately after a
+   wp_tablet_pad.enter, or whenever the ring is mapped to a different
+   action.
+  
+  
+
+
+
+  
+   This destroys the client's resource for this ring object.
+  
+
+
+
+  
+   Describes the source types for ring events. This indicates to the
+   client how a ring event was physically generated; a client may
+   adjust the user interface accordingly. For example, events
+   from a "finger" source may trigger kinetic scrolling.
+  
+  
+
+
+
+  
+   Source information for ring events.
+
+   This event does not occur on its own. It is sent before a
+   wp_tablet_pad_ring.frame event and carries the source information
+   for all events within that frame.
+
+   The source specifies how this event was generated. If the source is
+   wp_tablet_pad_ring.source.finger, a wp_tablet_pad_ring.stop event
+   will be sent when the user lifts the finger off the device.
+
+   This event is optional. If the source is unknown for an interaction,
+   no event is sent.
+  
+  
+
+
+
+  
+   Sent whenever the angle on a ring changes.
+
+   The angle is provided in 0.01 of a degree clockwise from the logical
+   north of the ring in the pad's current rotation.
+  
+  
+
+
+
+  
+   Stop notification for ring events.
+
+   For some wp_tablet_pad_ring.source types, a wp_tablet_pad_ring.stop
+   event is sent to notify a client that the interaction with the ring
+   has terminated.
+   This enables the client to implement kinetic scrolling.
+   See the wp_tablet_pad_ring.source documentation for information on
+   when this event may be generated.
+
+   Any wp_tablet_pad_ring.angle events with the same source after this
+   event should be considered as the start of a new interaction.
+  
+  
+
+
+
+  
+   Indicates the end of a set

Re: [PATCH v3] protocol: Extend wl_touch with touchpoint shape and orientation

2016-04-18 Thread Peter Hutterer
On Mon, Apr 18, 2016 at 12:36:39PM +0800, Jonas Ådahl wrote:
> On Fri, Apr 15, 2016 at 08:01:35AM -0700, Dennis Kempin wrote:
> > This CL updates the wl_touch interface with a shape and
> > orientation event.
> > The shape/orientation of a touch point is not relevant for most UI
> > applications, but allows a better experience in some cases
> > such as drawing apps.
> > 
> > The events are used by the compositor to inform the client
> > about changes in the shape and orientation of a touchpoint, which is
> > approximated by an ellipse and it's angle to the y-axis.
> > 
> > The event is optional and only sent when compositor and the
> > touch device support this type of information. The client is
> > responsible for making a reasonable assumption about the
> > touch shape if no shape is reported.
> > 
> > Signed-off-by: Dennis Kempin 
> > ---
> >  protocol/wayland.xml | 64 
> > 
> >  1 file changed, 59 insertions(+), 5 deletions(-)
> > 
> > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > index 12a6efd..b8a9835 100644
> > --- a/protocol/wayland.xml
> > +++ b/protocol/wayland.xml
> > @@ -1656,7 +1656,7 @@
> >  
> > 
> > 
> > -  
> > +  
> >  
> >A seat is a group of keyboards, pointer and touch devices. This
> >object is published as a global during start up, or when such a
> > @@ -1765,7 +1765,7 @@
> > 
> >
> > 
> > -  
> > +  
> >  
> >The wl_pointer interface represents one or more input devices,
> >such as mice, which control the pointer location and pointer_focus
> > @@ -2078,7 +2078,7 @@
> >  
> >
> > 
> > -  
> > +  
> >  
> >The wl_keyboard interface represents one or more keyboards
> >associated with a seat.
> > @@ -2192,7 +2192,7 @@
> >  
> >
> > 
> > -  
> > +  
> >  
> >The wl_touch interface represents a touchscreen
> >associated with a seat.
> > @@ -2242,7 +2242,12 @@
> > 
> >  
> >
> > - Indicates the end of a contact point list.
> > + Indicates the end of a contact point list. The wayland protocol requires
> > + touch point updates to be sent sequentially, however all events within a
> > + frame should be considered one hardware event. A wl_touch.frame terminates
> > + at least one event but otherwise no guarantee is provided about the set of
> > + events within a frame. A client must assume that any state not updated in 
> > a
> > + frame is unchanged from the previously known state.
> 
> Seems to be some formatting error. This paragraph should be indented
> with one tab, not one space. Same with the description in "shape" and
> "orientation".
> 
> I think this part should be a separate patch. Also no need to point out
> that this is the wayland protocol. I'd also suggest avoiding stating
> that they are considered one hardware event. It'd probably be better to
> follow what wl_pointer says here and say that they "logically belong
> together". Check out wl_pointer.frame for some inspiration.

ftr, that's is the wording we used in the tablet protocol for the frame
events:
Marks the end of a series of axis and/or button updates from the
tablet. The wayland protocol requires axis updates to be sent
sequentially, however all events within a frame should be considered
one hardware event.
but I agree that the wl_pointer.frame wording is better here.
 
> >
> >  
> > 
> > @@ -2262,6 +2267,55 @@
> >  
> >
> >  
> > +
> > +
> > +
> > +
> > +  
> > + Sent when a touchpoint has changed its shape. If the touch position
> > + or orientation changed at the same time, the wl_touch.motion,
> > + wl_touch.orientation and wl_touch.shape are sent within the same
> > + wl_touch.frame. Otherwise, only a wl_touch.shape is sent within this
> > + wl_touch.frame. The protocol does not guarantee specific ordering of
> > + wl_touch.orientation, wl_touch.shape and wl_touch.motion events.
> 
> I'd suggest changing this paragraph to be more clear that the
> non-guarantees of what may be sent together, while avoiding the wording
> "at the same time". Something like
> 
>   Sent when a touchpoint has changed its shape.
> 
>   This event event does not occur on its own. It is sent before a
>   wl_touch.frame event, and carries the new shape information for
>   the specific touch points for that frame.

I don't think you need the ',' here

>   
>   This event may be sent alone or together with other events
>   describing the touch point, such as wl_touch.motion and
>   wl_touch.orientation. The order of wl_touch.shape,
>   wl_touch.orientation and wl_touch.motion is not guaranteed.

"alone" and "together" aren't perfectly defined either, how about:
Other events describing the touch point such as wl_touch.motion or
wl_touch.orientation may be sent within the same wl_touch.frame. A
client should treat these events as a single logi

Questions about disabling key repeat for a keycode or a keysym

2016-04-18 Thread 박성진
Title: Samsung Enterprise Portal mySingle


Dear all,
I wonder how I can disable key repeat for a specific keycode or a specific keysym.
We're using EFL (ecore wayland) to process key events and every keys are being repeated
based on the values like repeat delay, rate getting from wayland compositor.
I'd like to make some of keys not to be repeated.
As far as I know, there is not API for setting repeat flag for a keycode or a keysym.
Do I need to do something inside xkeyboard-config or to do with libxkbcommon API ?
Any suggestions ? :)
 
Thanks in advance,
Sung-Jin Park___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v8 xdg-shell-unstable-v6] xdg-shell: Add min/max size requests

2016-04-18 Thread Jonas Ådahl
On Mon, Apr 18, 2016 at 09:19:48AM +0200, Olivier Fourdan wrote:
> Some application may wish to restrict their window in size, but
> xdg-shell has no mechanism for the client to specify a maximum or
> minimum size.
> 
> As a result, the compositor may try to maximize or fullscreen a window
> while the client would not allow for the requested size.
> 
> Add new requests "set_max_size" and "set_min_size" to xdg-shell so that
> the client can tell the compositor what would be its smallest/largest
> acceptable size, and that the compositor can decide if maximize or
> fullscreen is achievable, draw an accurate animation, etc.
> 
> Signed-off-by: Olivier Fourdan 

Reviewed-by: Jonas Ådahl  with one nit below. If
everyone is fine with the semantics with this version, I'll go ahead and
merge this to the wip/xdg-shell-unstable-v6 branch.

> Bugzilla: https://bugzilla.gnome.org/show_bug.cgi?id=764413
> ---
>  v2: Rename the request to "set_preferred_max_size",
>  add "set_preferred_min_size" as well
>  v3: Rebase above patch 72427 in branch xdg-shell-unstable-v6
>  Rephrase description to clarify the unscaled size and using 0 to
>  reset back the preferred size to an unspecified state
>  v4: Patch the correct xml file (v6, not v5 )
>  Fix multiple mismatch of min/max in the description
>  Remove mention of "unscaled", specify window geometry coordinates
>  and refer to set_window_geometry.
>  v5: Fix typos and remove "preferred" from the name and description as
>  requested by several people on the ML and irc.
>  v6: Specify the requests are double-buffered and require a commit,
>  rephrase the values never set as suggested by Jasper, state
>  that min > max is an invalid state and result in a protocol error
>  as suggested by Bill, Yong and Jonas.
>  v7: Specify that width/height values must be greater than or equal to
>  zero as discussed with Mike.
>  v8: Explicitely state that the compositor may ignore the mix/max size.
> 
>  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 88 
> 
>  1 file changed, 88 insertions(+)
> 
> diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
> b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> index 3fc7d42..1448c2c 100644
> --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> @@ -489,6 +489,94 @@
>
>  
>  
> +
> +  
> + Set a maximum size for the window.
> +
> + The client can specify a maximum size so that the compositor does
> + not try to configure the window beyond this size.
> +
> + The width and height arguments are in window geometry coordinates.
> + See set_window_geometry.
> +
> + Values set in this way are double-buffered. They will get applied
> + on the next commit.
> +
> + The compositor can use this information to allow or disallow
> + different states like maximize or fullscreen and draw accurate
> + animations.
> +
> + Similarly, a tiling window manager may use this information to
> + place and resize client windows in a more effective way.
> +
> + The client should not rely on the compositor to obey the maximum
> + size. The compositor may decide to ignore the values set by the
> + client and request a larger size. In such a case, the client can
> + either adapt to the requested size or refuse to acknowledge the
> + configure event sent by the compositor. See xdg_surface.configure
> + and xdg_surface.ack_configure for details.

Can probably remove everything after ".. the value set by the client
and request a larger size." since it goes on to try to describe the
configure event semantics. Not sure they are entirely correctly spelled
out here, so to avoid stalling this patch even further, I suggest to
drop this part (and the same for set_min_size).


Jonas

> +
> + If never set, or a value of zero in the request, means that the
> + client has no expected maximum size in the given dimension.
> + As a result, a client wishing to reset the maximum size
> + to an unspecified state can use zero for width and height in the
> + request.
> +
> + Requesting a maximum size to be smaller than the minimum size of
> + a surface is illegal and will result in a protocol error.
> +
> + The width and height must be greater than or equal to zero. Using
> + strictly negative values for width and height will result in a
> + protocol error.
> +  
> +  
> +  
> +
> +
> +
> +  
> + Set a minimum size for the window.
> +
> + The client can specify a minimum size so that the compositor does
> + not try to configure the window below this size.
> +
> + The width and height arguments are in window geometry coordinates.
> + See set_window_geometry.
> +
> + Values set in this way are double-buffered. They will get applied
> + on the next commit.
> +
> + The compositor can use this inf

Re: [PATCH v8 xdg-shell-unstable-v6] xdg-shell: Add min/max size requests

2016-04-18 Thread Jonas Ådahl
On Mon, Apr 18, 2016 at 10:53:42AM -0700, Bill Spitzak wrote:
> On Mon, Apr 18, 2016 at 12:19 AM, Olivier Fourdan 
> wrote:
> 
> >
> > +   The client should not rely on the compositor to obey the maximum
> > +   size. The compositor may decide to ignore the values set by the
> > +   client and request a larger size. In such a case, the client can
> > +   either adapt to the requested size or refuse to acknowledge the
> > +   configure event sent by the compositor. See xdg_surface.configure
> > +   and xdg_surface.ack_configure for details.
> >
> 
> The client is allowed to also select a different size, not just "refuse to
> acknowledge the configure event". This wording implies that if the client
> is smaller than it's maximum, and the compositor requests larger than the
> maximum, the only possible things a client can do is use the
> compositor-requested size, or leave the surface smaller than the maximum.
> Certainly resizing to the maximum is also allowed.
> 
> Real clients only "refuse to acknowledge the configure event" after they
> calculate the size they want and discover it is equal to the current size.
> This can happen for any configure event, there is nothing special about
> these out-of-range ones.
> 
> Better wording: "In such a case, the client can either adapt to the
> requested size or choose a different size (perhaps the maximum). See
> xdg_surface.configure for details."

All of that is not really related to these new requests. If
xdg_surface.configuration needs clarification, it can be done
separately.

> 
> +   Requesting a maximum size to be smaller than the minimum size of
> > +   a surface is illegal and will result in a protocol error.
> >
> 
> I think you have to add "when the commit request is sent". I still think
> the current wording does not make it clear that it can be temporarily
> incorrect and that clients do not have to remember the previous setting and
> reorder these requests to avoid the error.

I don't think its needed. It should be quite obvious that this is about
the requested state to be applied.


Jonas
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[ANNOUNCE] libinput 1.2.4

2016-04-18 Thread Peter Hutterer
libinput 1.2.4 is now available. 

The top software button area on the T440-series touchpads is now 30mm high
when the touchpad is disabled to make it easier to hit those buttons. No
changes when the touchpad is active.

The udev hwdb entries for the Chromebooks were updated to accommodate for
udev's silent replacing of non-alphanumeric characters with '_'. Thus, we
now have touchpads on chromebooks tagged correctly.

We also added a fuzz filter to tablet devices. Previously, a cursor tool
left on a tablet would cause continuous tiny subpixel motion. Now we're
using the kernel's fuzz value to ignore those sensor wobbles.

As usual, the git shortlog is below.

Peter Frühberger (1):
  touchpad: enlarge top button area by a factor 3 instead of 1.5

Peter Hutterer (4):
  test: reduce the default abs-max range to avoid ENOMEM
  tablet: add a fuzz-filter to avoid spamming callers with subpixel updates
  udev: update the hwdb matches to avoid use of ( and )
  libinput 1.2.4

git tag: 1.2.4

http://www.freedesktop.org/software/libinput/libinput-1.2.4.tar.xz
MD5:  1cbaa34f04a336f2703906d564e0a37a  libinput-1.2.4.tar.xz
SHA1: a39b5b75a317b06dd8e950c0614cdf06cd92e9b6  libinput-1.2.4.tar.xz
SHA256: aee3650ad2a864ab9a10e7e63df543cc2b475f6bf3974751037a2df325dabbb1  
libinput-1.2.4.tar.xz
PGP:  http://www.freedesktop.org/software/libinput/libinput-1.2.4.tar.xz.sig



signature.asc
Description: PGP signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v2] protocol: Extend wl_touch with touchpoint shape event

2016-04-18 Thread Peter Hutterer
On Mon, Apr 18, 2016 at 04:49:46PM +0200, Andreas Pokorny wrote:
> Hi,
> Some questions below..
> 
> On Wed, Apr 6, 2016 at 11:52 PM, Peter Hutterer 
> wrote:
> 
> > On Wed, Apr 06, 2016 at 10:17:35AM -0700, Dennis Kempin wrote:
> > > On Tue, Apr 5, 2016 at 5:26 PM, Peter Hutterer 
> > wrote:
> > > > On Tue, Apr 05, 2016 at 01:09:31PM -0700, Dennis Kempin wrote:
> > > >> [...]
> > > >> +  
> > > >> +  
> > > >> +  
> > > >> +  
> > > >> +   > > >> +   summary="angle between major axis and surface x-axis in
> > degrees"/>
> > > >> +
> >
> 
> Jumping on the major/minor train. Is there no interest in pressure values?
> It seems that more accurate touchscreens that may offer reproducible
> pressure values
> become mainstream..

separate value -> separate event :)

 
> > > [...]
> 
> > > Maybe we could include a fallback event for devices that do not have
> > > good support?
> > > One that would only use a single value to describe the size of a
> > > touch, normalized
> > > to 0..1. In practice, I do not think we could put any more requirements
> > on this
> > > value. There is no way for us to tell how tell if a device is using
> > > the full range
> > > (0 to reported max axis value), and in a bad case we might end up with
> > that
> > > size being in the range of 0 to 0.1.
> > >
> > > Let me know what you think about this.
> >
> > I'm mostly thinking about how to enable this in libinput from a technical
> > perspective, and the obvious solution would be whitelisting or blacklisting
> > devices, combined with the calibration you use as well.
> >
> > the fallback wouldn't be normalized but simply skipping it where we can't
> > be
> > sure. to do that, we need to do one thing though: split up the events into
> > area and orientation as separate events. we can send orientation even when
> > the area is unknown.
> >
> > how does that sound?
> >
> 
> Are you referring to libinput events or wayland events here? Why would
> splitting the events help?

it's better to not send an event than sending magic values that
the client then needs to interpret as "undefined". so splitting them up
gives us the option of sending only orientation on those touchpads where the
major/minor values are garbage.
orientation and skip the major/minor event.

> I have also seen touchscreens just offering a major axis and an
> orientation. That is an edge
> case but even there we could have finger-heuristic to estimate a minor
> value..

that is a case where IMO assuming it's a circle is sufficient. but this
assumption should be on the lower level anyway, not in wayland.

Cheers,
   Peter

> @Dennis: I am curious how complex the contact size calibration function in
> chrome os is.
> 
> regards
> Andreas
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v6 09/12] drm: Move drm's output configuration into the drm backend

2016-04-18 Thread Bryce Harrington
On Sun, Apr 17, 2016 at 01:26:58PM +0300, Giulio Camuffo wrote:
> Hi,
> 
> I don't understand this one at all, you're moving configuration back
> into the backend, partially reverting patch 5.

This was done to implement the change Benoit requested in the v5 series
for tracking gbm_format as its enum type rather than as string.
 
> 2016-04-16 6:28 GMT+03:00 Bryce Harrington :
> > Signed-off-by: Bryce Harrington 
> > ---
> >  src/compositor-drm.c | 93 
> > 
> >  src/compositor-drm.h | 41 ---
> >  src/main.c   | 43 
> >  3 files changed, 80 insertions(+), 97 deletions(-)
> >
> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > index 6ef706a..d129adc 100644
> > --- a/src/compositor-drm.c
> > +++ b/src/compositor-drm.c
> > @@ -75,6 +75,41 @@
> >  #define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
> >  #endif
> >
> > +enum weston_drm_backend_output_mode {
> > +   /** The output is disabled */
> > +   WESTON_DRM_BACKEND_OUTPUT_OFF,
> > +
> > +   /** The output will use the current active mode */
> > +   WESTON_DRM_BACKEND_OUTPUT_CURRENT,
> > +
> > +   /** The output will use the preferred mode. A modeline can be 
> > provided
> > +* by setting weston_backend_output_config::modeline in the form of
> > +* "WIDTHxHEIGHT" or in the form of an explicit modeline calculated
> > +* using e.g. the cvt tool. If a valid modeline is supplied it will 
> > be
> > +* used, if invalid or NULL the preferred available mode will be 
> > used. */
> > +   WESTON_DRM_BACKEND_OUTPUT_PREFERRED,
> > +};
> > +
> > +struct weston_drm_backend_output_config {
> > +   struct weston_backend_output_config base;
> > +
> > +   /** The pixel format to be used by the output. Valid values are:
> > +* - NULL - The format set at backend creation time will be used
> > +* - "xrgb"
> > +* - "rgb565"
> > +* - "xrgb2101010"
> > +*/
> > +   char *gbm_format;
> > +
> > +   /** The seat to be used by the output. Set to NULL to use the
> > +* default seat. */
> > +   char *seat;
> > +
> > +   /** The modeline to be used by the output. Refer to the 
> > documentation
> > +* of WESTON_DRM_BACKEND_OUTPUT_PREFERRED for details. */
> > +   char *modeline;
> > +};
> > +
> >  struct drm_backend {
> > struct weston_backend base;
> > struct weston_compositor *compositor;
> > @@ -121,16 +156,6 @@ struct drm_backend {
> > int32_t cursor_width;
> > int32_t cursor_height;
> >
> > -/** Callback used to configure the outputs.
> > -*
> > - * This function will be called by the backend when a new DRM
> > - * output needs to be configured.
> > - */
> > -enum weston_drm_backend_output_mode
> > -   (*configure_output)(struct weston_compositor *compositor,
> > -   bool use_current_mode,
> > -   const char *name,
> > -   struct weston_drm_backend_output_config 
> > *output_config);
> > bool use_current_mode;
> >  };
> >
> > @@ -2275,6 +2300,49 @@ connector_get_current_mode(drmModeConnector 
> > *connector, int drm_fd,
> > return 0;
> >  }
> >
> > +static enum weston_drm_backend_output_mode
> > +drm_configure_output(struct weston_compositor *c,
> > + bool use_current_mode,
> > + const char *name,
> > + struct weston_drm_backend_output_config *config)
> > +{
> > +   struct weston_config *wc = weston_compositor_get_user_data(c);
> 
> This is wrong, you're assuming the user data is a weston_config, which
> won't be true for non-weston compositors.
> 
> Sorry, but NAK from me.
> 
> 
> Giulio
> 
> > +   struct weston_config_section *section;
> > +   char *s;
> > +   int scale;
> > +   enum weston_drm_backend_output_mode mode =
> > +   WESTON_DRM_BACKEND_OUTPUT_PREFERRED;
> > +
> > +   section = weston_config_get_section(wc, "output", "name", name);
> > +   weston_config_section_get_string(section, "mode", &s, "preferred");
> > +   if (strcmp(s, "off") == 0) {
> > +   free(s);
> > +   return WESTON_DRM_BACKEND_OUTPUT_OFF;
> > +   }
> > +
> > +   if (use_current_mode || strcmp(s, "current") == 0) {
> > +   mode = WESTON_DRM_BACKEND_OUTPUT_CURRENT;
> > +   } else if (strcmp(s, "preferred") != 0) {
> > +   config->modeline = s;
> > +   s = NULL;
> > +   }
> > +   free(s);
> > +
> > +   weston_config_section_get_int(section, "scale", &scale, 1);
> > +   config->base.scale = scale >= 1 ? scale : 1;
> > +   weston_config_section_get_string(section, "transform", &s, 
> > "normal");
> > +   if (weston_parse_transform(s, &config->base.transform) < 0)
> > +   weston_

Re: [PATCH weston v6 10/12] drm: Pass gbm_format as enum rather than string in output config

2016-04-18 Thread Pekka Paalanen
On Mon, 18 Apr 2016 17:52:55 +0200
Benoit Gschwind  wrote:

> Le 18/04/2016 16:50, Pekka Paalanen a écrit :
> > On Fri, 15 Apr 2016 20:28:35 -0700
> > Bryce Harrington  wrote:
> >   
> >> Signed-off-by: Bryce Harrington 
> >> ---
> >>  src/compositor-drm.c | 23 +--
> >>  1 file changed, 13 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> >> index d129adc..a896785 100644
> >> --- a/src/compositor-drm.c
> >> +++ b/src/compositor-drm.c
> >> @@ -93,13 +93,13 @@ enum weston_drm_backend_output_mode {
> >>  struct weston_drm_backend_output_config {
> >>struct weston_backend_output_config base;
> >>  
> >> -  /** The pixel format to be used by the output. Valid values are:
> >> -   * - NULL - The format set at backend creation time will be used
> >> -   * - "xrgb"
> >> -   * - "rgb565"
> >> -   * - "xrgb2101010"
> >> +  /** The pixel format to be used by the output. Supported values are:
> >> +   * - 0: The format set at backend creation time will be used
> >> +   * - GBM_FORMAT_XRGB
> >> +   * - GBM_FORMAT_RGB565
> >> +   * - GBM_FORMAT_XRGB2101010
> >> */
> >> -  char *gbm_format;
> >> +  uint32_t gbm_format;
> >>  
> >>/** The seat to be used by the output. Set to NULL to use the
> >> * default seat. */
> >> @@ -2336,8 +2336,12 @@ drm_configure_output(struct weston_compositor *c,
> >>   s, name);
> >>free(s);
> >>  
> >> -  weston_config_section_get_string(section,
> >> -   "gbm-format", &config->gbm_format, 
> >> NULL);
> >> +  weston_config_section_get_string(section, "gbm-format", &s, NULL);
> >> +if (parse_gbm_format(s, GBM_FORMAT_XRGB, &config->gbm_format) 
> >> < 0)
> >> +  weston_log("Invalid gbm-format \"%s\" for output %s\n",
> >> + s, name);
> >> +  free(s);
> >> +
> >>weston_config_section_get_string(section, "seat", &config->seat, "");
> >>return mode;
> >>  }
> >> @@ -2391,8 +2395,7 @@ create_output_for_connector(struct drm_backend *b,
> >>  
> >>mode = drm_configure_output(b->compositor, b->use_current_mode,
> >>output->base.name, &config);
> >> -  if (parse_gbm_format(config.gbm_format, b->gbm_format, 
> >> &output->gbm_format) == -1)
> >> -  output->gbm_format = b->gbm_format;
> >> +  output->gbm_format = config.gbm_format;
> >>  
> >>setup_output_seat_constraint(b, &output->base,
> >> config.seat ? config.seat : "");  
> > 
> > Hi,
> > 
> > we cannot do this, because the backend or anything in libweston will
> > not have access to the weston_config_*() API at all.
> > 
> > I would not like to #include  in main.c either, if we just can
> > reasonably avoid it.
> > 
> > Drawing the line here is vague, and we might improve later. OTOH, I
> > certainly don't want libweston to have its own enumeration of pixel
> > formats either, though time will tell if that is avoidable.
> > 
> > 
> > Thanks,
> > pq
> >   
> 
> Hello Pekka,
> 
> I think what ever you try in that case, you have 2 choice when using a
> library under your own one:
>  1. you choose to hide anything under your library
>  2. or, you leave the user to know that your are using those library and
> just include this library in your own libfoo.h
> 
> IMO, it's a waste time to try to hide library dependency. In particular
> in the case of libweston, that will be more or less a glue library.

That's true, but let's not be hasty. We don't have to do that in
this series right now.

Another common pixel format enumeration is that of Pixman's, and
that is not backend-specific, while libdrm and GBM are. But I still
would not shove Pixman formats just because of that. Wayland core
protocol also has a huge list of pixel formats that could do as
well.

The most logical would be for DRM backend to use DRM pixel formats.

I just don't want to mix that discussion into this basic conversion
series.


Thanks,
pq


pgpCee9bSoMMh.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 v6 10/12] drm: Pass gbm_format as enum rather than string in output config

2016-04-18 Thread Benoit Gschwind
Le 18/04/2016 16:50, Pekka Paalanen a écrit :
> On Fri, 15 Apr 2016 20:28:35 -0700
> Bryce Harrington  wrote:
> 
>> Signed-off-by: Bryce Harrington 
>> ---
>>  src/compositor-drm.c | 23 +--
>>  1 file changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
>> index d129adc..a896785 100644
>> --- a/src/compositor-drm.c
>> +++ b/src/compositor-drm.c
>> @@ -93,13 +93,13 @@ enum weston_drm_backend_output_mode {
>>  struct weston_drm_backend_output_config {
>>  struct weston_backend_output_config base;
>>  
>> -/** The pixel format to be used by the output. Valid values are:
>> - * - NULL - The format set at backend creation time will be used
>> - * - "xrgb"
>> - * - "rgb565"
>> - * - "xrgb2101010"
>> +/** The pixel format to be used by the output. Supported values are:
>> + * - 0: The format set at backend creation time will be used
>> + * - GBM_FORMAT_XRGB
>> + * - GBM_FORMAT_RGB565
>> + * - GBM_FORMAT_XRGB2101010
>>   */
>> -char *gbm_format;
>> +uint32_t gbm_format;
>>  
>>  /** The seat to be used by the output. Set to NULL to use the
>>   * default seat. */
>> @@ -2336,8 +2336,12 @@ drm_configure_output(struct weston_compositor *c,
>> s, name);
>>  free(s);
>>  
>> -weston_config_section_get_string(section,
>> - "gbm-format", &config->gbm_format, 
>> NULL);
>> +weston_config_section_get_string(section, "gbm-format", &s, NULL);
>> +if (parse_gbm_format(s, GBM_FORMAT_XRGB, &config->gbm_format) < 
>> 0)
>> +weston_log("Invalid gbm-format \"%s\" for output %s\n",
>> +   s, name);
>> +free(s);
>> +
>>  weston_config_section_get_string(section, "seat", &config->seat, "");
>>  return mode;
>>  }
>> @@ -2391,8 +2395,7 @@ create_output_for_connector(struct drm_backend *b,
>>  
>>  mode = drm_configure_output(b->compositor, b->use_current_mode,
>>  output->base.name, &config);
>> -if (parse_gbm_format(config.gbm_format, b->gbm_format, 
>> &output->gbm_format) == -1)
>> -output->gbm_format = b->gbm_format;
>> +output->gbm_format = config.gbm_format;
>>  
>>  setup_output_seat_constraint(b, &output->base,
>>   config.seat ? config.seat : "");
> 
> Hi,
> 
> we cannot do this, because the backend or anything in libweston will
> not have access to the weston_config_*() API at all.
> 
> I would not like to #include  in main.c either, if we just can
> reasonably avoid it.
> 
> Drawing the line here is vague, and we might improve later. OTOH, I
> certainly don't want libweston to have its own enumeration of pixel
> formats either, though time will tell if that is avoidable.
> 
> 
> Thanks,
> pq
> 

Hello Pekka,

I think what ever you try in that case, you have 2 choice when using a
library under your own one:
 1. you choose to hide anything under your library
 2. or, you leave the user to know that your are using those library and
just include this library in your own libfoo.h

IMO, it's a waste time to try to hide library dependency. In particular
in the case of libweston, that will be more or less a glue library.

In other term, I in favor to keep enums, even if they come from a
foreign library.

Best regards.

> 
> 
> ___
> 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 v6 10/12] drm: Pass gbm_format as enum rather than string in output config

2016-04-18 Thread Pekka Paalanen
On Fri, 15 Apr 2016 20:28:35 -0700
Bryce Harrington  wrote:

> Signed-off-by: Bryce Harrington 
> ---
>  src/compositor-drm.c | 23 +--
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index d129adc..a896785 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -93,13 +93,13 @@ enum weston_drm_backend_output_mode {
>  struct weston_drm_backend_output_config {
>   struct weston_backend_output_config base;
>  
> - /** The pixel format to be used by the output. Valid values are:
> -  * - NULL - The format set at backend creation time will be used
> -  * - "xrgb"
> -  * - "rgb565"
> -  * - "xrgb2101010"
> + /** The pixel format to be used by the output. Supported values are:
> +  * - 0: The format set at backend creation time will be used
> +  * - GBM_FORMAT_XRGB
> +  * - GBM_FORMAT_RGB565
> +  * - GBM_FORMAT_XRGB2101010
>*/
> - char *gbm_format;
> + uint32_t gbm_format;
>  
>   /** The seat to be used by the output. Set to NULL to use the
>* default seat. */
> @@ -2336,8 +2336,12 @@ drm_configure_output(struct weston_compositor *c,
>  s, name);
>   free(s);
>  
> - weston_config_section_get_string(section,
> -  "gbm-format", &config->gbm_format, 
> NULL);
> + weston_config_section_get_string(section, "gbm-format", &s, NULL);
> +if (parse_gbm_format(s, GBM_FORMAT_XRGB, &config->gbm_format) < 
> 0)
> + weston_log("Invalid gbm-format \"%s\" for output %s\n",
> +s, name);
> + free(s);
> +
>   weston_config_section_get_string(section, "seat", &config->seat, "");
>   return mode;
>  }
> @@ -2391,8 +2395,7 @@ create_output_for_connector(struct drm_backend *b,
>  
>   mode = drm_configure_output(b->compositor, b->use_current_mode,
>   output->base.name, &config);
> - if (parse_gbm_format(config.gbm_format, b->gbm_format, 
> &output->gbm_format) == -1)
> - output->gbm_format = b->gbm_format;
> + output->gbm_format = config.gbm_format;
>  
>   setup_output_seat_constraint(b, &output->base,
>config.seat ? config.seat : "");

Hi,

we cannot do this, because the backend or anything in libweston will
not have access to the weston_config_*() API at all.

I would not like to #include  in main.c either, if we just can
reasonably avoid it.

Drawing the line here is vague, and we might improve later. OTOH, I
certainly don't want libweston to have its own enumeration of pixel
formats either, though time will tell if that is avoidable.


Thanks,
pq


pgpsnn7aQCTg4.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v2] protocol: Extend wl_touch with touchpoint shape event

2016-04-18 Thread Andreas Pokorny
Hi,
Some questions below..

On Wed, Apr 6, 2016 at 11:52 PM, Peter Hutterer 
wrote:

> On Wed, Apr 06, 2016 at 10:17:35AM -0700, Dennis Kempin wrote:
> > On Tue, Apr 5, 2016 at 5:26 PM, Peter Hutterer 
> wrote:
> > > On Tue, Apr 05, 2016 at 01:09:31PM -0700, Dennis Kempin wrote:
> > >> [...]
> > >> +  
> > >> +  
> > >> +  
> > >> +  
> > >> +   > >> +   summary="angle between major axis and surface x-axis in
> degrees"/>
> > >> +
>

Jumping on the major/minor train. Is there no interest in pressure values?
It seems that more accurate touchscreens that may offer reproducible
pressure values
become mainstream..


> > [...]

> > Maybe we could include a fallback event for devices that do not have
> > good support?
> > One that would only use a single value to describe the size of a
> > touch, normalized
> > to 0..1. In practice, I do not think we could put any more requirements
> on this
> > value. There is no way for us to tell how tell if a device is using
> > the full range
> > (0 to reported max axis value), and in a bad case we might end up with
> that
> > size being in the range of 0 to 0.1.
> >
> > Let me know what you think about this.
>
> I'm mostly thinking about how to enable this in libinput from a technical
> perspective, and the obvious solution would be whitelisting or blacklisting
> devices, combined with the calibration you use as well.
>
> the fallback wouldn't be normalized but simply skipping it where we can't
> be
> sure. to do that, we need to do one thing though: split up the events into
> area and orientation as separate events. we can send orientation even when
> the area is unknown.
>
> how does that sound?
>

Are you referring to libinput events or wayland events here? Why would
splitting the events help?
I have also seen touchscreens just offering a major axis and an
orientation. That is an edge
case but even there we could have finger-heuristic to estimate a minor
value..

@Dennis: I am curious how complex the contact size calibration function in
chrome os is.

regards
Andreas
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v6 08/12] drm: Don't hang onto the backend config object post-backend_init

2016-04-18 Thread Pekka Paalanen
On Sun, 17 Apr 2016 17:18:17 +0300
Giulio Camuffo  wrote:

> Ah, so this fixes the problem with patch 5. I think it would make
> sense to squash them together.
> One comment below.
> 
> 2016-04-16 6:28 GMT+03:00 Bryce Harrington :
> > The drm backend was copying most everything out of the config object
> > already, but now also copy the use_current_mode parameter and the
> > config_output function pointer, so that there are no remaining
> > references to the config object passed into backend_init().
> >
> > By decoupling the config struct to the backend, if in the future if the
> > structure definition changes in non-backwards compatible ways, then any
> > version compatibility adaptation will be limited to just the
> > backend_init() routine.
> >
> > With the use_current_mode moved into the main config class, the
> > drm_config wrapper is redundant.  Dropping it helps make the drm backend
> > config initialization more consistent with the other backends.
> >
> > Signed-off-by: Bryce Harrington 
> > Reviewed-by: Pekka Paalanen 
> >
> > v6:
> >  - Drop use of drm_config config wrapper
> >
> > Signed-off-by: Bryce Harrington 
> > ---
> >  src/compositor-drm.c | 24 +++-
> >  src/compositor-drm.h |  3 ++-
> >  src/main.c   | 47 +++
> >  3 files changed, 36 insertions(+), 38 deletions(-)
> >
> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > index 2384fd2..6ef706a 100644
> > --- a/src/compositor-drm.c
> > +++ b/src/compositor-drm.c
> > @@ -121,7 +121,17 @@ struct drm_backend {
> > int32_t cursor_width;
> > int32_t cursor_height;
> >
> > -   struct weston_drm_backend_config *config;
> > +/** Callback used to configure the outputs.
> > +*
> > + * This function will be called by the backend when a new DRM
> > + * output needs to be configured.
> > + */
> > +enum weston_drm_backend_output_mode
> > +   (*configure_output)(struct weston_compositor *compositor,
> > +   bool use_current_mode,
> > +   const char *name,
> > +   struct weston_drm_backend_output_config 
> > *output_config);
> > +   bool use_current_mode;
> >  };
> >
> >  struct drm_mode {
> > @@ -2311,8 +2321,8 @@ create_output_for_connector(struct drm_backend *b,
> > output->base.serial_number = "unknown";
> > wl_list_init(&output->base.mode_list);
> >
> > -   mode = b->config->configure_output(b->compositor, b->config,
> > -  output->base.name, &config);
> > +   mode = b->configure_output(b->compositor, b->use_current_mode,
> > +  output->base.name, &config);
> > if (parse_gbm_format(config.gbm_format, b->gbm_format, 
> > &output->gbm_format) == -1)
> > output->gbm_format = b->gbm_format;
> >
> > @@ -2699,11 +2709,6 @@ drm_destroy(struct weston_compositor *ec)
> > weston_launcher_destroy(ec->launcher);
> >
> > close(b->drm.fd);
> > -
> > -   free(b->config->gbm_format);
> > -   free(b->config->seat_id);
> > -   free(b->config);
> > -
> > free(b);
> >  }
> >
> > @@ -3054,7 +3059,8 @@ drm_backend_create(struct weston_compositor 
> > *compositor,
> > b->sprites_are_broken = 1;
> > b->compositor = compositor;
> > b->use_pixman = config->use_pixman;
> > -   b->config = config;
> > +   b->configure_output = config->configure_output;
> > +   b->use_current_mode = config->use_current_mode;
> >
> > if (parse_gbm_format(config->gbm_format, GBM_FORMAT_XRGB, 
> > &b->gbm_format) < 0)
> > goto err_compositor;
> > diff --git a/src/compositor-drm.h b/src/compositor-drm.h
> > index fdf5154..3b2dc17 100644
> > --- a/src/compositor-drm.h
> > +++ b/src/compositor-drm.h
> > @@ -114,9 +114,10 @@ struct weston_drm_backend_config {
> >  */
> > enum weston_drm_backend_output_mode
> > (*configure_output)(struct weston_compositor *compositor,
> > -   struct weston_drm_backend_config 
> > *backend_config,
> > +   bool use_current_mode,
> > const char *name,
> > struct weston_drm_backend_output_config 
> > *output_config);
> > +   bool use_current_mode;
> >  };
> >
> >  #ifdef  __cplusplus
> > diff --git a/src/main.c b/src/main.c
> > index feb967d..21b7f5c 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -674,19 +674,12 @@ load_backend_new(struct weston_compositor 
> > *compositor, const char *backend,
> > return backend_init(compositor, NULL, NULL, NULL, config_base);
> >  }
> >
> > -
> > -struct drm_config {
> > -   struct weston_drm_backend_config base;
> > -   bool use_current_mode;
> > -};
> > -
> >  static enum weston_drm_backend_output_mo

Re: [PATCH wayland-protocols 1/2] stable: add viewporter draft

2016-04-18 Thread Daniel Stone
Hi,

On 15 April 2016 at 15:53, Pekka Paalanen  wrote:
> +  This interface allows to define the source rectangle (src_x,
> +  src_y, src_width, src_height) from where to take the wl_buffer
> +  contents, and scale that to destination size (dst_width,
> +  dst_height). This state is double-buffered, and is applied on the
> +  next wl_surface.commit.

I like Yong's rewording, except with an additional bikeshed of 'allows
the client to control the [...]' perhaps.

> +  If the source rectangle is set, it defines what area of the
> +  wl_buffer is taken as the source. If the source rectangle is set and
> +  the destination size is not set, the surface size becomes the source
> +  rectangle size rounded up to the nearest integer. If the source size
> +  is already exactly integers, this results in cropping without scaling.

Ugh, I really intensely dislike that middle section. The cropping and
scaling are completely independent ... except where they aren't,
because sometimes we'll magically do it for you. I'd very much prefer
to see fractional crop + no scale defined as a protocol error, i.e.
the user must always explicitly set a configuration that results in an
integral destination size.

> +  If the source rectangle is partially or completely outside of the
> +  wl_buffer, then the surface contents are undefined (not void), and
> +  the surface size is still dst_width, dst_height.

Again, I'd rather see this defined as a protocol error. Without
explicit border-colour/mode controls (e.g. 'sample this colour for
anything outside buffer bounds'), the result of sampling outside the
source image can never be useful (at least AFAICT). So, why would you
ever want to do so? To achieve more perfect scaling factors, at the
expense of having to display some fractional garbage?

> +  If the wl_surface associated with the wl_viewport is destroyed,
> +  the wl_viewport object becomes inert.

Should attempting to call wl_viewport::set_* then result in an error?

> +   Arguments dst_x and dst_y do not exist here, use the x and y
> +   arguments to wl_surface.attach.

The x and y arguments perform relative translation of the surface, and
as such I don't think are really analagous. I'd suggest just deleting
this to avoid confusion.

> +
> +  
> +   Set the source rectangle of the associated wl_surface. See
> +   wl_viewport for the description, and relation to the wl_buffer
> +   size.
> +
> +   If width is -1.0 and height is -1.0, the source rectangle is unset
> +   instead. Any other pair of values for width and height that
> +   contains zero or negative values raises the bad_value protocol
> +   error.

This is different to the bare 'set' request, which doesn't (at least
in its documentation) allow for this. But that's being removed anyway,
so no problem. Should we require a pair of fixed values for x and y
here, either (0,0) as the state they'll be fixed to, or (-1,-1) for
symmetry?

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v6 11/12] Enforce destruction of all backend config objects after initialization

2016-04-18 Thread Pekka Paalanen
On Sun, 17 Apr 2016 17:22:50 +0300
Giulio Camuffo  wrote:

> Maybe it would make sense to squash this too, but either way:
> Reviewed-by: Giulio Camuffo 
> 
> 2016-04-16 6:28 GMT+03:00 Bryce Harrington :
> > Since the backend config struct versioning implies that there we expect
> > potential future descrepancy between main's definition of the config
> > object and the backend's, don't allow the backend to hang onto the
> > config object outside the initialization scope.
> >
> > Signed-off-by: Bryce Harrington 
> > Reviewed-by: Pekka Paalanen 
> >
> > v6:
> >  - Put backend configs on stack instead of zalloc()
> >
> > Signed-off-by: Bryce Harrington 
> > ---
> >  src/main.c | 113 
> > -
> >  1 file changed, 51 insertions(+), 62 deletions(-)
> >
> > diff --git a/src/main.c b/src/main.c
> > index dcd5ee6..9a8094f 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -657,7 +657,20 @@ load_backend_old(struct weston_compositor *compositor, 
> > const char *backend,
> > return backend_init(compositor, argc, argv, wc, NULL);
> >  }
> >
> > -/* Temporary function to be replaced by weston_compositor_load_backend(). 
> > */
> > +/** Main module call-point for backends.
> > + *
> > + * All backends should use this routine to access their init routine.
> > + * Backends may subclass weston_backend_config to add their own
> > + * configuration data, setting the major/minor version in config_base
> > + * accordingly.
> > + *
> > + * The config_base object should be treated as temporary, and any data
> > + * copied out of it by backend_init before returning.  The load_backend_new
> > + * callers may then free the config_base object.
> > + *
> > + * NOTE: This is a temporary function intended to eventually be replaced
> > + * by weston_compositor_load_backend().
> > + */
> >  static int
> >  load_backend_new(struct weston_compositor *compositor, const char *backend,
> >  struct weston_backend_config *config_base)

Hi,

I took the liberty to squash the headless part of this patch into the
headless conversion patch.

Furthermore I took the above documentation hunk for load_backend_new()
and pushed it upstream as a patch of its own:
   20b66c3..3c53094  master -> master

With all the splitting and squashing I understand it creates some
rebasing conflicts, so here is a rebased branch of the remaining patches
in this series:
https://git.collabora.com/cgit/user/pq/weston.git/log/?h=libweston-config-v6-rebased

I hope it saves you the extra effort my fiddling created when revising
the rest of the patches.


Thanks,
pq


pgp3EVNwGvgLR.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 4/7] compositor: rename scaler to viewport(er)

2016-04-18 Thread Yong Bakos
On Apr 15, 2016, at 9:53 AM, Pekka Paalanen  wrote:
> 
> From: Pekka Paalanen 
> 
> Since the interface is now called wp_viewport, rename functions from
> "scaler" to "viewporter" as well.
> 
> scaler_surface_to_buffer() is renamed to viewport_surface_to_buffer()
> because it is more about viewport than viewporter.
> 
> Signed-off-by: Pekka Paalanen 

This renaming looks fine to me.
Reviewed-by: Yong Bakos 

yong


> ---
> src/compositor.c | 44 ++--
> 1 file changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 551d4ed..b6a9aeb 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -877,8 +877,8 @@ weston_transformed_region(int width, int height,
> }
> 
> static void
> -scaler_surface_to_buffer(struct weston_surface *surface,
> -  float sx, float sy, float *bx, float *by)
> +viewport_surface_to_buffer(struct weston_surface *surface,
> +float sx, float sy, float *bx, float *by)
> {
>   struct weston_buffer_viewport *vp = &surface->buffer_viewport;
>   double src_width, src_height;
> @@ -912,8 +912,8 @@ weston_surface_to_buffer_float(struct weston_surface 
> *surface,
> {
>   struct weston_buffer_viewport *vp = &surface->buffer_viewport;
> 
> - /* first transform coordinates if the scaler is set */
> - scaler_surface_to_buffer(surface, sx, sy, bx, by);
> + /* first transform coordinates if the viewport is set */
> + viewport_surface_to_buffer(surface, sx, sy, bx, by);
> 
>   weston_transformed_coord(surface->width_from_buffer,
>surface->height_from_buffer,
> @@ -946,12 +946,12 @@ weston_surface_to_buffer_rect(struct weston_surface 
> *surface,
>   struct weston_buffer_viewport *vp = &surface->buffer_viewport;
>   float xf, yf;
> 
> - /* first transform box coordinates if the scaler is set */
> - scaler_surface_to_buffer(surface, rect.x1, rect.y1, &xf, &yf);
> + /* first transform box coordinates if the viewport is set */
> + viewport_surface_to_buffer(surface, rect.x1, rect.y1, &xf, &yf);
>   rect.x1 = floorf(xf);
>   rect.y1 = floorf(yf);
> 
> - scaler_surface_to_buffer(surface, rect.x2, rect.y2, &xf, &yf);
> + viewport_surface_to_buffer(surface, rect.x2, rect.y2, &xf, &yf);
>   rect.x2 = ceilf(xf);
>   rect.y2 = ceilf(yf);
> 
> @@ -4441,25 +4441,25 @@ static const struct wp_viewport_interface 
> viewport_interface = {
> };
> 
> static void
> -scaler_destroy(struct wl_client *client,
> -struct wl_resource *resource)
> +viewporter_destroy(struct wl_client *client,
> +struct wl_resource *resource)
> {
>   wl_resource_destroy(resource);
> }
> 
> static void
> -scaler_get_viewport(struct wl_client *client,
> - struct wl_resource *scaler,
> - uint32_t id,
> - struct wl_resource *surface_resource)
> +viewporter_get_viewport(struct wl_client *client,
> + struct wl_resource *viewporter,
> + uint32_t id,
> + struct wl_resource *surface_resource)
> {
> - int version = wl_resource_get_version(scaler);
> + int version = wl_resource_get_version(viewporter);
>   struct weston_surface *surface =
>   wl_resource_get_user_data(surface_resource);
>   struct wl_resource *resource;
> 
>   if (surface->viewport_resource) {
> - wl_resource_post_error(scaler,
> + wl_resource_post_error(viewporter,
>   WP_VIEWPORTER_ERROR_VIEWPORT_EXISTS,
>   "a viewport for that surface already exists");
>   return;
> @@ -4478,14 +4478,14 @@ scaler_get_viewport(struct wl_client *client,
>   surface->viewport_resource = resource;
> }
> 
> -static const struct wp_viewporter_interface scaler_interface = {
> - scaler_destroy,
> - scaler_get_viewport
> +static const struct wp_viewporter_interface viewporter_interface = {
> + viewporter_destroy,
> + viewporter_get_viewport
> };
> 
> static void
> -bind_scaler(struct wl_client *client,
> - void *data, uint32_t version, uint32_t id)
> +bind_viewporter(struct wl_client *client,
> + void *data, uint32_t version, uint32_t id)
> {
>   struct wl_resource *resource;
> 
> @@ -4496,7 +4496,7 @@ bind_scaler(struct wl_client *client,
>   return;
>   }
> 
> - wl_resource_set_implementation(resource, &scaler_interface,
> + wl_resource_set_implementation(resource, &viewporter_interface,
>  NULL, NULL);
> }
> 
> @@ -4679,7 +4679,7 @@ weston_compositor_create(struct wl_display *display, 
> void *user_data)
>   goto fail;
> 
>   if (!wl_global_create(ec->wl_display, &wp_viewporter_interface, 1,
> -   ec, bind_scaler))
> +   ec, bind_viewporter))
>   g

Re: [PATCH wayland-protocols 2/2] stable/viewporter: finish stabilization

2016-04-18 Thread Yong Bakos
On Apr 15, 2016, at 9:53 AM, Pekka Paalanen  wrote:
> 
> From: Pekka Paalanen 
> 
> Rename interfaces and the protocol to follow the policy.
> 
> Reset interface versions.
> 
> Remove the redundant wp_viewport.set request.
> 
> Replace "surface coordinates" with "surface local coordinates".
> 
> Hook up to build and install.
> 
> Signed-off-by: Pekka Paalanen 

All name replacements look good to me. I would add one line break
before the closing protocol tag, to match other xml docs. Not sure if
that should happen here, in 1/2, or in a minor patch after all of this
makes it into master.

Otherwise Reviewed-by: Yong Bakos 

(always, my r'bs are just fwiw)

yong


> ---
> Makefile.am  |  1 +
> stable/viewporter/viewporter.xml | 57 +++-
> 2 files changed, 16 insertions(+), 42 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 033789f..71d2632 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -12,6 +12,7 @@ unstable_protocols =
> \
> 
> stable_protocols =
> \
>   stable/presentation-time/presentation-time.xml  
> \
> + stable/viewporter/viewporter.xml
> \
>   $(NULL)
> 
> nobase_dist_pkgdata_DATA =
> \
> diff --git a/stable/viewporter/viewporter.xml 
> b/stable/viewporter/viewporter.xml
> index 0e482a6..1b47997 100644
> --- a/stable/viewporter/viewporter.xml
> +++ b/stable/viewporter/viewporter.xml
> @@ -1,5 +1,5 @@
> 
> -
> +
> 
>   
> Copyright © 2013-2014 Collabora, Ltd.
> @@ -24,7 +24,7 @@
> DEALINGS IN THE SOFTWARE.
>   
> 
> -  
> +  
> 
>   The global interface exposing surface cropping and scaling
>   capabilities is used to instantiate an interface extension for a
> @@ -38,7 +38,7 @@
>   
>   Informs the server that the client will not be using this
>   protocol object anymore. This does not affect any other objects,
> - wl_viewport objects included.
> + wp_viewport objects included.
>   
> 
> 
> @@ -51,18 +51,18 @@
>   
>   Instantiate an interface extension for the given wl_surface to
>   crop and scale its content. If the given wl_surface already has
> - a wl_viewport object associated, the viewport_exists
> + a wp_viewport object associated, the viewport_exists
>   protocol error is raised.
>   
> 
> -   +  summary="the new viewport interface id"/>
>   summary="the surface"/>
> 
>   
> 
> -  
> +  
> 
>   An additional interface to a wl_surface object, which allows the
>   client to specify the cropping and scaling of the surface
> @@ -85,7 +85,7 @@
>   this size. This overrides whatever the attached wl_buffer size is,
>   unless the wl_buffer is NULL. If the wl_buffer is NULL, the surface
>   has no content and therefore no size. Otherwise, the size is always
> -  at least 1x1 in surface coordinates.
> +  at least 1x1 in surface local coordinates.
> 
>   If the source rectangle is set, it defines what area of the
>   wl_buffer is taken as the source. If the source rectangle is set and
> @@ -97,7 +97,7 @@
>   the surface-local coordinates happen in the following order:
> 1. buffer_transform (wl_surface.set_buffer_transform)
> 2. buffer_scale (wl_surface.set_buffer_scale)
> -3. crop and scale (wl_viewport.set*)
> +3. crop and scale (wp_viewport.set*)
>   This means, that the source rectangle coordinates of crop and scale
>   are given in the coordinates after the buffer transform and scale,
>   i.e. in the coordinates that would be the surface-local coordinates
> @@ -113,10 +113,10 @@
>   still in the surface-local coordinate system, just like dst_width
>   and dst_height are.
> 
> -  If the wl_surface associated with the wl_viewport is destroyed,
> -  the wl_viewport object becomes inert.
> +  If the wl_surface associated with the wp_viewport is destroyed,
> +  the wp_viewport object becomes inert.
> 
> -  If the wl_viewport object is destroyed, the crop and scale
> +  If the wp_viewport object is destroyed, the crop and scale
>   state is removed from the wl_surface. The change will be applied
>   on the next wl_surface.commit.
> 
> @@ -133,37 +133,10 @@
>  summary="negative or zero values in width or height"/>
> 
> 
> -
> -  
> - Set both source rectangle and destination size of the associated
> - wl_surface. See wl_viewport for the description, and relation to
> - the wl_buffer size.
> -
> - The bad_value protocol error is raised if src_width or
> - src_height is negative, or if dst_width or dst_height is not
> - positive.
> -
> - The crop and scale state is double-buffered state, an

Re: [PATCH wayland-protocols 1/2] stable: add viewporter draft

2016-04-18 Thread Yong Bakos
On Apr 15, 2016, at 9:53 AM, Pekka Paalanen  wrote:
> 
> From: Pekka Paalanen 
> 
> This XML file has been copied verbatim from Weston 1.10.0 release,
> protocol/scaler.xml.
> 
> The interfaces still need renaming according to wayland-protocols
> policy. Also a redundant request needs to be removed. These will be done
> in a follow-up patch to clearly show the changes.
> 
> Signed-off-by: Pekka Paalanen 

There's one paragraph I think could use a rewriting, noted inline below,
but since this is just a move, the writing and such is
Reviewed-by: Yong Bakos 

yong


> ---
> stable/viewporter/README |   7 ++
> stable/viewporter/viewporter.xml | 208 +++
> 2 files changed, 215 insertions(+)
> create mode 100644 stable/viewporter/README
> create mode 100644 stable/viewporter/viewporter.xml
> 
> diff --git a/stable/viewporter/README b/stable/viewporter/README
> new file mode 100644
> index 000..e09057b
> --- /dev/null
> +++ b/stable/viewporter/README
> @@ -0,0 +1,7 @@
> +Viewporter: cropping and scaling extension for surface contents
> +
> +Previously known as wl_scaler.
> +
> +Maintainers:
> +Pekka Paalanen 
> +
> diff --git a/stable/viewporter/viewporter.xml 
> b/stable/viewporter/viewporter.xml
> new file mode 100644
> index 000..0e482a6
> --- /dev/null
> +++ b/stable/viewporter/viewporter.xml
> @@ -0,0 +1,208 @@
> +
> +
> +
> +  
> +Copyright © 2013-2014 Collabora, Ltd.
> +
> +Permission is hereby granted, free of charge, to any person obtaining a
> +copy of this software and associated documentation files (the 
> "Software"),
> +to deal in the Software without restriction, including without limitation
> +the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +and/or sell copies of the Software, and to permit persons to whom the
> +Software is furnished to do so, subject to the following conditions:
> +
> +The above copyright notice and this permission notice (including the next
> +paragraph) shall be included in all copies or substantial portions of the
> +Software.
> +
> +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> OR
> +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
> OTHER
> +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +DEALINGS IN THE SOFTWARE.
> +  
> +
> +  
> +
> +  The global interface exposing surface cropping and scaling
> +  capabilities is used to instantiate an interface extension for a
> +  wl_surface object. This extended interface will then allow
> +  cropping and scaling the surface contents, effectively
> +  disconnecting the direct relationship between the buffer and the
> +  surface size.
> +
> +
> +
> +  
> + Informs the server that the client will not be using this
> + protocol object anymore. This does not affect any other objects,
> + wl_viewport objects included.
> +  
> +
> +
> +
> +   + summary="the surface already has a viewport object associated"/>
> +
> +
> +
> +  
> + Instantiate an interface extension for the given wl_surface to
> + crop and scale its content. If the given wl_surface already has
> + a wl_viewport object associated, the viewport_exists
> + protocol error is raised.
> +  
> +
> +   +   summary="the new viewport interface id"/>
> +   +   summary="the surface"/>
> +
> +  
> +
> +  
> +
> +  An additional interface to a wl_surface object, which allows the
> +  client to specify the cropping and scaling of the surface
> +  contents.
> +
> +  This interface allows to define the source rectangle (src_x,
> +  src_y, src_width, src_height) from where to take the wl_buffer
> +  contents, and scale that to destination size (dst_width,
> +  dst_height). This state is double-buffered, and is applied on the
> +  next wl_surface.commit.

The first sentence of this paragraph is a bit cumbersome. Maybe:

This interface defines a source rectangle of the wl_buffer
contents (src_x, src_y, src_width, src_height), and a means of
scaling the source rectangle to a destination size (dst_width,
dst_height).


> +
> +  The two parts of crop and scale state are independent: the source
> +  rectangle, and the destination size. Initially both are unset, that
> +  is, no scaling is applied. The whole of the current wl_buffer is
> +  used as the source, and the surface size is as defined in
> +  wl_surface.attach.
> +
> +  If the destination size is set, it causes the surface size to become
> +  dst_width, dst_height. The source (rectangle) is scaled to e

Re: [PATCH weston v6 07/12] headless: port the headless backend to the new init api

2016-04-18 Thread Pekka Paalanen
On Sun, 17 Apr 2016 17:14:17 +0300
Giulio Camuffo  wrote:

> Reviewed-by: Giulio Camuffo 
> 
> 2016-04-16 6:28 GMT+03:00 Bryce Harrington :
> > From: Benoit Gschwind 
> >
> > refactor configuration API of headless-backend
> >
> > Signed-off-by: Bryce Harrington 
> > Reviewed-by: Pekka Paalanen 
> >
> > v6:
> >   - Define version number in the header
> >   - Don't use leading underscores in header guards
> >   - Add stub config_init_to_defaults()
> >   - Allocate config on stack
> >   - Drop unused display_name parameter
> >   - Add error message when config is invalid
> >   - Install compositor-headless.h and list it in headless-backend sources
> > v5:
> >   - Update to current trunk
> >   - Fixed typo 'struct weston_wayland_backend_config'
> >   - Dropped unused variables
> >   - Dropped weston_headless_backend_config_create() in favor of
> > directly zalloc'ing the object
> >   - Dropped weston_headless_backend_load() in favor of the more
> > generalized load_backend_new().
> >   - Dropped typedef from header
> >   - Restored use of 'backend_init' entry point
> >   - Backend_init() takes a base weston_backend_config object
> >   - Renamed 'param' to 'config' in a few places for consistency
> >   - Renamed 'headless_options' variable to 'options for consistency
> >   - Version the base struct
> >   - Free config on error
> >   - Don't free config during backend_init normal operations
> >   - Adjust header ordering
> >   - Make header guard naming consistent with other headers
> >   - Light reformatting for code style and consistency with other
> > backend config patches
> >
> > Signed-off-by: Bryce Harrington 
> > ---
> >  Makefile.am   |  3 ++
> >  src/compositor-headless.c | 74 
> > +--
> >  src/compositor-headless.h | 53 +
> >  src/main.c| 45 ++--
> >  4 files changed, 132 insertions(+), 43 deletions(-)
> >  create mode 100644 src/compositor-headless.h
> >

Hi,

since this patch was ready by both Giulio's and my opinion, I decided
to rebase it to apply upstream before the drm or x11 backend
conversions.

I also took the headless part of the patch "[PATCH weston v6 11/12]
Enforce destruction of all backend config objects after initialization"
and squashed it into this patch.

Pushed:
   20b66c3..3c53094  master -> master


Thanks,
pq


pgparL4isIJB6.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 v6 04/12] compositor: Version the backend configuration structures

2016-04-18 Thread Pekka Paalanen
On Fri, 15 Apr 2016 20:28:29 -0700
Bryce Harrington  wrote:

> With this struct versioning, it is possible to add new options without
> breaking the ABI, as long as all additions are made to the end of a
> struct and nothing existing is modified or removed.  When things are
> added, the structure's size will increase, and we'll use this size as
> our minor version number.  If existing things need to be changed, then
> the major version, struct_version, is incremented to indicate the ABI
> break.
> 
> From our call sites in main these major and minor version will be
> recorded as struct_version and struct_size.  Each backend will then
> verify these against its own assumptions.  So long as the backend's
> struct is equal or larger than what was passed in and the major versions
> are equal, we're good; but if it is larger, then this is a fatal error.
> 
> Signed-off-by: Bryce Harrington 
> Reviewed-by: Pekka Paalanen 
> 
> v6:
>  - Document refs for alternatives/assumptions for backend configs
> v5:
>  - Move the header changes to a pre-requisite patch from the drm backend
> 
> Signed-off-by: Bryce Harrington 
> ---
>  src/compositor.h | 22 ++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/src/compositor.h b/src/compositor.h
> index 5ca497c..a329dbe 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -684,8 +684,30 @@ struct weston_backend_output_config {
>   * passed to the backend's init entry point. The backend will
>   * likely want to subclass this in order to handle backend specific
>   * data.
> + *
> + * NOTE: Alternate designs were proposed (Feb 2016) for using opaque
> + * structures and for section+key/value getter/setters.  The rationale
> + * for selecting the transparent structure design is based on several
> + * assumptions which may require re-evaluating the design choice if they
> + * fail to hold.
>   */
>  struct weston_backend_config {
> +   /** Major version for the backend-specific config struct
> +*
> +* This version must match exactly what the backend expects, otherwise
> +* the struct is incompatible.
> +*/
> +   uint32_t struct_version;
> +
> +   /** Minor version of the backend-specific config struct
> +*
> +* This must be set to sizeof(struct backend-specific config).
> +* If the value here is smaller than what the backend expects, the
> +* extra config members will assume their default values.
> +*
> +* A value greater than what the backend expects is incompatible.
> +*/
> +   size_t struct_size;
>  };
>  
>  struct weston_backend {

Hi Bryce,

I brought back the links to the discussions you dug up earlier, and
pushed with that change:
   e9b8a2b..20b66c3  master -> master


Thanks,
pq


pgpoUO9374HkH.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 v6 09/12] drm: Move drm's output configuration into the drm backend

2016-04-18 Thread Pekka Paalanen
On Sun, 17 Apr 2016 13:26:58 +0300
Giulio Camuffo  wrote:

> Hi,
> 
> I don't understand this one at all, you're moving configuration back
> into the backend, partially reverting patch 5.
> 

Hi,

indeed, the very reason we are working on this patch series is to take
all .ini file parsing, struct weston_config, and command line parsing
out from the backends and libweston.

That is also why we need the configure_output callback for now, to let
main.c handle configuring outputs that were not explicitly mentioned in
configuration.

I'm with Giulio here.


Thanks,
pq

> 2016-04-16 6:28 GMT+03:00 Bryce Harrington :
> > Signed-off-by: Bryce Harrington 
> > ---
> >  src/compositor-drm.c | 93 
> > 
> >  src/compositor-drm.h | 41 ---
> >  src/main.c   | 43 
> >  3 files changed, 80 insertions(+), 97 deletions(-)
> >
> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > index 6ef706a..d129adc 100644
> > --- a/src/compositor-drm.c
> > +++ b/src/compositor-drm.c
> > @@ -75,6 +75,41 @@
> >  #define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
> >  #endif
> >
> > +enum weston_drm_backend_output_mode {
> > +   /** The output is disabled */
> > +   WESTON_DRM_BACKEND_OUTPUT_OFF,
> > +
> > +   /** The output will use the current active mode */
> > +   WESTON_DRM_BACKEND_OUTPUT_CURRENT,
> > +
> > +   /** The output will use the preferred mode. A modeline can be 
> > provided
> > +* by setting weston_backend_output_config::modeline in the form of
> > +* "WIDTHxHEIGHT" or in the form of an explicit modeline calculated
> > +* using e.g. the cvt tool. If a valid modeline is supplied it will 
> > be
> > +* used, if invalid or NULL the preferred available mode will be 
> > used. */
> > +   WESTON_DRM_BACKEND_OUTPUT_PREFERRED,
> > +};
> > +
> > +struct weston_drm_backend_output_config {
> > +   struct weston_backend_output_config base;
> > +
> > +   /** The pixel format to be used by the output. Valid values are:
> > +* - NULL - The format set at backend creation time will be used
> > +* - "xrgb"
> > +* - "rgb565"
> > +* - "xrgb2101010"
> > +*/
> > +   char *gbm_format;
> > +
> > +   /** The seat to be used by the output. Set to NULL to use the
> > +* default seat. */
> > +   char *seat;
> > +
> > +   /** The modeline to be used by the output. Refer to the 
> > documentation
> > +* of WESTON_DRM_BACKEND_OUTPUT_PREFERRED for details. */
> > +   char *modeline;
> > +};
> > +
> >  struct drm_backend {
> > struct weston_backend base;
> > struct weston_compositor *compositor;
> > @@ -121,16 +156,6 @@ struct drm_backend {
> > int32_t cursor_width;
> > int32_t cursor_height;
> >
> > -/** Callback used to configure the outputs.
> > -*
> > - * This function will be called by the backend when a new DRM
> > - * output needs to be configured.
> > - */
> > -enum weston_drm_backend_output_mode
> > -   (*configure_output)(struct weston_compositor *compositor,
> > -   bool use_current_mode,
> > -   const char *name,
> > -   struct weston_drm_backend_output_config 
> > *output_config);
> > bool use_current_mode;
> >  };
> >
> > @@ -2275,6 +2300,49 @@ connector_get_current_mode(drmModeConnector 
> > *connector, int drm_fd,
> > return 0;
> >  }
> >
> > +static enum weston_drm_backend_output_mode
> > +drm_configure_output(struct weston_compositor *c,
> > + bool use_current_mode,
> > + const char *name,
> > + struct weston_drm_backend_output_config *config)
> > +{
> > +   struct weston_config *wc = weston_compositor_get_user_data(c);  
> 
> This is wrong, you're assuming the user data is a weston_config, which
> won't be true for non-weston compositors.
> 
> Sorry, but NAK from me.
> 
> 
> Giulio



pgpiCVkb2Kyix.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v5 xdg-shell-unstable-v6] xdg-shell: Add min/max size requests

2016-04-18 Thread Bill Spitzak

On 04/12/2016 06:10 AM, Jonas Ådahl wrote:


Good point. Setting an invalid state should probably result in a
protocol error.


No this cannot be a protocol error because that makes it difficult to 
change the size range to a new one that does not intersect the old one.


Perhaps having the commit of an invalid state be a protocol error is ok.

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v6 03/12] compositor: Drop unneeded create_output callback

2016-04-18 Thread Pekka Paalanen
On Sun, 17 Apr 2016 16:35:03 +0300
Giulio Camuffo  wrote:

> I must say the approach you've taken in the x11 backend, passing all
> the configs at
> init time, is neat, but this leaves me wondering if we'll ever want to
> create an output after
> initialization... however i think until such need arises this is ok.
> 
> Reviewed-by: Giulio Camuffo 

Yup, like I said in another email, I expect big changes to the way we
manage outputs in the future.

Since this is a simple independent change, pushed:
   b993998..e9b8a2b  master -> master


Thanks,
pq

> 2016-04-16 6:28 GMT+03:00 Bryce Harrington :
> > Signed-off-by: Bryce Harrington 
> > ---
> >  src/compositor.h | 8 
> >  1 file changed, 8 deletions(-)
> >
> > diff --git a/src/compositor.h b/src/compositor.h
> > index 0ba00be..5ca497c 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -691,14 +691,6 @@ struct weston_backend_config {
> >  struct weston_backend {
> > void (*destroy)(struct weston_compositor *compositor);
> > void (*restore)(struct weston_compositor *compositor);
> > -   /* vfunc to create a new output with a given name and config.
> > -* backends not supporting the functionality will set this
> > -* to NULL.
> > -*/
> > -   struct weston_output *
> > -   (*create_output)(struct weston_compositor *compositor,
> > -const char *name,
> > -struct weston_backend_output_config 
> > *config);
> >  };
> >
> >  struct weston_compositor {
> > --
> > 1.9.1
> >
> > ___
> > 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



pgpoRWD3rDyY8.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 v6 01/12] drm: Spelling fix in comment

2016-04-18 Thread Pekka Paalanen
On Sun, 17 Apr 2016 16:25:54 +0300
Giulio Camuffo  wrote:

> Hi,
> 
> Trivial enough, Reviewed-by: Giulio Camuffo 

Yeah, pushed:
   aff703e..b993998  master -> master


Thanks,
pq

> 
> 2016-04-16 6:28 GMT+03:00 Bryce Harrington :
> > Signed-off-by: Bryce Harrington 
> > ---
> >  src/compositor-drm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > index 119b6c5..a47b453 100644
> > --- a/src/compositor-drm.c
> > +++ b/src/compositor-drm.c
> > @@ -1551,7 +1551,7 @@ create_gbm_device(int fd)
> >  }
> >
> >  /* When initializing EGL, if the preferred buffer format isn't available
> > - * we may be able to susbstitute an ARGB format for an XRGB one.
> > + * we may be able to substitute an ARGB format for an XRGB one.
> >   *
> >   * This returns 0 if substitution isn't possible, but 0 might be a
> >   * legitimate format for other EGL platforms, so the caller is
> > --
> > 1.9.1
> >
> > ___
> > 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



pgpPNe4_VlDG8.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 v5 03/11] drm: port the drm backend to the new init api

2016-04-18 Thread Pekka Paalanen
On Sat, 16 Apr 2016 08:49:22 +0300
Giulio Camuffo  wrote:

> 2016-04-16 1:02 GMT+03:00 Bryce Harrington :
> > On Fri, Apr 15, 2016 at 02:32:04PM +0200, Benoit Gschwind wrote:  
> >> > +   /** The seat to be used by the output. Set to NULL to use the
> >> > +* default seat. */
> >> > +   char *seat;
> >> > +   /** The modeline to be used by the output. Refer to the documentation
> >> > +* of WESTON_DRM_BACKEND_OUTPUT_PREFERRED for details. */
> >> > +   char *modeline;  
> >>
> >> Maybe enum with drmModeModeInfo* is better API than free string API?  
> >
> > I think you're probably right that weston_drm_backend_output_config
> > could hold a pointer to the enum rather than the string.  I've moved
> > weston_drm_backend_output_config to be a private struct in
> > compositor-drm.c so this should be doable.  
> 
> One of the earlier revisions had that but after a discussion with
> Quentin we decided to put the parsing in the backend rather than every
> compositor.

Also using a libdrm struct in the config API is a bit awkward. It would
be the only reason why compositor-drm.h needed libdrm headers. A string
can also easily use a shortcut notation like "800x600" instead of a
full modeline.

But, I expect the output configuration API to have a complete rewrite
and redesign in the future, to move into a pattern where libweston
notifies about new outputs and then the compositor decides to take them
into use and set them up, rather than the backend in libweston like it
does now.


Thanks,
pq


pgpvQgB3wyxwI.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] option-parser: Handle short double-arg options

2016-04-18 Thread Pekka Paalanen
On Fri, 15 Apr 2016 10:26:26 -0700
Bryce Harrington  wrote:

> On Fri, Apr 15, 2016 at 11:55:57AM +0300, Pekka Paalanen wrote:
> > On Thu, 14 Apr 2016 11:10:57 -0700
> > Bryce Harrington  wrote:
> >   
> > > On Thu, Apr 14, 2016 at 03:16:07PM +0300, Pekka Paalanen wrote:  
> > > > On Sat, 13 Feb 2016 23:56:38 +0100
> > > > Benoit Gschwind  wrote:
> > > > 
> > > > > Hello Bryce,
> > > > > 
> > > > > It seems the corner case '-f42xxx 2938475' doesn't work as expected 
> > > > > with 
> > > > > 'f' short option as integer:
> > > > > 
> > > > > 1. one dash then call short_option
> > > > > 2. in short_option will check arg[2] and call handle_option
> > > > > 3. in handle_option will call strtol, where *value is '4' and *p is 
> > > > > 'x' 
> > > > > thus *value && !*p is 0
> > > > > 4. return from handle_option with 0
> > > > > 5. return from short_option with 0
> > > > > 6. call short_option_with_arg with arg = "-f42xxx" and param = 
> > > > > "2938475"
> > > > > 7. pass through all test and call handle_option
> > > > > 8. give 2938475 to the option
> > > > > 
> > > > > The expected behavior is an error.
> > > > > 
> > > > > Should I use Reviewed-by ?
> > > > 
> > > > No, you only give Reviewed-by when you think everything is good. You
> > > > are pointing out a problem instead.
> > > > 
> > > > Looks like this patch landed. Isn't there something to fix?
> > > 
> > > The option handling code is relatively concise, which is good but the
> > > consequence is that there's a heap of corner cases like the above.
> > > 
> > > Personally I don't think it's worth going overboard in trying to solve
> > > every potential case, just ones that are likely to come up in use,
> > > because if we do want a really robust option parsing system we probably
> > > should be looking at one of the several widely used option handling
> > > libraries like popt or whatnot.
> > >  
> > 
> > Hi Bryce,
> > 
> > in that case, shouldn't we just revert this patch?  
> 
> This solved a problem that came up in actual use, while I was testing
> the idle inhibition functionality.  I did try to keep the patch as
> minimal as possible though.
>  
> > > > I think we might have tests for the option parser. Would be good to add
> > > > this particular corner-case as a test there. If we don't have tests,
> > > > would be nice to have some.
> > > 
> > > Of course I'm a strong +1 for more tests, but again I think if we care
> > > that much about making the option handling solid, we really ought first
> > > do some due diligence looking at existing solutions.  This isn't an area
> > > where we're value-adding much...
> > > 
> > > Either way we go, I'd be happy to integrate a replacement lib or improve
> > > our existing solution, although I have some other matters that would be
> > > higher priority in the near term.  
> > 
> > Personally I've been ok with what it was. I have forgotten why it
> > didn't originally just wrap getopt_long (a GNUism? too much effort?).  
> 
> Or portability maybe?
> 
> getopt_long has a few quirks/limitations itself (for Inkscape we
> switched to popt).
> 
> > But you're right, it's a really unimportant thing, so I won't push it
> > in any direction myself. It just looked like you ignored a review
> > comment pointing out a new problem.  
> 
> Sorry.  We did actually exchange emails about it off-list.
> 
> > Changing the command line argument syntax might even be too disruptive
> > nowadays.  
> 
> Possibly, yeah, although some packages are pretty flexible and I would
> expect could replicate our syntax style adequately enough.  I agree that
> avoiding disruption would have to be a requirement in making any choice.
> 
> I suppose one thing to consider is that with the libweston backend
> configuration support going in, there might be more pressure for more
> featureful command line option functionality.  But that pressure will be
> pretty obvious if it comes.  

Except that all command line parsing and config file parsing will be
outside the scope of libweston. None of it will be in libweston, so it
will all stay specific to weston itself.


Thanks,
pq


pgpKFHftYNtmZ.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v8 xdg-shell-unstable-v6] xdg-shell: Add min/max size requests

2016-04-18 Thread Olivier Fourdan
Some application may wish to restrict their window in size, but
xdg-shell has no mechanism for the client to specify a maximum or
minimum size.

As a result, the compositor may try to maximize or fullscreen a window
while the client would not allow for the requested size.

Add new requests "set_max_size" and "set_min_size" to xdg-shell so that
the client can tell the compositor what would be its smallest/largest
acceptable size, and that the compositor can decide if maximize or
fullscreen is achievable, draw an accurate animation, etc.

Signed-off-by: Olivier Fourdan 
Bugzilla: https://bugzilla.gnome.org/show_bug.cgi?id=764413
---
 v2: Rename the request to "set_preferred_max_size",
 add "set_preferred_min_size" as well
 v3: Rebase above patch 72427 in branch xdg-shell-unstable-v6
 Rephrase description to clarify the unscaled size and using 0 to
 reset back the preferred size to an unspecified state
 v4: Patch the correct xml file (v6, not v5 )
 Fix multiple mismatch of min/max in the description
 Remove mention of "unscaled", specify window geometry coordinates
 and refer to set_window_geometry.
 v5: Fix typos and remove "preferred" from the name and description as
 requested by several people on the ML and irc.
 v6: Specify the requests are double-buffered and require a commit,
 rephrase the values never set as suggested by Jasper, state
 that min > max is an invalid state and result in a protocol error
 as suggested by Bill, Yong and Jonas.
 v7: Specify that width/height values must be greater than or equal to
 zero as discussed with Mike.
 v8: Explicitely state that the compositor may ignore the mix/max size.

 unstable/xdg-shell/xdg-shell-unstable-v6.xml | 88 
 1 file changed, 88 insertions(+)

diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
index 3fc7d42..1448c2c 100644
--- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
+++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
@@ -489,6 +489,94 @@
   
 
 
+
+  
+   Set a maximum size for the window.
+
+   The client can specify a maximum size so that the compositor does
+   not try to configure the window beyond this size.
+
+   The width and height arguments are in window geometry coordinates.
+   See set_window_geometry.
+
+   Values set in this way are double-buffered. They will get applied
+   on the next commit.
+
+   The compositor can use this information to allow or disallow
+   different states like maximize or fullscreen and draw accurate
+   animations.
+
+   Similarly, a tiling window manager may use this information to
+   place and resize client windows in a more effective way.
+
+   The client should not rely on the compositor to obey the maximum
+   size. The compositor may decide to ignore the values set by the
+   client and request a larger size. In such a case, the client can
+   either adapt to the requested size or refuse to acknowledge the
+   configure event sent by the compositor. See xdg_surface.configure
+   and xdg_surface.ack_configure for details.
+
+   If never set, or a value of zero in the request, means that the
+   client has no expected maximum size in the given dimension.
+   As a result, a client wishing to reset the maximum size
+   to an unspecified state can use zero for width and height in the
+   request.
+
+   Requesting a maximum size to be smaller than the minimum size of
+   a surface is illegal and will result in a protocol error.
+
+   The width and height must be greater than or equal to zero. Using
+   strictly negative values for width and height will result in a
+   protocol error.
+  
+  
+  
+
+
+
+  
+   Set a minimum size for the window.
+
+   The client can specify a minimum size so that the compositor does
+   not try to configure the window below this size.
+
+   The width and height arguments are in window geometry coordinates.
+   See set_window_geometry.
+
+   Values set in this way are double-buffered. They will get applied
+   on the next commit.
+
+   The compositor can use this information to allow or disallow
+   different states like maximize or fullscreen and draw accurate
+   animations.
+
+   Similarly, a tiling window manager may use this information to
+   place and resize client windows in a more effective way.
+
+   The client should not rely on the compositor to obey the minimum
+   size. The compositor may decide to ignore the values set by the
+   client and request a smaller size. In such a case, the client can
+   either adapt to the requested size or refuse to acknowledge the
+   configure event sent by the compositor. See xdg_surface.configure
+   and xdg_surface.ack_configure for details.
+
+   If never set, or a value of z