Re: [PATCH] Moved wined3dformat <-> dxgi_format conversion functions into wined3d
On 10 September 2016 at 00:18, Fabian Maurer <dark.shadow4(a)web.de> wrote:
This change is a measure of code-deduplication and to make these functions reusable in other parts.
I'm not sure how valuable that really is, but wined3d shouldn't depend on dxgi or its headers. I may be open to having these as inline helpers somewhere under include/wine/, but right now the two copies of this don't strike me as a big problem.
On Saturday, September 10, 2016 6:20:16 PM CEST Henri Verbeet wrote:
I'm not sure how valuable that really is, but wined3d shouldn't depend on dxgi or its headers. I may be open to having these as inline helpers somewhere under include/wine/, but right now the two copies of this don't strike me as a big problem.
Well I'm currently implementing "D3DX11CreateShaderResourceViewFromFileW", and to make things easier I wanted to make a generic textureloader for both DX11 and DX9 in wined3d (mostly moving code from DX9). That works fine, but since it works with WINED3D formats, they need to be converted in d3dx11. But I'd need to duplicate the conversion code a third time for that. And that's what I wanted to avoid, it seems like a lot of unnecessary duplication.
On 10 September 2016 at 18:28, Fabian Maurer <dark.shadow4(a)web.de> wrote:
Well I'm currently implementing "D3DX11CreateShaderResourceViewFromFileW", and to make things easier I wanted to make a generic textureloader for both DX11 and DX9 in wined3d (mostly moving code from DX9). That works fine, but since it works with WINED3D formats, they need to be converted in d3dx11. But I'd need to duplicate the conversion code a third time for that. And that's what I wanted to avoid, it seems like a lot of unnecessary duplication. Unfortunately d3dx dlls shouldn't depend on wined3d either. We've typically handled sharing across different versions of d3dx using PARENTSRC.
On Saturday, September 10, 2016 6:33:04 PM CEST you wrote:
Unfortunately d3dx dlls shouldn't depend on wined3d either. We've typically handled sharing across different versions of d3dx using PARENTSRC.
I see, what's the reason behind this? After all, d3dx_36 depends on d3d9 which in turn depends on wined3d. Just curious, I'll make it different if this is your standard. I can put the conversion functions in dxgi and make it public by using the .spec file, but I really don't know about the actual texture loader. I mean, I could make a copy from all the d3dx9 code for d3dx11, but it seems more logical to not just duplicate code. But where would I put the code to use it in both d3dx9 and d3dx11? A new folder d3dx_shared for PARENTSRC?
On 09/10/2016 06:47 PM, Fabian Maurer wrote:
On Saturday, September 10, 2016 6:33:04 PM CEST you wrote:
Unfortunately d3dx dlls shouldn't depend on wined3d either. We've typically handled sharing across different versions of d3dx using PARENTSRC.
I see, what's the reason behind this? After all, d3dx_36 depends on d3d9 which in turn depends on wined3d. Just curious, I'll make it different if this is your standard. Not really, you can use d3dx_36 on Windows thus use that d3d9 without depending on wined3d.
bye michael
On 10 September 2016 at 18:47, Fabian Maurer <dark.shadow4(a)web.de> wrote:
On Saturday, September 10, 2016 6:33:04 PM CEST you wrote:
Unfortunately d3dx dlls shouldn't depend on wined3d either. We've typically handled sharing across different versions of d3dx using PARENTSRC.
I see, what's the reason behind this? After all, d3dx_36 depends on d3d9 which in turn depends on wined3d. Just curious, I'll make it different if this is your standard.
In the first place it's conceptual, wined3d is Wine's equivalent to the D3D driver interface, it shouldn't be concerned with high-level utility functionality like D3DX. Second, and related, as Michael mentioned, Microsoft's d3dx9 doesn't depend directly on the D3D driver interface. You can use Microsoft's d3dx9 on Wine, and ideally you should be able to use a Win32 build of Wine's d3dx9 as a drop-in replacement on Windows, without additional dependencies.
I can put the conversion functions in dxgi and make it public by using the .spec file, but I really don't know about the actual texture loader. DXGI isn't really the right place either, for similar reasons. In general, we'd like to limit the number of Wine-specific exports.
I mean, I could make a copy from all the d3dx9 code for d3dx11, but it seems more logical to not just duplicate code. But where would I put the code to use it in both d3dx9 and d3dx11? A new folder d3dx_shared for PARENTSRC?
It's largely up to Matteo and Alexandre, but I think there are a few options. We could for example use a single PARENTSRC for d3dx9/10/11. That would still require some thought, since while I'd expect many things to be similar or the same between d3dx9/10/11, there are also a considerable number of differences. Another option could be a static library like libwpp or libwine_port, although that's not a popular option. Maybe it's warranted for d3dx. Maybe there are other options as well.
In the first place it's conceptual, wined3d is Wine's equivalent to the D3D driver interface, it shouldn't be concerned with high-level utility functionality like D3DX. Second, and related, as Michael mentioned, Microsoft's d3dx9 doesn't depend directly on the D3D driver interface. You can use Microsoft's d3dx9 on Wine, and ideally you should be able to use a Win32 build of Wine's d3dx9 as a drop-in replacement on Windows, without additional dependencies.
Ah well I see, makes sense.
It's largely up to Matteo and Alexandre, but I think there are a few options. We could for example use a single PARENTSRC for d3dx9/10/11. That would still require some thought, since while I'd expect many things to be similar or the same between d3dx9/10/11, there are also a considerable number of differences. Another option could be a static library like libwpp or libwine_port, although that's not a popular option. Maybe it's warranted for d3dx. Maybe there are other options as well.
Well, there are differences between d3dx9 and d3dx11, but it's possible to deduplicate at least a part of the code. I'm trying to get it done with texture loading, but I'd need to know where to put it. At least parsing the DDS or using WIC to load images, and the converter functions are nearly exactly the same. The only other option I see would be to not care about extracting these code parts and making a full blown copy in d3dx11 like I have it on my temporary branch. It works, but to me it seems like a bad approach.
2016-09-12 10:37 GMT+02:00 Henri Verbeet <hverbeet(a)gmail.com>:
On 10 September 2016 at 18:47, Fabian Maurer <dark.shadow4(a)web.de> wrote:
I mean, I could make a copy from all the d3dx9 code for d3dx11, but it seems more logical to not just duplicate code. But where would I put the code to use it in both d3dx9 and d3dx11? A new folder d3dx_shared for PARENTSRC?
It's largely up to Matteo and Alexandre, but I think there are a few options. We could for example use a single PARENTSRC for d3dx9/10/11. That would still require some thought, since while I'd expect many things to be similar or the same between d3dx9/10/11, there are also a considerable number of differences. Another option could be a static library like libwpp or libwine_port, although that's not a popular option. Maybe it's warranted for d3dx. Maybe there are other options as well.
I don't quite know what would be the best solution yet. Maybe a static library for the code in common between d3dx9/10/11 + PARENTSRC for the various _xx versions would be sensible, yes. It's going to be quite a bit of work to make the common, d3dx version-agnostic helper functions to put in the static lib though (including figuring out what they should look like). On the upside, that can be done incrementally, moving stuff there piece by piece as needed. The single d3dx* PARENTSRC idea seems a bit more messy, while the hypothetical multiple PARENTSRC route I mentioned for d3dcompiler doesn't seem a good fit here. I'm afraid this is one of those cases where it's going to be clear if a certain route is practical only after the fact i.e. once someone writes the patches or fails trying... Fabian, if you want to send patches for D3DX11CreateShaderResourceViewFromFileW "duplicating" the relevant code for the time being, that is fine to me. It might even be easier to decide how to share the code at that point.
On 09/12/2016 10:50 PM, Matteo Bruni wrote:
2016-09-12 10:37 GMT+02:00 Henri Verbeet <hverbeet(a)gmail.com>:
On 10 September 2016 at 18:47, Fabian Maurer <dark.shadow4(a)web.de> wrote:
I mean, I could make a copy from all the d3dx9 code for d3dx11, but it seems more logical to not just duplicate code. But where would I put the code to use it in both d3dx9 and d3dx11? A new folder d3dx_shared for PARENTSRC?
It's largely up to Matteo and Alexandre, but I think there are a few options. We could for example use a single PARENTSRC for d3dx9/10/11. That would still require some thought, since while I'd expect many things to be similar or the same between d3dx9/10/11, there are also a considerable number of differences. Another option could be a static library like libwpp or libwine_port, although that's not a popular option. Maybe it's warranted for d3dx. Maybe there are other options as well.
I don't quite know what would be the best solution yet. Maybe a static library for the code in common between d3dx9/10/11 + PARENTSRC for the various _xx versions would be sensible, yes. It's going to be quite a bit of work to make the common, d3dx version-agnostic helper functions to put in the static lib though (including figuring out what they should look like). On the upside, that can be done incrementally, moving stuff there piece by piece as needed. The static library was also my goal for the dmusic and related DLLs. Alexandre wasn't convinced so the incremental way to get there was to duplicate the dmobject.[ch] files. "Duplicate" as there are 8 copies, I just need to resurrect my dmusic cleanup to make those files even bigger and add more churn to them ;)
bye michael
Fabian, if you want to send patches for D3DX11CreateShaderResourceViewFromFileW "duplicating" the relevant code for the time being, that is fine to me. It might even be easier to decide how to share the code at that point.
Thanks, I sent the patches to the patches mailing list, I hope that's fine. I split them a bit so it's more visible which part I changed, it's really a lot copy&paste. You probably need to apply all patches at once, since they don't make much sense each on their own, but it shouldn't break compilation even if you just apply one. Also, if you want something to test it with, I implemented it for the samples from http://www.rastertek.com/dx11tut05.html. Currently still needs native d3dcompiler to work though. I'd be happy to hear some feedback, especially since it's a fairly big set of patches.
participants (4)
-
Fabian Maurer -
Henri Verbeet -
Matteo Bruni -
Michael Stefaniuc