<div dir="ltr">On 25 October 2013 16:45, Anuj Phogat <span dir="ltr"><<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Thsi function is used to test if we need to do per sample shading or<br>
per fragment shading.<br></blockquote><div><br></div><div>s/Thsi/This/<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Signed-off-by: Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>><br>
---<br>
 src/mesa/program/program.c | 31 +++++++++++++++++++++++++++++++<br>
 src/mesa/program/program.h |  3 +++<br>
 2 files changed, 34 insertions(+)<br></blockquote><div><br></div><div>This patch fails to compile because it relies on SYSTEM_BIT_SAMPLE_ID and SYSTEM_BIT_SAMPLE_POS, which are added in patch 5.  I think you can fix that by moving this patch after patch 5.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
diff --git a/src/mesa/program/program.c b/src/mesa/program/program.c<br>
index 093d372..e12e6ec 100644<br>
--- a/src/mesa/program/program.c<br>
+++ b/src/mesa/program/program.c<br>
@@ -1024,3 +1024,34 @@ _mesa_postprocess_program(struct gl_context *ctx, struct gl_program *prog)<br>
<br>
    }<br>
 }<br>
+<br>
+/* Gets the minimum number of shader invocations per fragment.<br>
+ * This function is useful to determine if we need to do per<br>
+ * sample shading or per fragment shading.<br>
+ */<br>
+GLint<br>
+_mesa_get_min_invocations_per_fragment(struct gl_context *ctx,<br>
+                                       const struct gl_fragment_program *prog)<br>
+{<br>
+   /* From ARB_sample_shading specification:<br>
+    * "Using gl_SampleID in a fragment shader causes the entire shader<br>
+    *  to be evaluated per-sample."<br>
+    *<br>
+    * "Using gl_SamplePosition in a fragment shader causes the entire<br>
+    *  shader to be evaluated per-sample."<br>
+    *<br>
+    * "If MULTISAMPLE or SAMPLE_SHADING_ARB is disabled, sample shading<br>
+    *  has no effect."<br></blockquote><div><br></div><div>There's a little more text in the spec right after this, which I think is worth quoting:<br><br>    Otherwise, an implementation must provide a minimum of<br>
<br>        max(ceil(MIN_SAMPLE_SHADING_VALUE_ARB*SAMPLES),1) <br><br>    unique color values and sets of texture coordinates for each<br>    fragment.<br><br></div><div>Since that justifies the formula you use in the "else if" block.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+    */<br>
+   if (ctx->Multisample.Enabled) {<br>
+      if (prog->Base.SystemValuesRead & SYSTEM_BIT_SAMPLE_ID ||<br>
+          prog->Base.SystemValuesRead & SYSTEM_BIT_SAMPLE_POS)<br>
+         return ctx->DrawBuffer->Visual.samples;<br>
+      else if (ctx->Multisample.SampleShading)<br>
+         return ceil(ctx->Multisample.MinSampleShadingValue *<br>
+                     ctx->DrawBuffer->Visual.samples);<br></blockquote><div><br></div><div>If the user sets ctx->Multisample.MinSampleShadingValue to 0.0 (which is allowed), then this function will return 0.  I think that risks bugs, since it's the only situation in which this funciton can return 0, so the caller might not realize they need to be prepared for that.<br>
<br>Let's just implement exactly the formula from the spec, which I believe works out to:<br><br>         return MAX2(ceil(ctx->Multisample.MinSampleShadingValue *<br>                          ctx->DrawBuffer->Visual.samples), 1);<br>
<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+      else<br>
+         return 1;<br>
+   }<br>
+   return 1;<br>
+}<br>
diff --git a/src/mesa/program/program.h b/src/mesa/program/program.h<br>
index 34965ab..353ccab 100644<br>
--- a/src/mesa/program/program.h<br>
+++ b/src/mesa/program/program.h<br>
@@ -187,6 +187,9 @@ _mesa_valid_register_index(const struct gl_context *ctx,<br>
 extern void<br>
 _mesa_postprocess_program(struct gl_context *ctx, struct gl_program *prog);<br>
<br>
+extern GLint<br>
+_mesa_get_min_invocations_per_fragment(struct gl_context *ctx,<br>
+                                       const struct gl_fragment_program *prog);<br>
<br>
 static inline GLuint<br>
 _mesa_program_target_to_index(GLenum v)<br>
<span class=""><font color="#888888">--<br>
1.8.1.4<br>
<br>
</font></span></blockquote></div><br></div><div class="gmail_extra">With those changes, the patch is:<br><br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br></div></div>