On Fri, Oct 9, 2015 at 7:33 PM, Emil Velikov <emil.l.veli...@gmail.com> wrote:
> On 9 October 2015 at 17:55, Marek Olšák <mar...@gmail.com> wrote:
>> On Thu, Oct 8, 2015 at 3:49 PM, Emil Velikov <emil.l.veli...@gmail.com>
>> wrote:
>>> On 8 October 2015 at 01:12, Marek Olšák <mar...@gmail.com> wrote:
>>>> From: Marek Olšák <marek.ol...@amd.com>
>>>>
>>>> They didn't do anything useful.
>>>> ---
>>>> src/mesa/drivers/dri/i915/i915_fragprog.c | 7 +-
>>>> src/mesa/drivers/dri/i965/brw_program.c | 10 +-
>>>> .../drivers/dri/i965/test_fs_cmod_propagation.cpp | 2 +-
>>>> .../dri/i965/test_fs_saturate_propagation.cpp | 2 +-
>>>> .../dri/i965/test_vec4_copy_propagation.cpp | 2 +-
>>>> .../dri/i965/test_vec4_register_coalesce.cpp | 2 +-
>>>> src/mesa/drivers/dri/r200/r200_vertprog.c | 4 +-
>>>> src/mesa/program/program.c | 133
>>>> +++------------------
>>>> src/mesa/program/program.h | 29 +----
>>>> src/mesa/state_tracker/st_cb_program.c | 43 +++----
>>>> 10 files changed, 50 insertions(+), 184 deletions(-)
>>>>
>>>> diff --git a/src/mesa/drivers/dri/i915/i915_fragprog.c
>>>> b/src/mesa/drivers/dri/i915/i915_fragprog.c
>>>> index 1a5943c..237d219 100644
>>>> --- a/src/mesa/drivers/dri/i915/i915_fragprog.c
>>>> +++ b/src/mesa/drivers/dri/i915/i915_fragprog.c
>>>> @@ -1316,8 +1316,8 @@ i915NewProgram(struct gl_context * ctx, GLenum
>>>> target, GLuint id)
>>>> {
>>>> switch (target) {
>>>> case GL_VERTEX_PROGRAM_ARB:
>>>> - return _mesa_init_vertex_program(ctx,
>>>> CALLOC_STRUCT(gl_vertex_program),
>>>> - target, id);
>>>> + return _mesa_init_gl_program(CALLOC_STRUCT(gl_vertex_program),
>>>> + target, id);
>>>>
>>> Worth doing the memory allocation to a temporary variable, and feeding
>>> &prog->base to _mesa_init_gl_program ?
>>
>> I tried that first, but it required more lines of code.
>>
> The ones that you'll gain, will be balanced out by the other suggestions :-)
>
>>>
>>>> --- a/src/mesa/drivers/dri/r200/r200_vertprog.c
>>>> +++ b/src/mesa/drivers/dri/r200/r200_vertprog.c
>>>> @@ -1205,9 +1205,9 @@ r200NewProgram(struct gl_context *ctx, GLenum
>>>> target, GLuint id)
>>>> switch(target){
>>>> case GL_VERTEX_PROGRAM_ARB:
>>>> vp = CALLOC_STRUCT(r200_vertex_program);
>>>> - return _mesa_init_vertex_program(ctx, &vp->mesa_program, target,
>>>> id);
>>>> + return _mesa_init_gl_program(&vp->mesa_program, target, id);
>>>> case GL_FRAGMENT_PROGRAM_ARB:
>>>> - return _mesa_init_fragment_program( ctx,
>>>> CALLOC_STRUCT(gl_fragment_program), target, id );
>>>> + return _mesa_init_gl_program(CALLOC_STRUCT(gl_fragment_program),
>>>> target, id);
>>> Ditto.
>>>
>>>
>>>> --- a/src/mesa/program/program.c
>>>> +++ b/src/mesa/program/program.c
>>>
>>>> @@ -309,34 +217,29 @@ _mesa_new_program(struct gl_context *ctx, GLenum
>>>> target, GLuint id)
>>>> struct gl_program *prog;
>>>> switch (target) {
>>>> case GL_VERTEX_PROGRAM_ARB: /* == GL_VERTEX_PROGRAM_NV */
>>>> - prog = _mesa_init_vertex_program(ctx,
>>>> CALLOC_STRUCT(gl_vertex_program),
>>>> - target, id );
>>>> + prog = _mesa_init_gl_program(CALLOC_STRUCT(gl_vertex_program),
>>>> + target, id);
>>> Ditto. + early return similar to st/mesa ?
>>>
>>>> --- a/src/mesa/program/program.h
>>>> +++ b/src/mesa/program/program.h
>>>
>>>> -extern struct gl_program *
>>>> -_mesa_init_compute_program(struct gl_context *ctx,
>>>> - struct gl_compute_program *prog,
>>>> - GLenum target, GLuint id);
>>>> +_mesa_init_gl_program(void *prog, GLenum target, GLuint id);
>>>>
>>> If you go for my suggestion you can keep prog as struct gl_program *.
>>>
>>>> --- a/src/mesa/state_tracker/st_cb_program.c
>>>> +++ b/src/mesa/state_tracker/st_cb_program.c
>>>
>>>> + switch (target) {
>>>> + case GL_VERTEX_PROGRAM_ARB:
>>>> + prog = (struct gl_program*)ST_CALLOC_STRUCT(st_vertex_program);
>>>> + break;
>>>> + case GL_FRAGMENT_PROGRAM_ARB:
>>> Worth adding case GL_FRAGMENT_PROGRAM_NV, or perhaps nuking it from
>>> mesa/program/program.c above ?
>>
>> I think GL_FRAGMENT_PROGRAM_NV is from an NV extension supported by
>> some classic drivers but not st/mesa.
>>
> Seems to be part of GL_NV_fragment_program, which was removed a while ago with
I think it's GL_NV_fragment_program_option, so you can't just remove
it unless you want to remove the whole extension.
>
> commit 2f350f360b00e786e140d9ccb9db83bf23269d8f
> Author: Kenneth Graunke <kenn...@whitecape.org>
> Date: Sun Oct 14 15:53:06 2012 -0700
>
> mesa: Remove get and enable bits for NV_fragment_program.
>
> and a few other follow-up commits.
>
> Seems like we still have a few remnants which I will clean-up after
> this series lands.
>
>>> Can we keep the variables as is and avoid the casts ?
>>
>> ST_CALLOC_STRUCT needs a cast.
>> Or &ST_CALLOC_STRUCT(...)->Base.Base if I fix the macro to allow that.
>>
> I was thinking about retaining the original style/approach and not use
> CALLOC_STRUCT(foo) as a function argument. Declaring a function
> (_mesa_init_gl_program) argument as void * only to down and up cast
> it, depending on the scenario, seems a bit ugly. If you don't mind I
> can follow up and address these separately ?
Ok.
Marek
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
