2016-08-26 21:12 GMT+02:00 Ruslan Kabatsayev b7.10110111@gmail.com:
Hello, I've addressed most of the concerns, but would like to get some clarifications before I continue with the rest.
On Mon, Aug 22, 2016 at 12:44 PM, Matteo Bruni matteo.mystral@gmail.com wrote:
It would be better to use explicit float constants when assigning to float variables (e.g. D3DMATERIAL).
Do you mean that numeric constants are to never be implicitly converted? Or only integral->floating-point? Or maybe only double->float?
It's not a "NEVER!!11!"-level rule, but if the conversion can be avoided by writing the constant properly e.g. simply adding the appropriate suffix, then it's good practice to do that. In that specific case, you should write 1.0f (or whatever the actual value is) explicitly.
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.
I've tried looking into splitting D3D code, but from the experience of fixing some issues like CamelCase->snake_case and similar it seems the split would be counter-productive for future changes. What I mean: the D3D 8 and 9 (as used in this code) only differ by interface version suffix and added/removed parameters in some methods (the parameters removed are zero here anyway, since the demos ideally should give identical results with different APIs), plus a few names changed. It seems to me that any change in functionality would require one to edit both D3D8 and D3D9 code in a _very_ similar way, and the differences between the split codes wouldn't be easy to understand: diff would give lots of unrelated differences, while substantial ones could be missed (resulting in subtly non-identical results). While this is inevitable between D3D and WGL, it's easily avoidable for such similar (in such basic use) APIs as D3D8 and D3D9. Do you still think the code should really be split?
Yes. I think looking at the d3d8 and d3d9 code side-by-side should be quite fine to check for unintended differences. At least I'd prefer that to the macros. Also once you want to do something different between the two APIs with the macros solution you'll end up sprinkling #ifdefs around and that can get ugly fast.
As I mentioned in the previous email, it doesn't mean you can't or shouldn't share code between the two, just that it would be better IMO to share the code in a different fashion. You can probably use a single shared main loop for all the APIs, create helpers for the common tasks (setting up the matrices, wndproc, ...) etc. There shouldn't be a lot of code actually ending up "replicated".
That's just my opinion, of course. I'll listen to counterarguments :)