On Fri, Aug 31, 2012 at 10:16 PM, Kenneth Graunke <kenn...@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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
