Sprite transforms in RandR

[Aaron Plattner]

On Sun, Dec 05, 2010 at 06:15:04PM -0800, Keith Packard wrote:
> 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.

Okay, I figured it was something like that.  The actual hardware rotation
is configured when setting the crtc config then, right?

> > 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?

Yes, that sounds reasonable.  We don't actually have any hardware like
that; after a lot of previous chips were huge pains in the butt, we've had
homogeneous crtc capabilities for the last several chip generations,
including Tegra.  Still, that seems like the sort of limitation that
hardware designers love to add so we should definitely make the protocol
capable of expressing it.

I'm less concerned about the ability to have multiple formats for different
limitations, but it seems like somebody might want to make a scanout engine
that can display rotated but only up to a certain size, and then display
unrotated up to a larger size.  I guess you can distinguish between those
based on the scanout pixmap size, format, and possible rotations?

> > 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.

Thanks, that makes sense.  Could a condensed version of this please go into
randrproto.txt?

> > 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.

Fair enough.

> > 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.

I like that plan.

> > 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...

Sorry this didn't get in tonight, I was on the road most of the day.  It's
getting late here so I'll need to look at these first thing tomorrow.
Delayed review is better than tired review, hopefully.

-- Aaron