On 20 August 2013 11:30, Paul Berry <stereotype...@gmail.com> wrote:
> When I initially generaized the vec4_visitor class in preparation for
> geometry shaders, I assumed that the setup_attributes() function would
> need to be different between vertex and geometry shaders, but its
> caller, setup_payload(), could be shared. So I made
> setup_attributes() a virtual function.
>
>From our in-person code review yesterday:
s/generaized/generalized/ in the commit message.
>
> It turns out this isn't true; setup_payload() needs to be different
> too, since the geometry shader payload sometimes includes an extra
> register (primitive ID) that has to come before uniforms.
>
> So setup_payload() needs to be the virtual function instead of
> setup_attributes().
> ---
> src/mesa/drivers/dri/i965/brw_vec4.cpp | 2 +-
> src/mesa/drivers/dri/i965/brw_vec4.h | 6 +++---
> src/mesa/drivers/dri/i965/test_vec4_register_coalesce.cpp | 3 +--
> 3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index bfef8e0..abdf3ab 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -1288,7 +1288,7 @@ vec4_visitor::setup_uniforms(int reg)
> }
>
> void
> -vec4_visitor::setup_payload(void)
> +vec4_vs_visitor::setup_payload(void)
> {
> int reg = 0;
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h
> b/src/mesa/drivers/dri/i965/brw_vec4.h
> index 587cb45..171f14d 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
> @@ -367,7 +367,6 @@ public:
> void setup_uniform_values(ir_variable *ir);
> void setup_builtin_uniform_values(ir_variable *ir);
> int setup_uniforms(int payload_reg);
> - void setup_payload();
> bool reg_allocate_trivial();
> bool reg_allocate();
> void evaluate_spill_costs(float *spill_costs, bool *no_spill);
> @@ -539,7 +538,7 @@ protected:
> void emit_vertex();
> void lower_attributes_to_hw_regs(const int *attribute_map);
> virtual dst_reg *make_reg_for_system_value(ir_variable *ir) = 0;
> - virtual int setup_attributes(int payload_reg) = 0;
> + virtual void setup_payload() = 0;
> virtual void emit_prolog() = 0;
> virtual void emit_program_code() = 0;
> virtual void emit_thread_end() = 0;
> @@ -562,7 +561,7 @@ public:
>
> protected:
> virtual dst_reg *make_reg_for_system_value(ir_variable *ir);
> - virtual int setup_attributes(int payload_reg);
> + virtual void setup_payload();
> virtual void emit_prolog();
> virtual void emit_program_code();
> virtual void emit_thread_end();
> @@ -570,6 +569,7 @@ protected:
> virtual vec4_instruction *emit_urb_write_opcode(bool complete);
>
> private:
> + int setup_attributes(int payload_reg);
> void setup_vp_regs();
> dst_reg get_vp_dst_reg(const prog_dst_register &dst);
> src_reg get_vp_src_reg(const prog_src_register &src);
> diff --git a/src/mesa/drivers/dri/i965/test_vec4_register_coalesce.cpp
> b/src/mesa/drivers/dri/i965/test_vec4_register_coalesce.cpp
> index e141305..ab4498b 100644
> --- a/src/mesa/drivers/dri/i965/test_vec4_register_coalesce.cpp
> +++ b/src/mesa/drivers/dri/i965/test_vec4_register_coalesce.cpp
> @@ -60,10 +60,9 @@ protected:
> return NULL;
> }
>
> - virtual int setup_attributes(int payload_reg)
> + virtual void setup_payload()
> {
> assert(!"Not reached");
> - return 0;
> }
>
> virtual void emit_prolog()
> --
> 1.8.3.4
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
