On 08/18/2013 05:58 PM, Kenneth Graunke wrote:
On 08/15/2013 10:52 PM, Paul Berry wrote:
On 14 August 2013 18:55, Kenneth Graunke <kenn...@whitecape.org
<mailto:kenn...@whitecape.org>> wrote:

    Now that we have the number of samplers available, we don't need to
    iterate over all 16.  This should be particularly helpful for vertex
    shaders.

    Signed-off-by: Kenneth Graunke <kenn...@whitecape.org
    <mailto:kenn...@whitecape.org>>
    ---
      src/mesa/drivers/dri/i965/brw_fs.cpp | 2 +-
      src/mesa/drivers/dri/i965/brw_vs.c   | 2 +-
      2 files changed, 2 insertions(+), 2 deletions(-)

    diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
    b/src/mesa/drivers/dri/i965/brw_fs.cpp
    index 69e544a..27263fd 100644
    --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
    +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
    @@ -3143,7 +3143,7 @@ brw_fs_precompile(struct gl_context *ctx,
    struct gl_shader_program *prog)

         key.clamp_fragment_color = ctx->API == API_OPENGL_COMPAT;

    -   for (int i = 0; i < MAX_SAMPLERS; i++) {
    +   for (unsigned i = 0; i < brw->wm.sampler_count; i++) {


Precompile is called during link, before the program is made current.
So brw->wm is some other program.  I think you need to do this instead:
_mesa_fls(fp->Base.SamplersUsed).

            if (fp->Base.ShadowSamplers & (1 << i)) {
               /* Assume DEPTH_TEXTURE_MODE is the default: X, X, X, 1 */
               key.tex.swizzles[i] =
    diff --git a/src/mesa/drivers/dri/i965/brw_vs.c
    b/src/mesa/drivers/dri/i965/brw_vs.c
    index 5b8173d..7df93c2 100644
    --- a/src/mesa/drivers/dri/i965/brw_vs.c
    +++ b/src/mesa/drivers/dri/i965/brw_vs.c
    @@ -544,7 +544,7 @@ brw_vs_precompile(struct gl_context *ctx, struct
    gl_shader_program *prog)
         key.base.program_string_id = bvp->id;
         key.base.clamp_vertex_color = ctx->API == API_OPENGL_COMPAT;

    -   for (int i = 0; i < MAX_SAMPLERS; i++) {
    +   for (unsigned i = 0; i < brw->vs.sampler_count; i++) {


Similarly, this should be _mesa_fls(vp->Base.SamplersUsed).

With those fixes, this patch is:

Reviewed-by: Paul Berry <stereotype...@gmail.com
<mailto:stereotype...@gmail.com>>

Yikes, you're right.  I think I'll just drop this patch - it's not worth
the complexity, especially since I doubt there's any measureable benefit.

--Ken

I take that back. Both patches need to land, or neither...otherwise precompile sets everything to XYZW, but the actual key setup sets them to 0, and we get pointless recompiles.

I've gone ahead and pushed a corrected version of this patch so they match.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to