Re: [glean-dev] RFC: dsconfig matching changes

[Allen A. <ak...@ar...>]

On Tue, Dec 08, 2009 at 10:23:39AM -0700, Brian Paul wrote:
>...
> In particular, this bit of code is used to compare the Z and stencil  
> attributes:
>
>    if (z && c.z)
>       error += abs(z - c.z);
>    if (s && c.s)
>       error += abs(s - c.s);

The (maybe too poorly documented) reason for this is mentioned in the
comments at the head of the function:

//      To keep this problem manageable, we'll assume that both the config
//      to be matched (call it the ``A'' config) and the vector of configs to
//      choose from (call them the ``B'' configs) were selected by a test
//      using a single filter.  Thus we can ignore any differences in buffer
//      availability (because we know those are irrelevant to the test), and
//      concentrate on picking configs for which the available buffers are
//      (in some sense) closest in size.

If a test must have depth and stencil buffers to produce meaningful
results, then its drawing surface filter should require a nonzero "z"
and "s".  As a result, all drawing surface configs passed to match() for
that test will have nonzero "z" and "s" values.  That eliminates the
case you ran into.

On the flip side, if a test doesn't care whether it has depth or stencil
buffers, then its drawing surface filter won't specify anything for
those.  As a result, some of the drawing surface configs passed to
match() will have zero "z" and "s" values, and some won't, but that
difference isn't significant.  The code increments the error total only
if both surfaces have nonzero "z" or "s" values -- that is, a missing
buffer doesn't affect the error total one way or another, an exact match
doesn't increase the error total, but a failure to match two nonzero
depths increases the error total.

So, my first guess is that the test that exhibits this problem doesn't
have a proper drawing surface filter.  Does changing that fix the
glitch, or do we still need a better match()?

> Furthermore, other fields like db and stereo weren't even compared.

If any test requires db or stereo in its drawing surface filter, then
match() should check those.  I don't remember that there were any such
tests, which may explain why match() didn't check those buffers.  But if
you change it, it still needs to do the right thing (as above) for tests
that don't care about db or stereo.

> To fix this, I modified the match function to first look for an _exact_ 
> match where all fields (including X visual ID) were the same...

The reason I didn't do this in the first place was that glean might be
applied to compare runs from two different driver versions or two
different drivers, so an XID match might be bogus.

Allen