[Mesa-dev] [PATCH 2/2] mesa: utilize context version routines

[Jordan Justen]

On Fri, Aug 31, 2012 at 10:16 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> I'm really ambivalent about these patches.
>
> 1. I'm not a huge fan of the name "have_version"...it sounds like it
> would return whether a driver supports a given version, not whether the
> current context's version is a certain value.

This doesn't really match the feedback you gave in the previous thread
(from July 27). I guess it is different to see the actual effect on
code.

> 2. Personally I think ctx->Version <= XY is clearer than
> !_mesa_have_version(ctx, X, Y + 1).  With the two-digit notation, you
> can just do whatever comparison you like, rather than having to negate
>>= and possibly increment the minor version being compared.

ctx->Version <= XY can still be written as
ctx->Version <= _mesa_uint_version(X, Y).

My original proposal was a macro which would have made it:
ctx->Version <= GLVER(X, Y)
(I actually still prefer this, although, I thought this patchset was
the consensus, and still an improvement.)

I only found one case of this: ctx->Version <= 30

It seems just as clear with !_mesa_have_version(ctx, 3, 1).

> 3. _mesa_have_version(ctx, 3, 1) is longer than ctx->Version >= 31

I don't think this caused an issue for the code altered in 2/2.

> 3. I really don't see the major * 10 + minor notation needing to be
> changed in the future.  Even if Khronos did offer (say) a GL 4.3.1
> release, the likelihood of it making incompatible changes over 4.3 that
> require special checks is infinitesimal.  It would just be clarifications...

I think the XY syntax is not clear, and X, Y helps clarify the major,
minor distinction.

I like that GLSL makes this more clear. It is easy to know what 140
means in the GLSL context.

I also don't like seeing code use major * 10 + minor when someone has
the major/minor in variables and need to compare it.

> Normally, I'm all for encapsulation, but I guess I just don't see much
> point.  That said, I won't object too strongly if people prefer this
> approach.

I prefer this patchset to the current state, but as mentioned above,
it was not my first choice.

-Jordan