<div dir="ltr">On 24 November 2013 21:00, Francisco Jerez <span dir="ltr"><<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</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">---<br>
src/mesa/main/config.h | 1 +<br>
src/mesa/main/dd.h | 1 +<br>
src/mesa/main/mtypes.h | 100 +++++++++++++++++++++++++++++++++++++++++++++++++<br>
src/mesa/main/texobj.c | 1 +<br>
4 files changed, 103 insertions(+)<br>
<br>
diff --git a/src/mesa/main/config.h b/src/mesa/main/config.h<br>
index 22bbfa0..8bd9765 100644<br>
--- a/src/mesa/main/config.h<br>
+++ b/src/mesa/main/config.h<br>
@@ -175,6 +175,7 @@<br>
#define MAX_COMBINED_ATOMIC_BUFFERS (MAX_UNIFORM_BUFFERS * 6)<br>
/* Size of an atomic counter in bytes according to ARB_shader_atomic_counters */<br>
#define ATOMIC_COUNTER_SIZE 4<br>
+#define MAX_IMAGE_UNITS 32<br></blockquote><div><br></div><div>It would be nice to have a note in the commit message explaining why you chose 32 for MAX_IMAGE_UNITS (the spec only requires 8 so it's not obvious).<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
/*@}*/<br>
<br>
/**<br>
diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h<br>
index b5b874f..648062f 100644<br>
--- a/src/mesa/main/dd.h<br>
+++ b/src/mesa/main/dd.h<br>
@@ -39,6 +39,7 @@ struct gl_buffer_object;<br>
struct gl_context;<br>
struct gl_display_list;<br>
struct gl_framebuffer;<br>
+struct gl_image_unit;<br>
struct gl_pixelstore_attrib;<br>
struct gl_program;<br>
struct gl_renderbuffer;<br>
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h<br>
index e9750f4..7be0664 100644<br>
--- a/src/mesa/main/mtypes.h<br>
+++ b/src/mesa/main/mtypes.h<br>
@@ -1207,6 +1207,9 @@ struct gl_texture_object<br>
<br>
/** GL_OES_EGL_image_external */<br>
GLint RequiredTextureImageUnits;<br>
+<br>
+ /** GL_ARB_shader_image_load_store */<br>
+ GLenum ImageFormatCompatibility;<br></blockquote><div><br></div><div>This is called IMAGE_FORMAT_COMPATIBILITY_TYPE in the GL spec. Can we rename it to ImageFormatCompatibilityType for consistency?<br></div><div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
};<br>
<br>
<br>
@@ -2373,6 +2376,29 @@ struct gl_shader<br>
*/<br>
GLenum OutputType;<br>
} Geom;<br>
+<br>
+ /**<br>
+ * Map from image uniform index to image unit (set by glUniform1i())<br>
+ *<br>
+ * An image uniform index is associated with each image uniform by<br>
+ * the linker. The image index associated with each uniform is<br>
+ * stored in the \c gl_uniform_storage::image field.<br></blockquote><div><br></div><div>Can we assume image uniform indices are always in the range [0, NumImages-1]? If so it would be nice to document that here.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ */<br>
+ GLubyte ImageUnits[MAX_IMAGE_UNITS];<br>
+<br>
+ /**<br>
+ * Access qualifier specified in the shader for each image uniform.<br>
+ * Either \c GL_READ_ONLY, \c GL_WRITE_ONLY or \c GL_READ_WRITE.<br>
+ *<br>
+ * It may be different, though only more strict than the value of<br>
+ * \c gl_image_unit::Access for the corresponding image unit.<br></blockquote><div><br></div><div>Is this array indexed by image uniform index or by image unit? The comment should clarify that.<br></div><div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ */<br>
+ GLenum ImageAccess[MAX_IMAGE_UNITS];<br></blockquote><div><br></div><div>With those changes, this patch is:<br><br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br>
</div></div></div></div>