<div dir="ltr"><div><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote">First, the 1/4 patch is really something you'll have to convince<br>
Alexandre of it's the right thing to do. The easiest way is probably<br>
to ask him about it on IRC. The amount of advice I can give on that<br>
one is limited, but it would probably help if you could show why Wine<br>
needs the winapifamily mechanism. The best way to do that may very<br>
well just be to show that applications need e.g. the<br>
WINAPI_FAMILY_DESKTOP_APP constant to compile against the Wine<br>
headers.<br></blockquote>I will ask him on IRC.<br></div><div>Yes well it would be more correct to have the header in the case where a program uses that ifdef<br></div><div>So this would match the official instructions from MS on using different api's<br><br><br><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote">I could also imagine that you'll be asked to partition the<br>
other Wine headers as well if we're going to have this mechanism in<br>
Wine. I.e., I don't think it makes a lot of sense to only do this for<br>
a couple of dxgi headers, but not for anything else. The prefix for<br>
the subject should be "include:". I'd move this patch to the end of<br>
the series. <br></blockquote></div><div>Initially I would have only done the dx stuff because mingw-w64 shares those headers. <br></div><div>Essentially they Jacek Caban pulls in the d3d11 headers and idls from wine in to mingw-w64 to update.<br></div><div><br></div><div>I could update the mingw-w64 project with the changes but I didn't want to leave wine out of the loop.<br>I do understand that all headers would need to be partitioned to support this eventually.<br></div><div>I was hoping the start would be with directx<br></div><div> </div><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote">The comment is pretty superfluous.<br></blockquote>I can take it out.<br><div><br><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote">In patch 2/4, you have unrelated style changes for e.g.<br>
IDXGIDevice2::OfferResources()<div id=":1az" class="">. I happen to like the new style better,<br>
but please don't do that. First add the new stuff, then send a patch<br>
to make the existing stuff consistent with dxgi.idl at the end of the<br>
series.</div></blockquote><div>Sure I can do that <br></div><br><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote">Some general style stuff:<br>
- I think "{" on the same line for enum/struct/interface declarations is ugly.<br>
- Use "DXGI_OUTDUPL_DESC *desc" instead of "DXGI_OUTDUPL_DESC* desc", etc.<br>
- There are a couple of parameter naming issues. e.g. "pDevice",<br>
"colorSpace", "pStats", "Data", etc.<br>
- I prefer the order of things to be macros/constants, enums,<br>
structs, interfaces, functions instead of having those interleaved<br>
through the header.<br>
- "FLOAT" -> "float", "LPCWSTR" -> "const WCHAR *"<br>
- "num_*" -> "*_count"<br>
<br>
The prefix for the dxgi IDL patches should be "dxgi:", for the d3d11<br>
one it should be "d3d11:".<br></blockquote><br></div><div>Seems easy enough to follow :)<br><br><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 6, 2015 at 1:05 PM, Henri Verbeet <span dir="ltr"><<a href="mailto:hverbeet@gmail.com" target="_blank">hverbeet@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I had a look at this series, and have a couple of comments.<br>
<br>
First, the 1/4 patch is really something you'll have to convince<br>
Alexandre of it's the right thing to do. The easiest way is probably<br>
to ask him about it on IRC. The amount of advice I can give on that<br>
one is limited, but it would probably help if you could show why Wine<br>
needs the winapifamily mechanism. The best way to do that may very<br>
well just be to show that applications need e.g. the<br>
WINAPI_FAMILY_DESKTOP_APP constant to compile against the Wine<br>
headers. I could also imagine that you'll be asked to partition the<br>
other Wine headers as well if we're going to have this mechanism in<br>
Wine. I.e., I don't think it makes a lot of sense to only do this for<br>
a couple of dxgi headers, but not for anything else. The prefix for<br>
the subject should be "include:". I'd move this patch to the end of<br>
the series.<br>
<br>
> +/* wine wants all the api's by default so we set to desktop + app */<br>
> +#ifndef WINAPI_FAMILY<br>
> +#define WINAPI_FAMILY WINAPI_FAMILY_DESKTOP_APP<br>
> +#endif<br>
The comment is pretty superfluous.<br>
<br>
In patch 2/4, you have unrelated style changes for e.g.<br>
IDXGIDevice2::OfferResources(). I happen to like the new style better,<br>
but please don't do that. First add the new stuff, then send a patch<br>
to make the existing stuff consistent with dxgi.idl at the end of the<br>
series.<br>
<br>
Some general style stuff:<br>
- I think "{" on the same line for enum/struct/interface declarations is ugly.<br>
- Use "DXGI_OUTDUPL_DESC *desc" instead of "DXGI_OUTDUPL_DESC* desc", etc.<br>
- There are a couple of parameter naming issues. e.g. "pDevice",<br>
"colorSpace", "pStats", "Data", etc.<br>
- I prefer the order of things to be macros/constants, enums,<br>
structs, interfaces, functions instead of having those interleaved<br>
through the header.<br>
- "FLOAT" -> "float", "LPCWSTR" -> "const WCHAR *"<br>
- "num_*" -> "*_count"<br>
<br>
The prefix for the dxgi IDL patches should be "dxgi:", for the d3d11<br>
one it should be "d3d11:".<br>
</blockquote></div><br></div>