Sprite transforms in RandR

[Keith Packard]

On Sun, 5 Dec 2010 16:38:52 -0800, Aaron Plattner <aplattner at nvidia.com> wrote:

Thanks for reviewing over the weekend.

> There are a few whitespace bugs, such as this:
> ┌───
>     RRQueryScanoutPixmaps
> \twindow: WINDOW
>       ▶
>         infos: LISTofSCANOUTPIXMAPINFO
> └───
> \tErrors: Window

I'll clean up the whitespace.

> The actual text sounds reasonable to me, but it's not clear what the
> 'rotations' field of RRCreateScanoutPixmap is used for.  Is it a set of
> hardware rotation modes that might be used for that crtc with that
> pixmap?

Yes. Daniel Stone was working with some whacky omap hardware last year
and if he wanted to have a scanout pixmap that *could* be rotated, he'd
need to know in advance.

> Also, is the set of "supported" rotations screen-wide, or associated with the
> particular format passed in?  Is it possible for a driver to list multiple
> SCANOUTPIXMAPINFO with matching formats?  E.g. I wonder if there's hardware
> that has, say, hardware rotation for one crtc but not another.  It sounds like
> RandR expects the crtcs to have matching capabilities.

Good point. I hadn't considered the case where different crtcs would
have different rotation support. Definitely seems possible
though. The current design doesn't tie the scanout pixmaps to a specific
crtc, so I didn't think of having per-crtc scanout pixmap formats.

I don't want to design beyond the hardware, so perhaps Daniel will know
if this was the case in the hardware he was dealing with.

If so, I'd suggest that the whole scanout pixmap formats stuff should be
per-crtc. And, if you want to use one scanout pixmap with multiple
crtcs, you'd better make sure they both support the desired format.

Seem reasonable?

> The whitespace for RRCrtcSetSpriteTransform has spaces followed by
> tabs.

Yay, whitespace bugs.

> Can you clarify the distinction between the screen size and the screen pixmap
> size?

The screen size is the size seen by applications. It is the size of the
root window, and also the size which input coordinates will be clipped
to. It forms the effective surface on which the desktop can be
constructed.

The screen pixmap size is the size of the backing store for any painting
happening on the screen. In a non-composited environment, all output to
all windows will be clipped to the screen pixmap.

These used to be tied together; it didn't make sense to have a screen
pixmap smaller than the root window; there wouldn't be any way to have
content visible beyond the screen pixmap.

With PCP and sprite transforms, you can now construct logical screen
regions that are visible (via PCP) and can be manipulated (via ST) as 
a part of the screen environment.

A simple example of this is the CRTC rotation case. With PCP and ST, the
compositing manager will allocate a CRTC pixmap to match the desired
mode. Window contents will be copied to the crtc pixmap through a
rotation transform, resulting in a rotated presentation of the
desktop. The screen pixmap, in this case, contains no useful content at
all. So, we set the screen pixmap size to some tiny little value, and
set the screen size to match the logical rotated dimensions.

A larger example is if you want to provide an extended desktop across
multiple monitors but your hardware can't scan out from a pixmap of that
size. Create per-crtc pixmaps for each crtc, set the screen pixmap to
'tiny', and set the screen size to match the full dimension of the
desktop.

> Do you think it's worth making the sizes larger than CARD16 for future
> expansion?  We're going to need to rev a lot of stuff if we want to move to
> larger sizes, but maybe it's better to start doing it piecemeal when we're
> already changing extensions.

I think that discussion needs to happen, but not in the context of minor
updates to existing extensions.

> Can you clarify the distinction between crtc (x, y) and (pixmap_x,
> pixmap_y)?  I guess the (x, y) position is used to produce the input sprite
> position for that crtc?  It would be nice to have that be spelled out a
> little more in randrproto.txt.

Sure.

crtc x,y set the position of the crtc in screen space; the default
input transformation uses these values to translate global sprite
coordinates to crtc-specific coordinates. And, without a scanout pixmap,
these coordinates also determine where in the screen pixmap the crtc
scans out from.

crtc xPixmap,yPixmap set the scanout location of the crtc within the
specific scanout pixmap. These are only used if the crtc has a scanout
pixmap separate from the screen pixmap.

> There are some unrelated whitespace changes, and a tab-space-tab pattern in
> the argument list for XRRSetCrtcConfigs.

Perhaps emacs can fix my whitespace problems.

> > xrandr changes:
> > 
> >         git://people.freedesktop.org/~keithp/xrandr master
> 
> I just skimmed this.  It seemed okay.
> 
> Acked-by: Aaron Plattner <aplattner at nvidia.com>

Yeah, the PCP stuff here is just a hack to show that it works, it's not
actually useful.

> > xserver changes:
> > 
> >         git://people.freedesktop.org/~keithp/xserver randr-1.4-subwork
> 
> There's a "scaout_pixmap" in randrstr.h.

Seems to have been fixed in my patch resequencing adventures.

> The crtc shadow functions seem obsolete given the new scanout pixmap
> creation function, but they can be removed in a later ABI.

The new scanout pixmap functions don't allow for the creation of a
scanout pixmap at server startup time, to do that, we'd have to create
some new 'scanout pixmap' object and then lazily create the PixmapRec as
needed.

Frankly, if we get compositing managers to deal with rotation
internally, I see us getting rid of the existing software transform
stuff entirely.

> Is the "client->swapped" code in the protocol handling ever used?  It
> looks like the SProc versions are plugged into the dispatch vector and they
> just return BadImplementation.

These are fixed in the version I posted to the list, and which is now on
my randr-1.4 branch. Request swapping is not that exciting, but it is
necessary...

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20101205/29b18e1e/attachment.pgp>