<div dir="ltr">On 17 September 2013 12:18, Paul Berry <span dir="ltr"><<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div class="h5">On 15 September 2013 00:10, Francisco Jerez <span dir="ltr"><<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>></span> wrote:<br>
</div></div><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
    /**<br>
+    * Return the amount of atomic counter storage required for a type.<br>
+    */<br>
+   unsigned atomic_size() const<br>
+   {<br>
+      if (base_type == GLSL_TYPE_ATOMIC_UINT)<br>
+         return ATOMIC_COUNTER_SIZE;<br>
+      else if (is_array())<br>
+         return length * element_type()->atomic_size();<br>
+      else<br>
+         return 0;<br>
+   }<br></blockquote></div></div><div><div class="h5"><div><br></div></div></div><div>Can atomic counters appear inside structs?  If so, we probably need an is_record() case here.  If not, it would be nice to have a comment explaining why it's unnecessary.<br>
</div></div></div></div></blockquote><div><br></div><div>I also notice that most of your uses of atomic_size() in this patch series are merely to see whether the type contains any atomics.  You might consider adding a contains_atomic() function (like we've already done with contains_sampler() and contains_integer()) just for code clarity.<br>
</div></div></div></div>