[PATCH 02/13] randrproto: Panning protocol description

[Keith Packard]

On Fri, 2008-11-28 at 18:29 +0100, Matthias Hopf wrote:

This looks good to me, I only have a few minor comments about the style,
nothing substantial below.

Thanks for getting this together.

> +	Note that changes to the screen size might invalidate panning
> +	parameters.  In these cases panning might be silently disabled, or the
> +	panning parameters are updated automatically as necessary.  The exact
> +	behavior of the implementation is undefined.  If the panning parameters
> +	do not conflict with new screen size, panning remains enabled
> +	unchanged.

I don't see a need to leave this undefined; let's specify what screen
size changes do to panning and then implement it. Or vice-versa. And,
no, I don't care what the defined  behaviour is (it could be as simple
as 'panning is disabled on any screen size change').

> +
>  ┌───
>      RRGetScreenResources
>  	window: WINDOW
> @@ -938,6 +945,12 @@ dynamic changes in the display environment.
>  	then re-enabling the CRTC at the new configuration to avoid an
>  	invalid intermediate configuration.
>  
> +	Note that changes to the CRTC might invalidate panning parameters.  In
> +	these cases panning might be silently disabled, or the panning
> +	parameters are updated automatically as necessary.  The exact behavior
> +	of the implementation is undefined.  If the panning parameters do not
> +	conflict with new CRTC parameters, panning remains enabled unchanged.
> +

Same here -- don't leave this undefined.

>  	When this request succeeds, 'status' contains Success and the
>  	requested changes to configuration will have been made.
>  	
> @@ -1059,6 +1072,103 @@ This request returns the pending and current transforms for the specified
>  CRTC. The pending transform will be the same as the current transform if no
>  new pending transform has been set since the last call to RRSetCrtcConfig.
>  
> +┌───
> +    RRGetPanning
> +	crtc: CRTC
> +	config-timestamp: TIMESTAMP
> +      ▶
> +	status: RRCONFIGSTATUS
> +	timestamp: TIMESTAMP
> +	left, top, width, height: CARD16
> +	track_left, track_top, track_width, track_height: CARD16
> +	border_left, border_top, border_right, border_bottom: INT16
> +└───
> +
> +	Errors: Crtc
> +
> +	Version 1.3 adds panning support again. If multiple crtcs are active
> +	the panning behavior can be defined per crtc individually.
> +	RRGetPanning returns information about the currently set panning
> +	configuration for the specified crtc.
> +
> +	If 'config-timestamp' does not match the current configuration
> +	timestamp (as returned by RRGetScreenResources), 'status' is set to
> +	InvalidConfigTime and the remaining reply data is empty. Otherwise,
> +	'status' is set to Success.

just eliminate the config-timestamp stuff, it isn't useful now that we
have stable resource IDs etc. Note that the rest of the RandR protocol
doesn't look at config-timestamp anymore as it only causes trouble.

> +	If 'timestamp' is less than the time when the configuration was last
> +	successfully set, the request is ignored and InvalidTime returned in
> +	status.
> +	
> +	If 'config-timestamp' is not equal to when the CRTC's configuration
> +	last changed, the request is ignored and InvalidConfigTime returned in
> +	status. This could occur if the CRTC changed since you last made a
> +	RRGetCrtcInfo request, perhaps by setting a different mode.  Rather
> +	than allowing an incorrect call to be executed based on stale data,
> +	the server will ignore the request.

I'd say all of this timestamp stuff can be eliminated.

> +	'left', 'top', 'width', and 'height' contain the total panning area
> +	for this CRTC.  'width' has to be larger than the CRTC's width, and
> +	'left'+'width' must be within the screen size, else a Match error
> +	results.  Equivalent restrictions for the height exist.  The exception
> +	is 'width' == 'height' == 0 which indicates that panning should be
> +	disabled.

I'd say that width >= crtc_width, the same for height. That way, you can
disable panning in one dimension while leaving it enabled in the other.
Also, if width == crtc_width and height == crtc_height, then panning is
disabled entirely. 

You could make width == 0 mean to set width to the crtc_width (like
ClearArea does).

> +	'track_left', 'track_top', 'track_width', and 'track_height' contain
> +	the pointer area for which the panning region is updated.  For normal
> +	use cases it should enclose the panning area minus borders, and is
> +	typically set to either the panning area minus borders, or to the
> +	total screen size. If set to the total screen size, the CRTC will pan
> +	in the remaining axis even if the pointer is outside the panning area
> +	on a different CRTC.

So, if the pointer is within this space, then the crtc is moved as far
as possible within the panning region to try and make the cursor
visible?

> +	'border_left', 'border_top', 'border_right', and 'border_bottom'
> +	define the distances from the CRTC borders that will activate panning
> +	if the pointer hits them.  If the borders are 0, the screen will pan
> +	when the pointer hits the CRTC borders (behavior of pre-RandR Xserver
> +	panning).  If the borders are positive, the screen will pan when the
> +	pointer gets close to the CRTC borders, if they are negative, the
> +	screen will only pan when the pointer is already way past the CRTC 
> +	borders.  Negative values might confuse users and are discouraged.
> +	border_left + border_right has to be lower or equal than the CRTC's
> +	width, else a Match error results.  An equivalent restriction for the
> +	height exists.

Is there some reason to use borders instead of a separate rectangle
here?

Also, a couple of ascii-art pictures here would help a huge amount. I
had to read these several times to get any idea of what they all do, and
I'm still not sure I understand.

> +	This request sets the panning parameters.  As soon as panning is
> +	enabled, the CRTC position can change with every pointer move.
> +	RRCrtcChangeNotify events are sent to the clients requesting those.
> +
> +	Note that changes to the CRTC or screen might invalidate panning
> +	parameters.  In these cases panning might be silently disabled, or the
> +	panning parameters are updated automatically as necessary.  The exact
> +	behavior of the implementation is undefined.  If the panning parameters
> +	do not conflict with new CRTC parameters or screen size, panning
> +	remains enabled unchanged.

As above, we need well defined behaviour.

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg/attachments/20081128/4b38810f/attachment.pgp>