Gradients for Xrender

[Lars Knoll]

On Wednesday 01 June 2005 19:02, David Reveman wrote:
> On Mon, 2005-05-30 at 19:22 +0200, Lars Knoll wrote:
> Nice to see that someone is finally doing some work on gradients for
> Render.
>
> I've had a quick look at the xserver patch and I've got a few questions
> about the patch and some comments about the protocol changes. I'll start
> with the protocol changes.
>
> I'm not a big fan of sticking the gradients into a read-only Picture. I
> don't like that we'll have a bunch of picture attributes that only works
> with certain types of pictures. Can you set a source clip for a
> gradient? alpha map? filter? component alpha? 

A read only picture is rather easy to do, and I don't think we gain anything 
by separating the types. IMO it would make the whole API harder to use and 
implementation in the server more difficult). An additional type doesn't give 
you a whole lot more then some syntactic sugar. 

> Having repeat and pad[*] 
> attributes in pictures and also a spread type in gradients is very
> confusing. I think the extend attributes for pictures need to be fixed
> and it seems like a good idea to use them for gradients. I think the
> extend types supported by glitz are good: Transparent, Nearest, Repeat
> and Reflect. I'm not sure how useful Transparent is with gradients but
> it's easy to implement, so for consistency we might just support that
> with gradients too. I've got a patch for libpixman that implements these
> extend types. If people think it's a good idea, I'll make sure it gets
> into libpixman and the xserver soon.

That would mean either 
(a) changing the repeat member of the XRenderPictureAttributes from a boolean 
to an int. Since we're in C we can do this without breaking compatibilty 
(provided Transparent = 0 and Repeat = 1).
or (b) adding a second spread attribute and deprecate repeat.

Both ways I wouldn't mind moving the attribute in there, and adding 
Transparent as an additional spread for gradients.

Does anyone have any strong opinions about the two alternatives (a) or (b)?

> We might also want to use the filter attribute with gradients but I
> don't think anything but the filter aliases (Fast, Good, Best) makes
> sense for gradients and I'm not sure what to do about that.

Well, none of them makes sense, as we can calculate the gradient point (0...1) 
exactly even with transformations (and without additional overhead). So 
you'll always get "Best". But we shouldn't complain if anyone tries to set 
them.

> I like the pattern idea that Owen mentioned earlier better than
> read-only pictures, but that's a pretty large change to Render and I
> understand if people like to avoid that. If we fix the extend
> attributes, I think I can live with the read-only pictures for
> gradients.
>
> In your patch, I see that you're computing a color table when creating a
> gradient. If you render gradients that are larger than the color table
> or if you just use a transformation that enlarge the gradient a lot, the
> results can be very wrong.

As long as you have not too many stop points you should be ok. The fixed point 
precision does limit you to a maximum of 65000 stops anyway.  The color table 
could be wrong if someone adds a 1000 stops, but that's more of a theoretical 
problem. 

We've tried doing the calculations directly in our Qt code, and found out that 
you can gain a factor of 5 in speed using a precalculated color table. We 
could store both the exact stops and a precalculated color table in the 
server if this really turns out to be a problem.

> Are the color stops pre-multiplied or non-pre-multiplied? We'll need to
> support non-pre-multiplied color stops. Maybe we want to support
> pre-multiplied color stops as well, I'm not sure.

Why would you need non premultiplied color stops? You can always convert them 
to premultiplied on the client side. All of the Render API currently expects 
premultiplied colors and I see no reason to change that. 

> I'd like to see radial gradients with support for an inner radius, at
> least in libpixman, as we could then throw away my software gradient
> code in cairo and use libpixman instead. It's actually a lot harder to
> implement the inner radius than just a focal point. My inner radius code
> in cairo seems to be working fine but it's not very efficient and it's
> all in floating point. Let me know if you figure out a smarter way of
> doing them.

It's not hard to do an inner radius. If you need it I can add it to our 
gradient code.

> Overall I think the patch looks good. I'll hook up glitz's gradient
> acceleration in Xgl once this goes into CVS.

Good :)
I'll submit it then as soon as we have an agreement on the final protocal and 
client API.

Cheers,
Lars