WineD3D patches for review and discussion - not to be applied as-is.
All of these apply to the latest Git tree, and should come after my last patch submission to wine-patches, but it shouldn't matter. I'm just looking for a little more feedback on these before I submit. This should lay the groundwork to begin implementing GLSL opcode generation. I've hacked around with some of them, and this will succesfully generate a GLSL shader program if the opcodes are implemented properly. Please let me know if any of my ideas here are fundamentally flawed. We've worked a bunch of it out on IRC, but this should be a nicer version to look at than my stuff at pastebin. :-) None of this code changes anything for the end-result ARB_vertex/fragment_program. It only generates GLSL if you have this regkey: HKLU\Software\Wine\Direct3D\UseGLSL = "enabled"
On 26/05/06, Jason Green <jave27(a)gmail.com> wrote: /* baseshader.c */
+ /* Declare address variables */ + for (i = 0; i < This->baseShader.limits.address; i++) { + if (reg_maps->address & (1 << i)) + shader_addline(buffer, "ivec4 A%ld;\n", i); + } + + /* Declare temporary variables */ + for(i = 0; i < This->baseShader.limits.temporary; i++) { + if (reg_maps->temporary & (1 << i)) + shader_addline(buffer, "vec4 R%lu;\n", i); + } Not so much about this patch, but should probably be mentioned nevertheless: The register maps are a 32-bit int, but depending on which shader model version you're targetting, the value for the limits can be much higher than that. I think using a byte array would be better.
/* vertexshader.c / pixelshader.c */
+ /* Declare all named attributes (TODO: Add this to the reg_maps and only declare those that are needed)*/ + for (i = 0; i < 16; ++i) + shader_addline(&buffer, "attribute vec4 attrib%i;\n", i); Is there a reason this can't be done from inside generate_base_shader()?
+ GL_EXTCALL(glGetObjectParameterivARB(shader_obj, + GL_OBJECT_COMPILE_STATUS_ARB, &success_flag)); + if (success_flag == FALSE) /* Print any compile errors in our shader */ + print_glsl_info_log(&GLINFO_LOCATION, shader_obj); I don't think this shouldn't be needed. Afaik calling print_glsl_info_log() should do nothing if linking is succesfull.
/* */
+ /* If we're using GLSL, we need to possibly create a Shader Program at this point, + * unless one is already set, then we need to attach the vertex shader object to it */ + if (NULL != pShader && wined3d_settings.shader_mode == SHADER_GLSL + && oldShader != pShader) { We should take into account the case where a shader is currently set, and SetVertexShader is called with NULL. The shader should be detached in that case. The structure of the code should probably look something like this:
if (old_shader == new_shader) return; program_id = get_program(); if (old_shader) detach(old_shader_id); if (new_shader) attach(new_shader_id); link(program_id);
+ GLhandleARB programId = This->updateStateBlock->shaderPrgId; + GLhandleARB shaderObj = ((IWineD3DVertexShaderImpl*)pShader)->baseShader.prgId; + int success_flag = 0; + + if (programId == 0) { + /* No shader program is set, create a new program */ + programId = GL_EXTCALL(glCreateProgramObjectARB()); + TRACE_(d3d_shader)("Creating new shader program %u\n", programId); + + /* Store this programId on the stateblock */ + This->updateStateBlock->shaderPrgId = programId; Getting/Creating the program id could probably be a function of it's own. You need it in when setting the pixel / vertex shader, in both the device and the stateblock's Capture / Apply methods.
+ GL_EXTCALL(glGetObjectParameterivARB(programId, + GL_OBJECT_LINK_STATUS_ARB, &success_flag)); + if (!success_flag) { + /* Print any linking errors in our shader */ + print_glsl_info_log(&GLINFO_LOCATION, programId); See above.
+ /* Bind vertex attributes to a corresponding index number */ + int i; + char tmp_name[10]; + for (i = 0; i < 16; ++i) { + sprintf(tmp_name, "attrib%i", i); + glBindAttribLocationARB(programId, i, tmp_name); + } Similar to the comment for vertexshader.c / pixelshader.c, can't this be in the same place we set all the other program attributes?
/* stateblock.c */ When we Capture / Apply the stateblock, we can't just copy the shader id's over. We need to do the whole detach/attach/link thing again, creating a new program object if needed. Somewhat related to that, I think most of the Set functions on the device should just call a function in the stateblock, rather than duplicating things between the device and the stateblock.
participants (2)
-
H. Verbeet -
Jason Green