Add a simple 3D APIs demo

[Ruslan Kabatsayev]

On Mon, Aug 22, 2016 at 12:44 PM, Matteo Bruni <matteo.mystral at gmail.com> wrote:
>
> 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.
I was thinking of this, so looked at some other patches which changed
configure.ac, which also appeared to alter configure script, so did
this. Will remove that change, it's cleaner this way indeed.
>
> 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.
Yes, I've even made a dialog for dxdiag with the System tab, but when
I began working on the Display tab, it appeared to require some
functionality which Wine's dxdiag.dll doesn't even support — providing
information about video card, its memory etc. I decided it'd be ugly
to just show several buttons to launch demos with no other
information, so did the demos a separate program. In the future it'll
be not that hard to integrate the demo as just a button triggering
CreateProcess(). The demos do return specific error codes on each
error, so the dxdiag UI would be able to record more or less explicit
log of why demo failed.
Also, I think it'd be better to make the demos runnable even if for
some strange reason dxdiag framework were broken, so this was another
reason why I decided to make it a separate program.
>
> 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.
Oh yeah, I was trying to avoid C99 comments, but as GCC doesn't seem
to have a switch to warn about using them, some did slip through.
> 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.
D3D code for 8 and 9 being merged was an attempt to avoid forgetting
to fix one while fixing another. D3D 8&9 are too similar to need
separate files. But OK, I'll split it then.
> 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.
Yeah, just forgot about the import.