Hi,
For a while we have been discussing on how to clean up / rewrite some parts of the surface code in WineD3D. I did submit a number of patches to initiate a rewrite of some parts but not all patches were correct and it wasn't clear what final design I had in mind. I'm posting my ideas to here to get some feedback on how to proceed. For people outside wined3d this might not be that interesting but I'll give a short overview of the problems, so that you could also understand it a little bit.
The main issue is that pieces of modern code and legacy DirectDraw code are entangled in a way which is not easy to untangle. There are various places which can be improved but for now I want to limit myself to 'BltOverride'. It is a big function shared between DirectDraw/D3D7 blitting and D3D8 CopyRects / D3D9 StretchRect and over the years it has become large and very ugly.
The way I thought about simplifying this call is to move some parts to helper functions and to remove legacy functionality (e.g. move colorfill back to Blt). A lot of the work which has to be done is not just about moving code around. Some functionality can these days be implemented in a nicer way thanks to functionality like shaders.
Below I have some pseudo code on how I think a chunk of BltOverride and the blit_shader calls it uses, should be implemented. The main idea is to move the majority of the code to the blit_shader. Unless color keying / 8-bit is required, I expect that no actual shader is needed. In case we need a shader, there are two cases namely when blit_shader is implemented using GPU shaders and the case we need a fall back. In case of the fall back, I think we need big chunks of the 'old' BltOverride code which needs to be moved over to ffp_blit.
BltOverride (color keyed) offscreen surface -> render target blitting { if(orm_fbo && can_stretch) stretch_rect_fbo <-- this check is around now; but I guess we ultimately want a IWineD3DDevice_StretchRect which contains this code and which can fall back to BltOverride) context = context_acquire(CTXUSAGE_BLIT) blit_shader->set_shader(src_surface) blit_shader->draw_quad(src_surface, src_rect, dst_rect) blit_shader->unset_shader(); }
arb_blit_shader based implementation { blit_shader->set_shader(src_surface) { generate_shader (if needed)
activate_shader if(color_keying_active) upload_colorkey_to_shader if(p8_active) load_p8_palette } blit_shader->draw(src_surface, src_rect, dst_rect) { surface_internal_preload(src_surface) { under the hood, LoadTexture/LoadLocation should not perform surface conversion for us basically the issue is how to distinguish between 2d and 3d color keying and how to separete this }
/* drawing */ if(fbo_blit) fbo_blit() else draw_quad() } blit_shader->unset_shader(); }
ffp_blit implementation { blit_shader->set_shader(src_surface) { if(p8_active) load_palette else not_much_todo } blit_shader->draw(src_surface, src_rect, dst_rect) { if(need_color_keying) play_with_color_keying_flags (like current BltOverride does) surface_internal_preload(src_surface); if(need_color_keying) restore_color_keying flags activate_alpha_test_magic
draw_quad() } blit_shader->unset_shader(); }
I think the design described above could work (perhaps I missed something, please correct me then). One of the issues is color keying. Right now color key conversion is performed in LoadLocation based on some flag magic. In case of a 2d blit using shaders this is not needed but for Direct3D purposes it is needed. I'm not sure how to handle this in a clear way. (I fear that we also get some new issues when a game alternates between 2d and 3d color keying).
I don't expect issues regarding 8-bit as long as we implement them in software and use opengl only during the upload (UnlockRect). If we would allow hardware blitting, it would need to be 8-bit -> 8-bit blits and no converted blits else we need hacks like paletteOverride which I would like to eliminate.
Roderick