-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 06/28/2011 06:49 AM, Paul Berry wrote:
> On 27 June 2011 18:30, Ian Romanick <i...@freedesktop.org> wrote:
>> I like this a lot. It's a really good clean up of a rotting cesspool.
>
> Thanks!
>
>>> that are avaiable in geometry shaders.
>>
>> available
>
> *smacks forehead* Oops.
>
>>> + /* Name of the extension when referred to in a GLSL extension
>>> + * statement */
>>
>> Field comments should get the doxygen
>>
>> /**
>> * Short description
>> *
>> * Detailed description
>> */
>>
>> or
>>
>> /** Description */
>>
>> treatment.
>>
>> Also, we generally prefer the closing */ of a multiline comment to be on
>> its own line.
>
> Ok, I can do doxygen-style. BTW, is there a web site where the
> doxygen-extracted documentation for mesa is automatically uploaded?
> It would be convenient to be able to browse the docs without having to
> build them locally.
>
>>> + /* Flag in the gl_extensions struct indicating whether this
>>> + * extension is supported by the driver, or
>>> + * &gl_extensions::dummy_true if supported by all drivers */
>>> + const GLboolean gl_extensions::* supported_flag;
>>
>> WTF? Seriously. What does this do? I was expecting to see this code
>> use the offsetof macro (like src/mesa/main/extensions.c does), but I'm
>> now suspecting that C++ has some built-in magic for this. Is that true?
>
> Yes. The feature is called "pointer to data member" and I'm surprised
> it doesn't get more press, considering how frequently people reinvent
> this particular wheel. The syntax is pretty straightforward once you
> get the hang of it:
>
> - foo bar::* p declares p to be an "offset" to a field of type foo
> that exists within struct bar
> - &bar::baz computes the "offset" of field baz within struct bar
> - x.*p accesses the field of x that exists at "offset" p
> - x->*p is equivalent to (*x).*p
>
> I hope my use of this C++ feature doesn't come across as too
> newfangled. IMHO it's superior to the offsetof macro because (a) it
> can represent null pointers unambiguously, and (b) the compiler
> detects mistakes like referring to a data member of the wrong type, or
> referring to a member of the wrong class (both of which would be
> uncaught by offsetof).
>
> But it's not always an easy call. Offsetof, for instance, handles
> structures-within-structures and arrays-within-structures more
> gracefully than pointers to data members do.
>
> BTW, if you're worried about whether all C++ compilers support this
> feature, rest assured that it has been part of the language since the
> old CFront days.
Wow. I'm surprised I didn't know about that. Unless Eric objects, I
approve.
>>> +const _mesa_glsl_extension _mesa_glsl_supported_extensions[] = {
>>
>> This should probably be static.
>
> Good point. I'll fix this.
>
>>> + case vertex_shader: if (!this->avail_in_VS) return false; break;
>>> + case geometry_shader: if (!this->avail_in_GS) return false; break;
>>> + case fragment_shader: if (!this->avail_in_FS) return false; break;
>>
>> Delete the spurious breaks.
>
> Geez, what was I smoking?
>
>>> + for (unsigned int i = 0; i < Elements(_mesa_glsl_supported_extensions);
>>> + ++i) {
>>
>> Just 'unsigned'.
>
> Really? I'm surprised you care about this detail, esp. considering
> that there are many instances of "unsigned int" already in Mesa. But
> I'll acquiesce, especially since I'm trying to talk you into letting
> me use a little-known C++ feature above :)
Usually it doesn't matter, but in this case 'unsigned int' causes an
ugly line wrap. If it weren't for that, it would have been too small a
nit for me to pick.
>>> + if (extension->compatible_with_state(state)) {
>>> + _mesa_glsl_supported_extensions[i].set_flags(state,
>>> behavior);
>>
>> extension->set_flags(state, behavior);
>
> Yes, you're right, of course.
>
> Thanks for your comments, Ian. I'll post a revised patch later this morning.
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk4KPfoACgkQX1gOwKyEAw/+7QCgoLweDIyZafb2tt/RYB4vCDuB
ywEAoJyf5QUqz9pfh0OthBfOmVmsIt3r
=DPDN
-----END PGP SIGNATURE-----
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
