2009/4/23 Tobias Jakobi Liquid.Acid@gmx.net:
ps_np2fixup_t is still stored in ps_compiled_shader and I have no intention to change this, since the ARB code is going to use the exact same structure.
NAK
NAK
Suggestions? Explanations?
Like explained putting this into prog_link won't work. It doesn't make sense in the first place. Using a separate struct for GLSL and ARB isn't also making sense, since the data stored in the struct is independant of GLSL and ARB.
Am Donnerstag, 23. April 2009 14:54:54 schrieb Tobias Jakobi:
NAK
Suggestions? Explanations?
Like explained putting this into prog_link won't work. It doesn't make sense in the first place. Using a separate struct for GLSL and ARB isn't also making sense, since the data stored in the struct is independant of GLSL and ARB.
I think the ps_compiled_shader should look like this:
struct ps_compiled_shader { struct ps_compile_args args; void *backend_private_data; };
That stores per-gl-shader backend private data, and then ARB and GLSL can declare their private structures(which will be similar but not exactly the same, like GLuint prgId in ARB and GLHandle in GLSL).
That's not really a fault in Tobias' patchset though. It is something I did wrong when I introduced the ps_compiled_shader structure, and that brokeness was something that already existed when both GLSL and ARB shared a GLuint prgId; in the pixel shader impl structure. I can see if I can fix this over the weekend(shouldn't be to hard or take too much time)
Stefan Dösinger wrote:
Am Donnerstag, 23. April 2009 14:54:54 schrieb Tobias Jakobi:
NAK
Suggestions? Explanations?
Like explained putting this into prog_link won't work. It doesn't make sense in the first place. Using a separate struct for GLSL and ARB isn't also making sense, since the data stored in the struct is independant of GLSL and ARB.
I think the ps_compiled_shader should look like this:
struct ps_compiled_shader { struct ps_compile_args args; void *backend_private_data; };
That stores per-gl-shader backend private data, and then ARB and GLSL can declare their private structures(which will be similar but not exactly the same, like GLuint prgId in ARB and GLHandle in GLSL).
That's not really a fault in Tobias' patchset though. It is something I did wrong when I introduced the ps_compiled_shader structure, and that brokeness was something that already existed when both GLSL and ARB shared a GLuint prgId; in the pixel shader impl structure. I can see if I can fix this over the weekend(shouldn't be to hard or take too much time)
Hi Stefan,
I think Henri is generally against adding something to the ps_compiled_shader structure - but I might be wrong there.
However I don't know how to implement the code without introducing new data to ps_compiled_shader.
Greets, Tobias
I think Henri is generally against adding something to the ps_compiled_shader structure - but I might be wrong there.
However I don't know how to implement the code without introducing new data to ps_compiled_shader.
No, he doesn't like having this info visible outside the backend. You can certainly introduce new per-compiled-gl shader information, but it should not be 'visible' outside the respective shader backend. So in an ideal world your patchset would not touch wined3d_private.h, only glsl_shader.c, and later arb_program_shader.c, since all the NP2 fixup interfaces the backend needs are already in place.
In the past we've done a considerable amount of work to separate the shader backend implementations from the rest of the code. E.g. the ps_compile_args structure is part of that(avoids direct stateblock accesses), Henri moved parsing the D3D bytecode into the frontend(the backends operate on bytecode-independent structures), etc. The goals are an abstraction from the bytecode to enable d3d10 support, the stateblock separation allowed shader duplication, etc. However, the work is not entirely done yet,
I can implement the modification of ps_compiled_shader so you can add your data structures properly. An example of what it should more or less look like is the fragment pipeline replacement - e.g. the ATIFS specific structures are all defined in ati_fragment_shader.c, and all the frontend offers is a void * to store a pointer in the device. The question is obviously when I'll get to it, as I still have to play around with that java-jboss-whatever stuff for university.
Stefan Dösinger wrote:
I think Henri is generally against adding something to the ps_compiled_shader structure - but I might be wrong there.
However I don't know how to implement the code without introducing new data to ps_compiled_shader.
No, he doesn't like having this info visible outside the backend. You can certainly introduce new per-compiled-gl shader information, but it should not be 'visible' outside the respective shader backend. So in an ideal world your patchset would not touch wined3d_private.h, only glsl_shader.c, and later arb_program_shader.c, since all the NP2 fixup interfaces the backend needs are already in place.
OK, I totally misunderstood him then. Sry Henri!!
In the past we've done a considerable amount of work to separate the shader backend implementations from the rest of the code. E.g. the ps_compile_args structure is part of that(avoids direct stateblock accesses), Henri moved parsing the D3D bytecode into the frontend(the backends operate on bytecode-independent structures), etc. The goals are an abstraction from the bytecode to enable d3d10 support, the stateblock separation allowed shader duplication, etc. However, the work is not entirely done yet,
I can implement the modification of ps_compiled_shader so you can add your data structures properly. An example of what it should more or less look like is the fragment pipeline replacement - e.g. the ATIFS specific structures are all defined in ati_fragment_shader.c, and all the frontend offers is a void * to store a pointer in the device. The question is obviously when I'll get to it, as I still have to play around with that java-jboss-whatever stuff for university.
OK, I'll wait then. Thanks again Stefan for explaining this!
Greets, Tobias
-
Henri Verbeet -
Stefan Dösinger -
Tobias Jakobi