[PATCH randrproto] Add "Border" and "BorderDimensions" properties

[Stéphane Marchesin]

2011/8/2 Aaron Plattner <aplattner at nvidia.com>:
> On Tue, Aug 02, 2011 at 01:54:39PM -0700, Stéphane Marchesin wrote:
>> 2011/8/2 Aaron Plattner <aplattner at nvidia.com>:
>> > On Tue, Aug 02, 2011 at 01:15:19PM -0700, Stéphane Marchesin wrote:
>> >> On Fri, Jul 29, 2011 at 12:27, Aaron Plattner <aplattner at nvidia.com> wrote:
>> >> > Signed-off-by: Aaron Plattner <aplattner at nvidia.com>
>> >> > ---
>> >> > Does this interface make sense? I tried to describe what it should do as
>> >> > accurately as possible.
>> >> >
>> >> > I'm working on a corresponding change to the server to make it read this
>> >> > property and apply it during modesets. This is option #3 as described in
>> >> > http://lists.x.org/archives/xorg-devel/2011-July/023888.html
>> >> >
>> >> >  randr.h        |    2 +
>> >> >  randrproto.txt |   87 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
>> >> >  2 files changed, 84 insertions(+), 5 deletions(-)
>> >> >
>> >> > diff --git a/randr.h b/randr.h
>> >> > index 41aedd5..9cb9403 100644
>> >> > --- a/randr.h
>> >> > +++ b/randr.h
>> >> > @@ -181,5 +181,7 @@ typedef unsigned long       XRandrModeFlags;
>> >> >  #define RR_PROPERTY_CONNECTOR_NUMBER   "ConnectorNumber"
>> >> >  #define RR_PROPERTY_COMPATIBILITY_LIST "CompatibilityList"
>> >> >  #define RR_PROPERTY_CLONE_LIST         "CloneList"
>> >> > +#define RR_PROPERTY_BORDER             "Border"
>> >> > +#define RR_PROPERTY_BORDER_DIMENSIONS  "BorderDimensions"
>> >> >
>> >> >  #endif /* _RANDR_H_ */
>> >> > diff --git a/randrproto.txt b/randrproto.txt
>> >> > index ce462ea..03a5351 100644
>> >> > --- a/randrproto.txt
>> >> > +++ b/randrproto.txt
>> >> > @@ -138,6 +138,12 @@ underlying hardware to clients
>> >> >      providing the server a complete configuration for appropriate
>> >> >      resource management.
>> >> >
>> >> > +   • An optional Border property. This property allows a client to
>> >> > +     specify that the viewport of the CRTC is smaller than the actve
>> >>
>> >> active
>> >
>> > Whoops, thanks.
>> >
>> >> > +     display region described its mode.  This is useful, for example,
>> >> > +     for compensating for the overscan behavior of certain
>> >> > +     televisions.
>> >> > +
>> >> >  The first two additions, per-crtc pixmaps and sprite transforms are
>> >> >  designed to solve two problems:
>> >> >
>> >> > @@ -1006,8 +1012,8 @@ dynamic changes in the display environment.
>> >> >
>> >> >        'width' and 'height' indicate the size of the area within the screen
>> >> >        presented by this CRTC. This may be different than the size of the
>> >> > -       mode due to rotation. They will be set to 0 when the CRTC
>> >> > -       is disabled.
>> >> > +       mode due to rotation, the projective transform, and the Border property
>> >> > +       described below.  They will be set to 0 when the CRTC is disabled.
>> >> >
>> >> >        'mode' indicates which mode is active, or None indicating that the
>> >> >        CRTC has been disabled and is not displaying the screen contents.
>> >> > @@ -1585,8 +1591,8 @@ factors, such as re-cabling a monitor, etc.
>> >> >        rotation: ROTATION;             new rotation
>> >> >        x: INT16                        x position of CRTC within screen
>> >> >        y: INT16                        y position of CRTC within screen
>> >> > -       width: CARD16                   width of new mode
>> >> > -       height: CARD16                  height of new mode
>> >> > +       width: CARD16                   width of new configuration
>> >> > +       height: CARD16                  height of new configuration
>> >> >  └───
>> >> >        This event is generated whenever the CRTC configuration is changed
>> >> >        and sent to requesting clients. 'timestamp' indicates when the
>> >> > @@ -1594,7 +1600,9 @@ factors, such as re-cabling a monitor, etc.
>> >> >        event. 'mode' is the new mode, or None if the crtc is disabled.
>> >> >        'x' and 'y' mark the location in the screen where this CRTC
>> >> >        is reading data. 'width' and 'height' indicate the size of the
>> >> > -       mode. 'x', 'y, 'width' and 'height' are all zero when 'mode' is None.
>> >> > +       CRTC viewport, which is the mode size adjusted by the optional
>> >> > +       Border output property described below. 'x', 'y, 'width' and
>> >> > +       'height' are all zero when 'mode' is None.
>> >> >
>> >> >        This event is sent whenever the monitor's configuration changes
>> >> >        or if a new monitor configuration becomes available that was
>> >> > @@ -1766,6 +1774,68 @@ doesn't handle a mandatory property correctly.
>> >> >        a signal change (e.g. TV formats).  Clients are allowed to change the
>> >> >        properties in order to select a different signal subformat.
>> >> >
>> >> > +    "Border"                   aka RR_PROPERTY_BORDER
>> >> > +       Type:                   int16 [n]
>> >
>> > I'm thinking maybe I should change this to uint16, to match the type of the
>> > width/height fields of MODEINFO.  I don't know why you'd want a 65535x65535 mode
>> > with a 1x1 active area, but it seems silly to disallow it in the protocol.
>> >
>>
>> I thought about it, but didn't see any unsigned int anywhere else in
>> the file. Not sure if that was intentional.
>
> You mean in properties?  It's definitely possible, using type XA_CARDINAL.
> It might be worth changing these types to the corresponding type atoms &
> formats just for clarity's sake.
>
> The mode sizes are CARD16, so they'd corresponding to property type
> XA_CARDINAL, format=16.
>
>> >> > +       Flags:                  Immutable
>> >>
>> >> Hmm, why Immutable?
>> >
>> > Immutable just prevents clients from reconfiguring or deleting the
>> > property.  It doesn't prevent clients changing the size or contents of the
>> > property (I know, that confused me too).
>> >

Reviewed-by: Stéphane Marchesin <marcheu at chromium.org>

Stéphane