[PATCH 07/10] glamor: Add glamor_program PolyPoint implementation

[Keith Packard]

Markus Wick <markus at selfnet.de> writes:


>> +    glamor_get_context(glamor_priv);
>> +
>> +    if (prog->failed) {
>> +        glamor_put_context(glamor_priv);
>
> Why don't you check this issue before calling glamor_get_context?

Could do, but compiles "shouldn't" fail?

> What do you think about a small API change for glamor_build_program?
> imo it would be cleaner to just call glamor_build_program everytime and 
> do the fail/successful and already build checks there. Here we don't 
> care about everything else but true/false.

Yeah, that sounds like a nice cleanup. A bit at odds with your previous
comment though :-)

> How often does this loop iterate typically?

Once is the most common. It might be nice to capture some statistics
From applications and see if they're using more complicated clip lists
these days though.

> Maybe it's worth to think about a way to also upload all boxes.

Depends if we think drawing with more than one clip box is interesting;
all of my data (from years ago, alas) said that applications drew almost
everything off-screen to a pixmap and then copied to the screen, so
rendering was always done with one rect, but blts sometimes had a few more.

> I guess box must be reseted in the pixmap_loop.

Yeah, I don't remember if there was a bug in this version, but the newer
one makes it clear that both nbox and box are reset for each loop.

> We are used to also disable the gl program here unnecessaryly.
> I want to strip them all, so I'm fine without it, but it isn't fair for 
> a performance comparison.

I stripped them out in an earlier patch; we could measure performance at
that point?

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 810 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20140320/0dafc2cb/attachment-0001.sig>