Perhaps this is sufficiently far removed from core Direct3D that nobody cares, but for what it's worth:
On 31 October 2017 at 03:01, Fabian Maurer dark.shadow4@web.de wrote:
Please don't typedef structures just to save keystrokes.
Please use lowercase variable names.
Not implementing an arbitrary interface is a WARN at best.
I think Nikolay already hinted at this, but this is fairly different from the canonical QueryInterface() implementation, especially for an object that only implements a single interface.
Please use lowercase parameter names, and avoid the stray 'h'.
I certainly have my preferences when it comes to trace formatting, but I think "%i" is questionable for flags regardless of preference.
+static const ID3DX8Vtbl ID3DX8_Vtbl =
"d3dx8_vtbl"
"sizeof(*object)"
Hi Henry, thanks for the quick reply.
Mind explaining the reason for this? It's like this in a lot of other places, and I'm curious to why you prefer it to not be a typedef.
Again, it's like this in 95% of all other cases. I'll change it if you want, I just wanted it to remain consistent with the existing code.
Mind explaining why? The wiki states that "Messages in this class are meant to signal unimplemented features, known bugs, etc. They serve as a constant and active reminder of what needs to be done." This sounds fitting for me.
But they don't only implement one interface, they have those other GUIDs that are queried, no?
I mostly just took the official names, want me to edit all of them? Wouldn't be an issue for me, just need to know.
Maybe, I'm not good with trace formatting. Is there a list for which type I should use what? I'd really appreciate that, maybe in the wiki? I didn't find anything.
I too took this from already existing code, but I'll change it.
Regards, Fabian Maurer
Hi Fabian,
I'm not Henri, but I'll try to answer your questions:
Am 2017-10-31 um 20:48 schrieb Fabian Maurer:
Here I can mostly refer to the other d3d libraries (e.g. d3d8) that do not use typedefs for structs. We had a discussion about this on wine-devel a while ago that I could not find in the archive, but here's e.g. the kernel's guide on this: https://www.kernel.org/doc/html/v4.10/process/coding-style.html#typedefs .
I don't think we have an official policy, and as you correctly point out a lot of Wine code (and Microsoft's headers) use typedefs. But since Henri is the d3d maintainer his word counts here.
(It certainly isn't helpful that https://wiki.winehq.org/Wine_Developer%27s_Guide/COM_in_Wine#Implementing_a_.... contains loads of old suggestions)
Similarly I can only refer to ddraw/d3d8/9/10/11 here, but other places in Wine certainly differ.
It's largely because some applications query various (sometimes even app-internal) interfaces and expect a failure, e.g. as a way to detect what kind of object they got out of their internal data structures. Printing a FIXME in QueryInterface is just as likely to mislead a user or developer into thinking the interface should be supported as it is to point out a bug.
If you know an interface should be supported, e.g. through tests or because the documentation says so, but you don't implement it, that would be a case for a FIXME.
Yeah, we try to keep a distance from Microsoft's formatting as far as possible without breaking compatibility. Renaming a parameter keeps the code compatible to Microsoft's headers, but e.g. renaming a struct member would break things.
This is mostly an issue for writing our own headers because it is technically possible (but not allowed) to copypaste them from Microsoft's headers.
Flags are ORed bits. so %#x is the canonical way to print them. You'll get output like 0x105 and see that there are flags 0x100, 0x004 and 0x001 set. With %i you get "261", which is much harder to read (256 | 4 | 1). (the # prints "0x105" instead of "105", which makes it clear that it is a hexadecimal number)
Hi Stefan,
Thanks for the explanation, I'm fine with that. I'll rework this.
No problem, I'll change that. It's not a problem to use words that are reserved words in C++ since we're using only plain C, right?
Ah right, I just assume it's required since it's queried, but it makes sense what you say. I'll turn it in to the regular warning in these cases then.
That's good to know, because I don't like the formatting either. I'll rewrite that into proper names then.
What about enums/LONG/int, is this all "%d"?
Thanks for the detailed explanations.
Regards, Fabian Maurer
2017-10-31 22:30 GMT+01:00 Fabian Maurer dark.shadow4@web.de:
It's not a problem but in d3d-related code we tend to avoid using "this" as the name for object variables (e.g. you would name it "d3dx8" here, or something like that).
In general, please give a look at recent wined3d code / commits to have an idea WRT the code style we prefer to use in d3d.
Am 2017-10-31 um 22:30 schrieb Fabian Maurer:
What about enums/LONG/int, is this all "%d"?
My gut feeling says yes, although I am not entirely sure myself.
On 1 November 2017 at 11:40, Stefan Dösinger stefandoesinger@gmail.com wrote:
It depends on the usage. For enums I'd say %#x by default, but for some a debug helper function (%s), %u or %d could make sense. For e.g. int, %d may make sense for a display coordinate, but %#x may be more appropriate for a file or memory offset.