[PATCH 1/4] dx8vb: Add stubs for D3DX8 interface

Fabian Maurer dark.shadow4 at web.de
Tue Oct 31 16:30:16 CDT 2017


Hi Stefan,

> 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.

Thanks for the explanation, I'm fine with that. I'll rework this.

> (It certainly isn't helpful that
> https://wiki.winehq.org/Wine_Developer%27s_Guide/COM_in_Wine#Implementing_a_
> COM_interface. contains loads of old suggestions)
> 
> >>> +static HRESULT WINAPI d3dx8_QueryInterface(ID3DX8 *iface, REFIID riid,
> >>> void **ppv) +{
> >>> +    d3dx8 *This = impl_from_ID3DX8(iface);
> >> 
> >> Please use lowercase variable names.
> > 
> > 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.
> 
> Similarly I can only refer to ddraw/d3d8/9/10/11 here, but other places
> in Wine certainly differ.

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?

> > 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.
> 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.

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.

> >>> +static HRESULT WINAPI d3dx8_CreateFont(ID3DX8 *iface, Direct3DDevice8
> >>> *device, +          LONG hFont, D3DXFont **retFont)
> >> 
> >> Please use lowercase parameter names, and avoid the stray 'h'.
> > 
> > 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.
> 
> 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.

That's good to know, because I don't like the formatting either. I'll rewrite 
that into proper names then.

> 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.
> 
> > 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.
> 
> 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)

What about enums/LONG/int, is this all "%d"?

Thanks for the detailed explanations.

Regards,
Fabian Maurer





More information about the wine-devel mailing list