On 12/20/2013 09:26 PM, Kenneth Graunke wrote:
On 11/27/2013 05:28 AM, Petri Latvala wrote:
v2: Don't add function parameters, pass the required size in
prog_data->nr_params.
Signed-off-by: Petri Latvala <petri.latv...@intel.com>
---
src/mesa/drivers/dri/i965/brw_vec4.h | 5 +++--
src/mesa/drivers/dri/i965/brw_vec4_gs.c | 5 +++++
src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 7 +++++++
src/mesa/drivers/dri/i965/brw_vs.c | 8 ++++++++
4 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h
b/src/mesa/drivers/dri/i965/brw_vec4.h
index 5cec9f9..5f5f5cd 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.h
+++ b/src/mesa/drivers/dri/i965/brw_vec4.h
@@ -325,8 +325,9 @@ public:
*/
dst_reg output_reg[BRW_VARYING_SLOT_COUNT];
const char *output_reg_annotation[BRW_VARYING_SLOT_COUNT];
- int uniform_size[MAX_UNIFORMS];
- int uniform_vector_size[MAX_UNIFORMS];
+ int *uniform_size;
+ int *uniform_vector_size;
+ int uniform_param_count; /*< Size of uniform_[vector_]size arrays */
I'm not crazy about this variable name. Between the params arrays,
uniform_* arrays, nr_params count, and uniforms count...we already have
a lot of distinct things that sound alike.
How about:
int uniform_array_size; /**< Size of uniform_[vector_]size arrays. */
That seems clearer to me. Especially seeing that the value here is
really the size of the array, which is an overestimate/upper bound on
the number of uniforms, not the actual number of elements in the
uniforms or params arrays.
Sounds good. Changing the name.
int uniforms;
src_reg shader_start_time;
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs.c
b/src/mesa/drivers/dri/i965/brw_vec4_gs.c
index 018b0b6..7cf9bac 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_vec4_gs.c
@@ -64,6 +64,11 @@ do_gs_prog(struct brw_context *brw,
c.prog_data.base.param = rzalloc_array(NULL, const float *, param_count);
c.prog_data.base.pull_param = rzalloc_array(NULL, const float *,
param_count);
+ /* Setting nr_params here NOT to the size of the param and pull_param
+ * arrays, but to the number of uniform components vec4_visitor
+ * needs. vec4_visitor::setup_uniforms() will set it back to a proper value.
+ */
+ c.prog_data.base.nr_params = param_count / 4 + gs->num_samplers;
Hmm. You're counting the number of vec4s, but...don't samplers take up
a single entry, since they're just integers? This seems odd to me.
No, that value is the number of float-size components. param_count,
calculated above that code, multiplies the number of components by 4 to
have room elsewhere for scalar params that eat up a vec4. I'm just
dividing the number back.
You might also consider doing ALIGN(param_count, 4) / 4 so that you
round up rather than truncating on the division.
Ok, changing that. Here and in vs.
I also would really like to keep nr_params in consistent units, i.e.
always uniform float-size components or always vec4s.
I would really like that too, but unfortunately the inconsistency was
already present.
The best option would be to have another variable for uniform memory
use, but I didn't want to make too invasive changes. nr_param gets used
here for uniform float-size counts, and later on for something else.
This results in some overuse of memory when uniforms are smaller than vec4s.
if (gp->program.OutputType == GL_POINTS) {
/* When the output type is points, the geometry shader may output data
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index a13eafb..b9226dc 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -3253,6 +3253,10 @@ vec4_visitor::vec4_visitor(struct brw_context *brw,
fail_msg(NULL),
first_non_payload_grf(0),
need_all_constants_in_pull_buffer(false),
+ /* Initialize uniform_param_count to at least 1 because gen6 VS requires
at
+ * least one. See setup_uniforms() in brw_vec4.cpp.
+ */
I think you mean "Gen4-5 requires at least one push constant", not "gen6
VS." At least, that's what setup_uniforms() is doing.
Argh, yes. Fixing that comment.
+ uniform_param_count(prog_data->nr_params ? prog_data->nr_params : 1),
I think this would be clearer as:
MAX2(prog_data->nr_params, 1)
Yes, changing.
--
Petri Latvala
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
