2016-08-19 21:41 GMT+02:00 Ruslan Kabatsayev <b7.10110111(a)gmail.com>om>: I have a few pretty generic suggestions. You don't need to include configure changes in the patch, they will be regenerated by Alexandre anyway since those depend on the version of the autotools. Did you look into adding those tests to dxdiag instead of as a separate program? I don't think a separate program is necessarily bad but if integrating into dxdiag is reasonable it's probably better to do that from the start. Avoid camelCase or similar mixed-case identifier styles, just use lowercase_with_underscores. For pointer declarations you want to stick the '*' to the variable, not the type. Also please avoid LPTYPES, just use explicit pointer types. C++ comments (// comment) aren't allowed in Wine sources. Please replace them with normal C comments (/* comment */), or drop them entirely if they don't add anything that isn't clear from just looking at the code. It would be better to use explicit float constants when assigning to float variables (e.g. D3DMATERIAL). There aren't many other general style rules in Wine (it's mostly about using the style already used in the component so for new components you're mostly free to pick your own) as long as there is consistency. I see you mixing space after ',' with no space, pick one (preferably the space variant IMO) and stick to it. We usually use parentheses with the "sizeof" operator in Wine sources, so while it's not strictly required it would probably be nice to follow the general trend. More specific comments: I'd split this over multiple patches. You can probably make an initial patch adding the program with no real functionality, then multiple patches adding the various graphical API demos. In that regard, the d3d8 / d3d9 "merged" handling via macros seems a bit obfuscating to me. Just replicating the code twice seems cleaner. Maybe you can create a few helper functions for the common code. BTW, you can probably make things a bit more structured by moving the main loop to common code and have it call API backend functions. You'd have for each backend a structure like: static const struct render_backend d3d9_backend { d3d9_list_modes, d3d9_init, d3d9_draw, d3d9_wndproc, d3d9_destroy, }; (just an idea WRT the actual functions, I haven't really looked in detail) and you'd pass the structure from the selected API to the "main loop" function. I don't think you're using GLU anymore (good) so you should drop the glu32 import. I haven't looked in detail at the actual demos / renderers but I think there is enough stuff to work on already :)