Re: [PATCH] windowscodecs: Add support for DIB format.
Christian Costa <titan.costa(a)gmail.com> wrote:
cpp_quote("DEFINE_GUID(CLSID_WICBmpDecoder, 0x6b462062,0x7cbf,0x400d,0x9f,0xdb,0x81,0x3d,0xd1,0x0f,0x27,0x78);") +cpp_quote("DEFINE_GUID(CLSID_WICDibDecoder, 0x6b462062,0x7cbf,0x400d,0x9f,0xdb,0x81,0x3d,0xd1,0x0f,0x27,0x79);") cpp_quote("DEFINE_GUID(CLSID_WICPngDecoder, 0x389ea17b,0x5078,0x4cde,0xb6,0xef,0x25,0xc1,0x51,0x75,0xc7,0x51);") cpp_quote("DEFINE_GUID(CLSID_WICIcoDecoder, 0xc61bfcdf,0x2e0f,0x4aad,0xa8,0xd7,0xe0,0x6b,0xaf,0xeb,0xcd,0xfe);") cpp_quote("DEFINE_GUID(CLSID_WICJpegDecoder, 0x9456a480,0xe88b,0x43ea,0x9e,0x73,0x0b,0x2d,0x9b,0x71,0xb1,0xca);") @@ -1002,6 +1003,7 @@ cpp_quote("DEFINE_GUID(CLSID_WICWmpEncoder, 0xac4ce3cb,0xe1c1,0x44cd,0x82,0x15,0 cpp_quote("DEFINE_GUID(CLSID_WICDefaultFormatConverter, 0x1a3f11dc,0xb514,0x4b17,0x8c,0x5f,0x21,0x54,0x51,0x38,0x52,0xf1);")
cpp_quote("DEFINE_GUID(GUID_ContainerFormatBmp, 0x0af1d87e,0xfcfe,0x4188,0xbd,0xeb,0xa7,0x90,0x64,0x71,0xcb,0xe3);") +cpp_quote("DEFINE_GUID(GUID_ContainerFormatDib, 0x0af1d87e,0xfcfe,0x4188,0xbd,0xeb,0xa7,0x90,0x64,0x71,0xcb,0xe4);")
Is this mocked up names? I couldn't find WIC DIB decoder definitions in the PSDK headers. -- Dmitry.
Oupsss. I forgot to explain. No it's not it native windowscodecs. It's a wine extension to support DIB files used in d3dx9_36. Everything was already in place. I just had to hook things up. I asked to Vincent who told to do this way. At least it's what I understood. Christian 2013/1/21 Dmitry Timoshkov <dmitry(a)baikal.ru>:
Christian Costa <titan.costa(a)gmail.com> wrote:
cpp_quote("DEFINE_GUID(CLSID_WICBmpDecoder, 0x6b462062,0x7cbf,0x400d,0x9f,0xdb,0x81,0x3d,0xd1,0x0f,0x27,0x78);") +cpp_quote("DEFINE_GUID(CLSID_WICDibDecoder, 0x6b462062,0x7cbf,0x400d,0x9f,0xdb,0x81,0x3d,0xd1,0x0f,0x27,0x79);") cpp_quote("DEFINE_GUID(CLSID_WICPngDecoder, 0x389ea17b,0x5078,0x4cde,0xb6,0xef,0x25,0xc1,0x51,0x75,0xc7,0x51);") cpp_quote("DEFINE_GUID(CLSID_WICIcoDecoder, 0xc61bfcdf,0x2e0f,0x4aad,0xa8,0xd7,0xe0,0x6b,0xaf,0xeb,0xcd,0xfe);") cpp_quote("DEFINE_GUID(CLSID_WICJpegDecoder, 0x9456a480,0xe88b,0x43ea,0x9e,0x73,0x0b,0x2d,0x9b,0x71,0xb1,0xca);") @@ -1002,6 +1003,7 @@ cpp_quote("DEFINE_GUID(CLSID_WICWmpEncoder, 0xac4ce3cb,0xe1c1,0x44cd,0x82,0x15,0 cpp_quote("DEFINE_GUID(CLSID_WICDefaultFormatConverter, 0x1a3f11dc,0xb514,0x4b17,0x8c,0x5f,0x21,0x54,0x51,0x38,0x52,0xf1);")
cpp_quote("DEFINE_GUID(GUID_ContainerFormatBmp, 0x0af1d87e,0xfcfe,0x4188,0xbd,0xeb,0xa7,0x90,0x64,0x71,0xcb,0xe3);") +cpp_quote("DEFINE_GUID(GUID_ContainerFormatDib, 0x0af1d87e,0xfcfe,0x4188,0xbd,0xeb,0xa7,0x90,0x64,0x71,0xcb,0xe4);")
Is this mocked up names? I couldn't find WIC DIB decoder definitions in the PSDK headers.
-- Dmitry.
Christian Costa <titan.costa(a)gmail.com> wrote:
Oupsss. I forgot to explain. No it's not it native windowscodecs. It's a wine extension to support DIB files used in d3dx9_36. Everything was already in place. I just had to hook things up. I asked to Vincent who told to do this way. At least it's what I understood.
I'd suggest to use public APIs instead of inventing "extenstions". CreateBitmapFromMemory should work for this purpose if you really need to do it using windowscodecs, otherwise GDI APIs may work as well. -- Dmitry.
I'd suggest to use public APIs instead of inventing "extenstions". CreateBitmapFromMemory should work for this purpose if you really need to do it using windowscodecs, otherwise GDI APIs may work as well.
I've been of the opinion that we should have a separate dll for Wine extensions to WIC, which could also work on WIndows, but I was told to put them in the same dll. We have two extensions already: the ICNS encoder and the TGA decoder. Yes, there are other ways we could do this. Probably GDI is the next simplest way. However, this is a case where we can very easily reuse code that was already written to support the BMP and ICO decoders as well as the code in d3dx9 that uses WIC decoders, so to me it still seems like the best option. (I was setting up for this when I was working on the ICO decoder, but that was only a matter of keeping separate the two changes I had to make in the logic for reading ICO frames, and writing DibDecoder_CreateInstance so anyone who went to implement a new one would see it.) The GUIDs for the other extensions are defined in private headers rather than wincodecs.idl, so I guess we should do that here.
Vincent Povirk <madewokherd(a)gmail.com> wrote:
I'd suggest to use public APIs instead of inventing "extenstions". CreateBitmapFromMemory should work for this purpose if you really need to do it using windowscodecs, otherwise GDI APIs may work as well.
I've been of the opinion that we should have a separate dll for Wine extensions to WIC, which could also work on WIndows, but I was told to put them in the same dll. We have two extensions already: the ICNS encoder and the TGA decoder.
That's fine to have WIC support for really useful image formats if there is no other way to handle them, I was thinking to add PCX support at some point. But adding this support should not break other things. For instance using native gdiplus or windowscodecs for testing already breaks in some ways winemenubuilder, and adding a DIB decoder will break d3dx9 by adding a dependency that could be easily avoided.
Yes, there are other ways we could do this. Probably GDI is the next simplest way. However, this is a case where we can very easily reuse code that was already written to support the BMP and ICO decoders as well as the code in d3dx9 that uses WIC decoders, so to me it still seems like the best option.
Personally I don't see why DIB handling needs a WIC codec in the first place, GDI supports it just fine.
(I was setting up for this when I was working on the ICO decoder, but that was only a matter of keeping separate the two changes I had to make in the logic for reading ICO frames, and writing DibDecoder_CreateInstance so anyone who went to implement a new one would see it.)
The ICO decoder probably could be replaced by CreateBitmapFromHICON on the client side, since the latter one is implemented now. For other cases CreateBitmapFromHBITMAP or CreateBitmapFromMemory should work. -- Dmitry.
If there's really a demand for using native windowscodecs without breaking d3dx9 (which already happens because of the TGA decoder), I can supply a win32 build of our WIC extensions that winetricks could install with it. I can't do anything about ICNS.
Yes, there are other ways we could do this. Probably GDI is the next simplest way. However, this is a case where we can very easily reuse code that was already written to support the BMP and ICO decoders as well as the code in d3dx9 that uses WIC decoders, so to me it still seems like the best option.
Personally I don't see why DIB handling needs a WIC codec in the first place, GDI supports it just fine.
Sure, but we ALREADY HAVE CODE for packed dibs that was essentially free and that is currently maintained. To do this from GDI, we still have to write code in d3dx9 that parses the packed DIB enough that we can pass it so some GDI function, then write code that converts a GDI bitmap to a D3DX surface. It gets a little more complicated when you consider things like palettes and rle encoding.
(I was setting up for this when I was working on the ICO decoder, but that was only a matter of keeping separate the two changes I had to make in the logic for reading ICO frames, and writing DibDecoder_CreateInstance so anyone who went to implement a new one would see it.)
The ICO decoder probably could be replaced by CreateBitmapFromHICON on the client side, since the latter one is implemented now. For other cases CreateBitmapFromHBITMAP or CreateBitmapFromMemory should work.
So now we need code in WIC that copies ICO streams into a file on disk so we can get an HICON for the bitmap frames (they could all be PNG) so we can pass that to CreateBitmapFromHICON? No thanks.
Vincent Povirk <madewokherd(a)gmail.com> wrote:
If there's really a demand for using native windowscodecs without breaking d3dx9 (which already happens because of the TGA decoder), I can supply a win32 build of our WIC extensions that winetricks could install with it. I can't do anything about ICNS.
TGA is the simplest format I've ever seen, CreateBitmapFromMemory is the straight way to handle it. I haven't looked at ICNS support, but probably it could be implemented using public APIs as well.
Yes, there are other ways we could do this. Probably GDI is the next simplest way. However, this is a case where we can very easily reuse code that was already written to support the BMP and ICO decoders as well as the code in d3dx9 that uses WIC decoders, so to me it still seems like the best option.
Personally I don't see why DIB handling needs a WIC codec in the first place, GDI supports it just fine.
Sure, but we ALREADY HAVE CODE for packed dibs that was essentially free and that is currently maintained. To do this from GDI, we still have to write code in d3dx9 that parses the packed DIB enough that we can pass it so some GDI function, then write code that converts a GDI bitmap to a D3DX surface. It gets a little more complicated when you consider things like palettes and rle encoding.
DIB sections and things like SetDIBits should take care of all of this.
(I was setting up for this when I was working on the ICO decoder, but that was only a matter of keeping separate the two changes I had to make in the logic for reading ICO frames, and writing DibDecoder_CreateInstance so anyone who went to implement a new one would see it.)
The ICO decoder probably could be replaced by CreateBitmapFromHICON on the client side, since the latter one is implemented now. For other cases CreateBitmapFromHBITMAP or CreateBitmapFromMemory should work.
So now we need code in WIC that copies ICO streams into a file on disk so we can get an HICON for the bitmap frames (they could all be PNG) so we can pass that to CreateBitmapFromHICON? No thanks.
I don't see why you need to write anything to a file at all for a decoder if you already have bitmap bits. -- Dmitry.
If there's really a demand for using native windowscodecs without breaking d3dx9 (which already happens because of the TGA decoder), I can supply a win32 build of our WIC extensions that winetricks could install with it. I can't do anything about ICNS.
TGA is the simplest format I've ever seen, CreateBitmapFromMemory is the straight way to handle it. I haven't looked at ICNS support, but probably it could be implemented using public APIs as well.
The main reason I put the TGA decoder in WIC is that I know nothing about d3dx9 and wanted to help solve the problem without having to learn a new codebase. I don't know enough about d3dx9 to say whether it makes sense to put new image format handling code in there (other than DDS, which has features WIC can't support), but I can't imagine it making sense to move an existing, working format there.
Sure, but we ALREADY HAVE CODE for packed dibs that was essentially free and that is currently maintained. To do this from GDI, we still have to write code in d3dx9 that parses the packed DIB enough that we can pass it so some GDI function, then write code that converts a GDI bitmap to a D3DX surface. It gets a little more complicated when you consider things like palettes and rle encoding.
DIB sections and things like SetDIBits should take care of all of this.
SetDIBits still requires us to calculate the offset of the bitmap bits, which requires understanding of the format. Which we would have to duplicate between WIC and D3DX, because we already need that logic in the WIC ICO decoder. And it definitely does not give us any usable D3DX structures without further work. These aren't very hard problems, sure, but I can't see the justification for writing new solutions to any problems when we already have something that should work with no extra effort.
So now we need code in WIC that copies ICO streams into a file on disk so we can get an HICON for the bitmap frames (they could all be PNG) so we can pass that to CreateBitmapFromHICON? No thanks.
I don't see why you need to write anything to a file at all for a decoder if you already have bitmap bits.
Writing to a file is the only way I can see to make the standard icon handling functions (ExtractIcon) handle the format. If we don't use those, we have to handle the quirks of masks and double heights, at which point GDI doesn't help us very much. Maybe if we were starting from scratch it would've made sense to solve these problems differently. Obviously I disagree or I wouldn't have done things the way I did. But the ICO/BMP decoder we have now works (and probably works for DIB as well) and has no known problems that we can't solve with the current design. Why would we want to tear it down and replace it with three new things, or even just add one new thing to d3dx to handle a case the decoder can handle now?
Vincent Povirk <madewokherd(a)gmail.com> wrote:
TGA is the simplest format I've ever seen, CreateBitmapFromMemory is the straight way to handle it. I haven't looked at ICNS support, but probably it could be implemented using public APIs as well.
The main reason I put the TGA decoder in WIC is that I know nothing about d3dx9 and wanted to help solve the problem without having to learn a new codebase. I don't know enough about d3dx9 to say whether it makes sense to put new image format handling code in there (other than DDS, which has features WIC can't support), but I can't imagine it making sense to move an existing, working format there.
So instead of writing much less code in d3dx9 to support TGA you've decided to write the whole new WIC decoder (even if it ended in a lot much more code) just because you know WIC code base better? And now you are proposing to do the same to support DIB? I object to this approach. -- Dmitry.
TGA is the simplest format I've ever seen, CreateBitmapFromMemory is the straight way to handle it. I haven't looked at ICNS support, but probably it could be implemented using public APIs as well.
The main reason I put the TGA decoder in WIC is that I know nothing about d3dx9 and wanted to help solve the problem without having to learn a new codebase. I don't know enough about d3dx9 to say whether it makes sense to put new image format handling code in there (other than DDS, which has features WIC can't support), but I can't imagine it making sense to move an existing, working format there.
So instead of writing much less code in d3dx9 to support TGA you've decided to write the whole new WIC decoder (even if it ended in a lot much more code) just because you know WIC code base better? And now you are proposing to do the same to support DIB? I object to this approach.
Yes, it is possible that I wrote more code than was necessary to support TGA because I knew WIC better and because I thought it'd be cool (in theory the decoder can now be used outside the context of d3dx9, for instance on windows with the builtin image viewer - not all that useful but still pretty cool imo). For DIB it is less code because all of the logic involved is already used to support BMP or ICO, and all of the real work for it was done at the same time as adding ICO support.
Vincent Povirk <madewokherd(a)gmail.com> wrote:
Yes, it is possible that I wrote more code than was necessary to support TGA because I knew WIC better and because I thought it'd be cool (in theory the decoder can now be used outside the context of d3dx9, for instance on windows with the builtin image viewer - not all that useful but still pretty cool imo).
Adding Wine specific extensions should be slightly more justified IMHO.
For DIB it is less code because all of the logic involved is already used to support BMP or ICO, and all of the real work for it was done at the same time as adding ICO support.
I'd argue that it's even less code to load a DIB since gdi32 has full built-in support for it. Taking into account all the overhead that COM adds (although in d3dx9 case it's already there) writing a new WIC codec is not always justified. -- Dmitry.
participants (3)
-
Christian Costa -
Dmitry Timoshkov -
Vincent Povirk