Re: [3/4] kernel32/tests: Add 0-length read tests for a pipe.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://newtestbot.winehq.org/JobDetails.pl?Key=2250 Your paranoid android. === wvista (32 bit pipe) === Failure running script in VM: network read error: Resource temporarily unavailable
Re: [3/4] imm32: use thread data from target HWND
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://newtestbot.winehq.org/JobDetails.pl?Key=2256 Your paranoid android. === build (build) === Patch failed to apply
Re: [3/4] ws2_32: Filter invalid socket parameters and return the appropriate error
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://newtestbot.winehq.org/JobDetails.pl?Key=2228 Your paranoid android. === build (build) === Patch failed to apply
Re: [3/4] ws2_32: Filter invalid socket parameters and return the appropriate error
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://newtestbot.winehq.org/JobDetails.pl?Key=2237 Your paranoid android. === build (build) === Patch failed to apply
Re: [3/4] gdi32: IntersectClipRect should update actual clipping region for a EMF DC.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=24396 Your paranoid android. === WINEBUILD (build) === Patch failed to apply
Re: [3/4] windowscodecs: Implement CreateBitmapFromMemory.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=23710 Your paranoid android. === WINEBUILD (build) === Patch failed to apply
Re: [3/4] windowscodecs: Implement CreateBitmapFromMemory.
Vincent Povirk madewokh...@gmail.com wrote:
+hr = IWICBitmap_Lock(bitmap, NULL, WICBitmapLockWrite, lock);
+if (SUCCEEDED(hr))
+{
+UINT buffersize;
+BYTE *buffer;
+
+hr = IWICBitmapLock_GetDataPointer(lock, buffersize, buffer);
+
+memcpy(buffer, pbBuffer, uiHeight * cbStride);
+
+IWICBitmapLock_Release(lock);
+}
It's better to actually check return value of IWICBitmapLock_GetDataPointer
before memecpy(), or if that's not supposed to fail drop 'hr' assignment.
My approach to pass buffer directly to BitmapImpl_Create looks much simpler
than fiddling with IWICBitmap_Lock IMHO.
--
Dmitry.
Re: [3/4] windowscodecs: Implement CreateBitmapFromMemory.
On Wed, Jan 2, 2013 at 11:42 PM, Dmitry Timoshkov dmi...@baikal.ru wrote: It's better to actually check return value of IWICBitmapLock_GetDataPointer before memecpy(), or if that's not supposed to fail drop 'hr' assignment. Whoops, good catch. (I don't think it can fail, but I didn't mean to make that assumption.) I guess I prefer to keep interfaces as simple as possible even if it means writing more code? I'd consider writing a helper function, but none of the other methods would use it.
Re: [3/4] windowscodecs: Implement CreateBitmapFromMemory.
Vincent Povirk madewokh...@gmail.com wrote: I guess I prefer to keep interfaces as simple as possible even if it means writing more code? I'd consider writing a helper function, but none of the other methods would use it. Is there any hurry with CreateBitmapFromMemory implementation, or you somehow prefer your own one? I'm asking because I was planning to send my implementation (fixed to create a copy of the passed in buffer) after new year vacations, but it looks like you need it right now. -- Dmitry.
Re: [3/4] comctl32: fix notifications and return value when collapsing already collapsed node (rebased)
Hi, Daniel. You forgot a patch.
Re: [3/4] comctl32: fix notifications and return value when collapsing already collapsed node (rebased)
Hello, Nikolay. Thanks for reminder. Attached it now. 2012/10/15 Nikolay Sivov bungleh...@gmail.com: Hi, Daniel. You forgot a patch.
Re: [3/4] shell32/tests: Simplify shlexec's test_argify() and test_lpFile_parsed() and avoid numeric literals.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=22056 Your paranoid android. === WINEBUILD (build) === Patch failed to apply
Re: [3/4] dxgi: Avoid division by zero. Based on patch by Eduard - Gabriel Munteanu.
On 23 September 2012 22:40, Ričardas Barkauskas rbarkaus...@codeweavers.com wrote: +static UINT dxgi_rational_to_uint(DXGI_RATIONAL *rational) rational should be const.
Re: [3/4] windowscodecs: Implement Image Descriptor metadata reader.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=21584 Your paranoid android. === WINEBUILD (build) === Patch failed to apply
Re: [3/4] gdiplus: Destination points passed to GdipDrawImagePointsRect should be in device units.
Dmitry Timoshkov dmi...@baikal.ru wrote: +scale_x = units_scale(srcUnit, graphics-unit, graphics-xres); +scale_y = units_scale(srcUnit, graphics-unit, graphics-yres); This depends on a not yet sent series which introduces units_scale(), please ignore for now, I'll resend it a bit later. -- Dmitry.
Re: [3/4] gdiplus: Add some tests for GdipGetPropertyItemSize and GdipGetPropertyItem.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://winetestbot.dolphin/JobDetails.pl?Key=100 Your paranoid android. === debiantesting (build) === Patch failed to apply
Re: [3/4] gdiplus: Add some tests for GdipGetPropertyItemSize and GdipGetPropertyItem. Take 2.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://winetestbot.dolphin/JobDetails.pl?Key=158 Your paranoid android. === debiantesting (build) === Patch failed to apply
Re: [3/4] gdiplus: Add some tests for GdipGetPropertyItemSize and GdipGetPropertyItem. Take 2.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=19764 Your paranoid android. === WINEBUILD (build) === Patch failed to apply
Re: [3/4] gdiplus: Add some tests for GdipGetPropertyItemSize and GdipGetPropertyItem. Take 2.
Marvin test...@testbot.winehq.org wrote: === WINEBUILD (build) === Patch failed to apply Either the testbot needs to reset its incoming queue once a day, or mail delivery problems for patch batches in winehq should be fixed. -- Dmitry.
Re: [3/4] gdiplus: Add some tests for GdipGetPropertyItemSize and GdipGetPropertyItem.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=19594 Your paranoid android. === W7PROX64 (64 bit image) === image.c:2811: Test failed: 0: expected property size 17 or 36, got 44 image.c:2811: Test failed: 2: expected property size 144 or 0, got 152 image.c:2811: Test failed: 3: expected property size 20 or 0, got 28 === TEST64_W7SP1 (64 bit image) === image.c:2811: Test failed: 0: expected property size 17 or 36, got 44 image.c:2811: Test failed: 2: expected property size 144 or 0, got 152 image.c:2811: Test failed: 3: expected property size 20 or 0, got 28
Re: [3/4] gdiplus: Add some tests for GdipGetPropertyItemSize and GdipGetPropertyItem.
Marvin test...@testbot.winehq.org wrote: === W7PROX64 (64 bit image) === image.c:2811: Test failed: 0: expected property size 17 or 36, got 44 image.c:2811: Test failed: 2: expected property size 144 or 0, got 152 image.c:2811: Test failed: 3: expected property size 20 or 0, got 28 === TEST64_W7SP1 (64 bit image) === image.c:2811: Test failed: 0: expected property size 17 or 36, got 44 image.c:2811: Test failed: 2: expected property size 144 or 0, got 152 image.c:2811: Test failed: 3: expected property size 20 or 0, got 28 Looks like testbot picked up wrong e-mail sequence. It looks like an e-mail delivery problem in the mailing list software, and it is getting pretty annoying. -- Dmitry.
Re: [3/4] gdiplus: Add some tests for GdipGetPropertyItemSize and GdipGetPropertyItem.
Dmitry Timoshkov dmi...@baikal.ru wrote: Marvin test...@testbot.winehq.org wrote: === W7PROX64 (64 bit image) === image.c:2811: Test failed: 0: expected property size 17 or 36, got 44 image.c:2811: Test failed: 2: expected property size 144 or 0, got 152 image.c:2811: Test failed: 3: expected property size 20 or 0, got 28 === TEST64_W7SP1 (64 bit image) === image.c:2811: Test failed: 0: expected property size 17 or 36, got 44 image.c:2811: Test failed: 2: expected property size 144 or 0, got 152 image.c:2811: Test failed: 3: expected property size 20 or 0, got 28 Looks like testbot picked up wrong e-mail sequence. It looks like an e-mail delivery problem in the mailing list software, and it is getting pretty annoying. Just in case sent precompiled version of the tests to testbot once again, and they passed: https://testbot.winehq.org/JobDetails.pl?Key=19596 -- Dmitry.
Re: [3/4] gdiplus: Add some tests for GdipGetPropertyItemSize and GdipGetPropertyItem.
On 27/06/2012 7:35 PM, Dmitry Timoshkov wrote: Marvin test...@testbot.winehq.org wrote: === W7PROX64 (64 bit image) === image.c:2811: Test failed: 0: expected property size 17 or 36, got 44 image.c:2811: Test failed: 2: expected property size 144 or 0, got 152 image.c:2811: Test failed: 3: expected property size 20 or 0, got 28 === TEST64_W7SP1 (64 bit image) === image.c:2811: Test failed: 0: expected property size 17 or 36, got 44 image.c:2811: Test failed: 2: expected property size 144 or 0, got 152 image.c:2811: Test failed: 3: expected property size 20 or 0, got 28 Looks like testbot picked up wrong e-mail sequence. It looks like an e-mail delivery problem in the mailing list software, and it is getting pretty annoying. The winehq.org mailserver only allows one SMTP connection per relaying IP, so emails sent at the same time are likely to be received out of order unless the relaying mailserver only transfers emails sequentially. -- Ben Peddell IT Support Bowen, Collinsville and Proserpine Catholic schools http://klightspeed.killerwolves.net/
Re: [3/4] gdiplus: Add some tests for GdipGetPropertyItemSize and GdipGetPropertyItem.
Ben Peddell klightsp...@netspace.net.au wrote: Looks like testbot picked up wrong e-mail sequence. It looks like an e-mail delivery problem in the mailing list software, and it is getting pretty annoying. The winehq.org mailserver only allows one SMTP connection per relaying IP, so emails sent at the same time are likely to be received out of order unless the relaying mailserver only transfers emails sequentially. It doesn't explain the lost of some e-mails. For instance patch tracker may see 2 of 5 patches while http://www.winehq.org/pipermail/wine-patches lists 3 of 5. -- Dmitry.
Re: [3/4] server: Perform an access check for kernel objects without a security descriptor using access rights of the owner'
Marvin test...@testbot.winehq.org wrote: === WVISTAADM (32 bit sync) === sync.c:923: Test failed: DeleteTimerQueueTimer This failure has nothing to do with that patch. -- Dmitry.
Re: [3/4] server: Perform an access check for kernel objects without a security descriptor using access rights of the owner'
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=17911 Your paranoid android. === WVISTAADM (32 bit sync) === sync.c:923: Test failed: DeleteTimerQueueTimer
Re: [3/4] ddraw: Set correct HEL and HAL color models
Am Samstag, 10. Dezember 2011, 22:26:34 schrieb Stefan Dösinger: This looks strange. The system doesn't support hardware d3d, since the CreateDevice calls for HAL and HW TL HAL calls fail, but it still enumerates them. I guess adding a broken() is the way to go. I'll send a patch next week. I just noticed that I never sent the promised patch. Can you file bugs for those issues next time and assign them to me? A bugtracker is a better way to keep track of things than my poorly organized inbox. signature.asc Description: This is a digitally signed message part.
Re: [3/4] ddraw: Set correct HEL and HAL color models
Stefan, it looks like your patch http://source.winehq.org/git/wine.git/commitdiff/9e0baa55cec232656048c972e94a9dc2a15ec30b has introduced 2 failures on one virtual w2k3-vmware machine: http://test.winehq.org/data/e7bbb4ef1e95396b72a58f813b4346d9abccb699/2003_wtb-w2k3r2sex64-32/ddraw:d3d.html See earlier results: http://test.winehq.org/data/tests/ddraw:d3d.html Would you mind fixing this, please:)? S.
Re: [3/4] ddraw: Set correct HEL and HAL color models
Am Samstag, 10. Dezember 2011, 18:44:32 schrieb Saulius Krasuckas: Stefan, it looks like your patch http://source.winehq.org/git/wine.git/commitdiff/9e0baa55cec232656048c972e94 a9dc2a15ec30b has introduced 2 failures on one virtual w2k3-vmware machine: http://test.winehq.org/data/e7bbb4ef1e95396b72a58f813b4346d9abccb699/2003_wt b-w2k3r2sex64-32/ddraw:d3d.html This looks strange. The system doesn't support hardware d3d, since the CreateDevice calls for HAL and HW TL HAL calls fail, but it still enumerates them. I guess adding a broken() is the way to go. I'll send a patch next week.
Re: [3/4] winemaker: Be less picky when detecting the target type (try 2)
Am 19.11.2011 20:18, schrieb Vitaliy Margolen:
On 11/19/2011 11:42 AM, André Hentschel wrote:
this catches more than one 0 after the space and possible 0s after the x
+if (/[[:space:]]0+x0*101$/) {
For more then one you should use + not *. * means any number, including
0.
Vitaliy
So i used:
+ before x to match at least one 0, but don't complain if it's more than one
like 00x0101
* after x to match no 0 like in 00x101 or expect much more like in
00x0101 or maybe something like 0x0099
that's what i meant with possible 0s
--
Best Regards, André Hentschel
Re: [3/4] winemaker: Be less picky when detecting the target type (try 2)
On 11/19/2011 11:42 AM, André Hentschel wrote:
this catches more than one 0 after the space and possible 0s after the x
+if (/[[:space:]]0+x0*101$/) {
For more then one you should use + not *. * means any number, including 0.
Vitaliy
Re: [3/4] ddraw: Add more tests and fixes for SetSurfaceDesc
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=15326 Your paranoid android. === WINEBUILD (build) === Patch failed to apply
Re: [3/4] msxml3: Add IDispatchEx support for IXMLDOMNamedNodeMap
Hi Nikolay,
On 11/04/11 16:46, Nikolay Sivov wrote:
+if(This-data-vtbl This-data-vtbl-invoke) {
+hres = This-data-vtbl-invoke(This-outer, id, lcid, wFlags, pdp,
pvarRes, pei);
+if (hres != E_NOTIMPL) return hres;
+}
Using E_NOTIMPL for special meaning here seems wrong to me.
DISP_E_UNKNOWNNAME could be a better choice. However, if you'd move
custom invoke implementation to be the last choice, after dynamic and
typelib-based values, then you could simply return whatever invoke returns.
Jacek
Re: [3/4] msxml3: Add IDispatchEx support for IXMLDOMNamedNodeMap
On 11/4/2011 20:00, Jacek Caban wrote:
Hi Nikolay,
On 11/04/11 16:46, Nikolay Sivov wrote:
+if(This-data-vtbl This-data-vtbl-invoke) {
+hres = This-data-vtbl-invoke(This-outer, id, lcid,
wFlags, pdp, pvarRes, pei);
+if (hres != E_NOTIMPL) return hres;
+}
Using E_NOTIMPL for special meaning here seems wrong to me.
DISP_E_UNKNOWNNAME could be a better choice. However, if you'd move
custom invoke implementation to be the last choice, after dynamic and
typelib-based values, then you could simply return whatever invoke
returns.
I was thinking about additional call in vtable to use instead of
is_custom to check if dispid is supposed to be handled with custom
invoke. What do you think about it?
Jacek
Re: [3/4] msxml3: Add IDispatchEx support for IXMLDOMNamedNodeMap
On 11/04/11 17:11, Nikolay Sivov wrote:
On 11/4/2011 20:00, Jacek Caban wrote:
Hi Nikolay,
On 11/04/11 16:46, Nikolay Sivov wrote:
+if(This-data-vtbl This-data-vtbl-invoke) {
+hres = This-data-vtbl-invoke(This-outer, id, lcid,
wFlags, pdp, pvarRes, pei);
+if (hres != E_NOTIMPL) return hres;
+}
Using E_NOTIMPL for special meaning here seems wrong to me.
DISP_E_UNKNOWNNAME could be a better choice. However, if you'd move
custom invoke implementation to be the last choice, after dynamic and
typelib-based values, then you could simply return whatever invoke
returns.
I was thinking about additional call in vtable to use instead of
is_custom to check if dispid is supposed to be handled with custom
invoke. What do you think about it?
It really depends on how it's used in msxml3. The problem with this
solution is that it means more code in every object using custom dipids.
If we can avoid it, even at cost of more complicated code in dispex.c,
that's probably better in terms of code maintaining IMO.
Jacek
Re: [3/4] msxml3: Add IDispatchEx support for IXMLDOMNamedNodeMap
On 11/4/2011 20:21, Jacek Caban wrote:
On 11/04/11 17:11, Nikolay Sivov wrote:
On 11/4/2011 20:00, Jacek Caban wrote:
Hi Nikolay,
On 11/04/11 16:46, Nikolay Sivov wrote:
+if(This-data-vtbl This-data-vtbl-invoke) {
+hres = This-data-vtbl-invoke(This-outer, id, lcid,
wFlags, pdp, pvarRes, pei);
+if (hres != E_NOTIMPL) return hres;
+}
Using E_NOTIMPL for special meaning here seems wrong to me.
DISP_E_UNKNOWNNAME could be a better choice. However, if you'd move
custom invoke implementation to be the last choice, after dynamic
and typelib-based values, then you could simply return whatever
invoke returns.
I was thinking about additional call in vtable to use instead of
is_custom to check if dispid is supposed to be handled with custom
invoke. What do you think about it?
It really depends on how it's used in msxml3.
The only case I'm aware of so far is index based access of collection
elements, that's all. So I'll better go with DISP_E_UNKNOWNNAME for now
cause use of this thing is really limited in msxml.
The problem with this solution is that it means more code in every
object using custom dipids. If we can avoid it, even at cost of more
complicated code in dispex.c, that's probably better in terms of code
maintaining IMO.
Right, extra call is too much probably.
Jacek
Re: [3/4] d3d9/tests: Test partial block locks
On 31 October 2011 20:03, Stefan Dösinger ste...@codeweavers.com wrote: +case D3DPOOL_SCRATCH: +case D3DPOOL_SYSTEMMEM: +hr = IDirect3DDevice9_CreateOffscreenPlainSurface(device, 128, 128, formats[i].fmt, +D3DPOOL_SCRATCH, surface, 0); That doesn't really test D3DPOOL_SYSTEMMEM.
Re: [3/4] d3d9/tests: Test partial block locks
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=15192 Your paranoid android. === WINEBUILD (build) === Patch failed to apply
Re: [3/4] d3d9/tests: Test partial block locks
On Monday 31 October 2011 21:10:41 Henri Verbeet wrote:
On 31 October 2011 20:03, Stefan Dösinger ste...@codeweavers.com wrote:
+case D3DPOOL_SCRATCH:
+case D3DPOOL_SYSTEMMEM:
+hr =
IDirect3DDevice9_CreateOffscreenPlainSurface(device, 128, 128,
formats[i].fmt, +D3DPOOL_SCRATCH, surface,
0);
That doesn't really test D3DPOOL_SYSTEMMEM.
That was a pretty simple change, but when I re-ran the tests on Windows to be
sure they work the ATI2N test failed.
On Nvidia drivers ATI2N has 4x4 blocks, which makes sense, but if you violate
that the driver returns E_INVALIDARG instead of D3DERR_INVALIDCALL. Oh well.
On AMD, ATI2N has 2x2 blocks, which doesn't make sense to me. When I ran the
tests on Windows the last time the 1x1 blocks locked fine. I couldn't
reproduce that even with the last version of my patch.
I've changed the test and implementation according to those results, but
before I resubmit them I'll test a few more Windows machines(especially dx9
and xp based ones).
What's changed:
*) block_size removed, this was unused
*) Improved messages, print the pool
*) Use block_dimensions / 2 instead of block_dimensions - 1 to construct
invalid blocks.
*) Adjusted test to 2x2 / 4x4 ATI2N block sizes
From 98f5f0f70f09a501ea6d236047f70d777d0c2eae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20D=C3=B6singer?= ste...@codeweavers.com
Date: Fri, 14 Oct 2011 12:54:37 +0200
Subject: [PATCH 03/13] d3d9/tests: Test partial block locks
---
dlls/d3d9/tests/surface.c | 171 +
1 files changed, 171 insertions(+), 0 deletions(-)
diff --git a/dlls/d3d9/tests/surface.c b/dlls/d3d9/tests/surface.c
index 361e8a3..70ae71a 100644
--- a/dlls/d3d9/tests/surface.c
+++ b/dlls/d3d9/tests/surface.c
@@ -633,6 +633,176 @@ static void test_surface_double_unlock(IDirect3DDevice9 *device)
}
}
+static void test_surface_lockrect_blocks(IDirect3DDevice9 *device)
+{
+IDirect3DTexture9 *texture;
+IDirect3DSurface9 *surface;
+IDirect3D9 *d3d;
+D3DLOCKED_RECT locked_rect;
+unsigned int i, j;
+HRESULT hr;
+RECT rect;
+BOOL surface_only;
+
+const struct
+{
+D3DFORMAT fmt;
+const char *name;
+unsigned int block_width;
+unsigned int block_height;
+BOOL broken;
+}
+formats[] =
+{
+{D3DFMT_DXT1, D3DFMT_DXT1, 4, 4, FALSE},
+{D3DFMT_DXT2, D3DFMT_DXT2, 4, 4, FALSE},
+{D3DFMT_DXT3, D3DFMT_DXT3, 4, 4, FALSE},
+{D3DFMT_DXT4, D3DFMT_DXT4, 4, 4, FALSE},
+{D3DFMT_DXT5, D3DFMT_DXT5, 4, 4, FALSE},
+/* ATI2N has 2x2 blocks on AMD drivers, which doesn't make sense. */
+{MAKEFOURCC('A','T','I','2'), ATI2N, 4, 4, TRUE},
+};
+static const struct
+{
+D3DPOOL pool;
+const char *name;
+/* Don't check the return value, Nvidia returns D3DERR_INVALIDCALL for some formats
+ * and E_INVALIDARG/DDERR_INVALIDPARAMS for others. */
+BOOL success;
+}
+pools[] =
+{
+{D3DPOOL_DEFAULT, D3DPOOL_DEFAULT, FALSE},
+{D3DPOOL_SCRATCH, D3DPOOL_SCRATCH, TRUE},
+{D3DPOOL_SYSTEMMEM, D3DPOOL_SYSTEMMEM,TRUE},
+{D3DPOOL_MANAGED, D3DPOOL_MANAGED, TRUE},
+};
+
+hr = IDirect3DDevice9_GetDirect3D(device, d3d);
+ok(SUCCEEDED(hr), IDirect3DDevice9_GetDirect3D failed (%08x)\n, hr);
+
+for (i = 0; i (sizeof(formats) / sizeof(*formats)); ++i) {
+surface_only = FALSE;
+hr = IDirect3D9_CheckDeviceFormat(d3d, 0, D3DDEVTYPE_HAL, D3DFMT_X8R8G8B8, D3DUSAGE_DYNAMIC,
+D3DRTYPE_TEXTURE, formats[i].fmt);
+if (FAILED(hr))
+{
+hr = IDirect3D9_CheckDeviceFormat(d3d, 0, D3DDEVTYPE_HAL, D3DFMT_X8R8G8B8, 0, D3DRTYPE_SURFACE, formats[i].fmt);
+if (FAILED(hr))
+{
+skip(Format %s not supported, skipping lockrect offset test\n, formats[i].name);
+continue;
+}
+surface_only = TRUE;
+}
+
+for (j = 0; j (sizeof(pools) / sizeof(*pools)); j++)
+{
+switch (pools[j].pool)
+{
+case D3DPOOL_SYSTEMMEM:
+case D3DPOOL_MANAGED:
+if (surface_only) continue;
+/* Fall through */
+case D3DPOOL_DEFAULT:
+if (surface_only)
+{
+hr = IDirect3DDevice9_CreateOffscreenPlainSurface(device, 128, 128, formats[i].fmt,
+pools[j].pool, surface, 0);
+ok(SUCCEEDED(hr), IDirect3DDevice9_CreateOffscreenPlainSurface failed (%08x)\n, hr);
+}
+else
+{
+hr =
Re: [3/4] wined3d: reject ffp blits if the destination is not fbo attachable
On 11 October 2011 22:30, Stefan Dösinger ste...@codeweavers.com wrote:
+if (wined3d_settings.offscreen_rendering_mode == ORM_FBO)
+{
+if (!((dst_format-flags WINED3DFMT_FLAG_FBO_ATTACHABLE) ||
(dst_usage WINED3DUSAGE_RENDERTARGET)))
+return FALSE;
+}
+else if (!(dst_usage WINED3DUSAGE_RENDERTARGET))
+{
+return FALSE;
+}
Same consideration as for 2/4. Guess I should have caught this when
bccfd7cc introduced it.
Re: [3/4] msxml3: Add error code defines (resend)
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=6690 Your paranoid android. === WINEBUILD (build) === Make of import library failed Make of import library failed
Re: [3/4] ddraw: fix DDSCAPS_3DDEVICE surfaces always setting DDSCAPS_VISIBLE
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=6460 Your paranoid android. === W98SE (32 bit dsurface) === No test summary line found
Re: [3/4]strmbase: implement OLE registration in AMovieDllRegisterServer2
Aric Stewart a...@codeweavers.com writes:
HRESULT WINAPI AMovieDllRegisterServer2(BOOL bRegister)
{
HRESULT hr;
int i;
IFilterMapper2 *pIFM2 = NULL;
+HMODULE psapi;
+fnGetModuleFileNameExW pFunc;
+WCHAR szFileName[MAX_PATH];
+
+psapi = LoadLibraryA(psapi.dll);
+if (psapi)
+{
+pFunc = (fnGetModuleFileNameExW)GetProcAddress(psapi,
GetModuleFileNameExW);
+if (pFunc)
+{
+if (!pFunc(GetCurrentProcess(), g_hInst, szFileName, MAX_PATH))
+{
+ERR(Failed to get module file name for registration\n);
+return E_FAIL;
+}
+}
+else
+{
+ERR(Failed to get get function GetModuleFileNameExW\n);
+return E_FAIL;
+}
+}
+else
+{
+ERR(Failed to load psapi.dll\n);
+return E_FAIL;
+}
+FreeLibrary(psapi);
There's no reason to use psapi for this.
--
Alexandre Julliard
julli...@winehq.org
Re: [3/4] msxml3: Fix IXMLDOMNode::get_namespaceURI() for empty URIs
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=5208 Your paranoid android. === W98SE (32 bit domdoc) === domdoc.c:6187: Test failed: failed to create CLSID_DOMDocument instance: 0x80040154 === WNT4WSSP6 (32 bit domdoc) === domdoc.c:6187: Test failed: failed to create CLSID_DOMDocument instance: 0x80040154 === W2KPROSP4 (32 bit domdoc) === domdoc.c:6187: Test failed: failed to create CLSID_DOMDocument instance: 0x80040154
Re: [3/4] msxml3/tests: Some tests for ::get_ownerDocument() returned document instance
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=5061 Your paranoid android. === W98SE (32 bit domdoc) === domdoc.c:5947: Test failed: failed to create CLSID_DOMDocument instance: 0x80040154 === WNT4WSSP6 (32 bit domdoc) === domdoc.c:5947: Test failed: failed to create CLSID_DOMDocument instance: 0x80040154 === W2KPROSP4 (32 bit domdoc) === domdoc.c:5947: Test failed: failed to create CLSID_DOMDocument instance: 0x80040154
Re: [3/4] oleaut32/olepicture: Fix ::get_hPal() behaviour for certain picture types
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=4737 Your paranoid android. === W7PROX64 (64 bit olepicture) === olepicture.c:765: Test failed: got 0 olepicture.c:777: Test failed: got 0
Re: [3/4] shdocvw: Add and use AdviseSinkInerface for IE
On 7/27/10 1:25 PM, Alexander Nicolaysen Sørnes wrote:
This will be used to change the address bar URL when navigating to new pages
+*ppvoid = NULL;
+if(IsEqualGUID(IID_IAdviseSink, riid))
You also need to handle IUnknown.
+{
+*ppvoid = iface;
+AdviseSink_AddRef(iface);
Direct call to interface function is not really nice.
+typedef struct _AdviseSink {
+const IAdviseSinkVtbl *lpAdviseSinkVtbl;
+LONG ref;
+DocHost* doc_host;
+} AdviseSink;
Is there any reason to make it a separated struct?
Also, you don't need it for URL change notification. We already have better way
of doing that in shdocvw. See IPropertyNotifySink related code.
Jacek
Re: [3\4] d3d9: Partially implement GetAdapterDisplayModeEx
On Tuesday, July 20, 2010 2:28:44 pm Louis Lenders wrote: +TRACE(iface %p, adapter %u, mode %p.\n, iface, adapter, mode); +TRACE(iface %p, adapter %u, mode %p, rotation %p\n, iface, adapter, mode, rotation); This doesn't look right. You probably only need the second TRACE statement, there. It also looks like adapterinfo is being leaked. Any reason you HeapAlloc one to pass in, instead of passing a pointer to a stack object? Eg: struct wined3d_adapter_info_ex adapterinfo; ... hr = IWineD3D_GetAdapterDisplayMode(This-WineD3D, adapter, m, adapterinfo);
Re: 3/4 wined3d: add color_fill to blit_shader [try 2]
That's more reasonable, but you still have a NULL function pointer in arbfp_blit for color_fill(), and blit_operation is unused.
Re: 3/4 wined3d: add color_fill to blit_shader
On 25 March 2010 23:04, Roderick Colenbrander thunderbir...@gmail.com wrote:
This patch adds a first part of the new blit_shader infrastructure. In
a few days I plan to submit more parts of it which will extend the
blit_shader and rewrite parts in the end.
This is patch is pretty hacky.
+static const struct blit_shader *blitters[] =
+{
+ffp_blit,
+cpu_blit
+};
This doesn't make much sense here. You'll probably want to make it a
variable local to surface_select_blitter() as a start, but eventually
it should be stored in the device / adapter.
+const struct blit_shader *surface_select_blitter(const struct
wined3d_gl_info *gl_info, IWineD3DSurfaceImpl *src, IWineD3DSurfaceImpl *dst,
enum blit_operation op)
Why do we have both this and select_blit_implementation() now? Also, static.
+switch(op)
+{
+case BLIT_OP_COLOR_FILL:
+return TRUE;
+default:
+return FALSE;
+}
This is a silly way to write return op == BLIT_OP_COLOR_FILL;.
+const struct blit_shader cpu_blit = {
+NULL, /* alloc_private */
+NULL, /* free_private */
+NULL, /* set_shader */
+NULL, /* unset_shader */
+NULL, /* color_fixup_supported */
+cpu_blit_can_blit,
+cpu_blit_color_fill
};
You can't do that, callers don't check these for NULL.
+typedef enum blit_operation
+{
+BLIT_OP_COLOR_FILL=1,
+BLIT_OP_BLIT_SYSMEM_TO_DRAWABLE=2,
+BLIT_OP_BLIT_DRAWABLE_TO_TEXTURE=3
+} blit_operation;
Besides being unused, the distinction between SYSMEM_TO_DRAWABLE,
DRAWABLE_TO_TEXTURE, and other potential variants is an
implementation detail callers shouldn't care about. Also, we don't
care about the exact values of the enum elements, and the typedef only
obscures the type of blit_operation.
BOOL (*color_fixup_supported)(const struct wined3d_gl_info *gl_info,
struct color_fixup_desc fixup);
+BOOL (*can_blit)(const struct wined3d_gl_info *gl_info, blit_operation
op, IWineD3DSurfaceImpl *src_surface, IWineD3DSurfaceImpl *dst_surface);
The functionality of can_blit is a superset of color_fixup_supported.
+IWineD3DSurface_GetContainer((IWineD3DSurface*)dst_surface,
IID_IWineD3DSwapChain, (void **)dst_swapchain);
+if(dst_swapchain) IWineD3DSwapChain_Release((IWineD3DSwapChain *)
dst_swapchain);
That's three different styles of doing a cast, in two lines of code.
And code that's supposedly being rewritten at that. At this point I
don't even care about the exact style you're using (although I do have
a preference), but try to at least care about consistency. Same goes
for these:
+if(dst_swapchain) IWineD3DSwapChain_Release((IWineD3DSwapChain *)
dst_swapchain);
+switch(op)
+if ((IWineD3DSurface*)dst_surface !=
device-render_targets[0]
+const struct blit_shader *surface_select_blitter(const struct
wined3d_gl_info *gl_info, IWineD3DSurfaceImpl *src, IWineD3DSurfaceImpl *dst,
enum blit_operation op)
This line is a bit long for my taste. I think I have relatively large
screens, but this takes up over half a screen. Personally I think 120
characters is a reasonable upper limit, but there are probably plenty
of people that would be happier with 80.
Essentially, I think you're trying to take too large steps. E.g.
splitting of functionality into cpu_blit_color_fill(),
ffp_blit_color_fill(), etc. is ok, hacking it into struct blit_shader
isn't. Extending color_fixup_supported() into something like
blit_supported() could also be a reasonable patch, on its own, but
trying to do it all at the same time becomes hacky pretty quickly.
Re: [3/4] d3d9: Remove the double unlock test
On 17 March 2010 11:56, Stefan Dösinger ste...@codeweavers.com wrote: Windows 7 returns a different value than the test expects, and since its a todo_wine it seems no game depends on it. http://bugs.winehq.org/show_bug.cgi?id=19862 depends on that.
Re: [3/4] jscript: Fix various memory and reference count leaks.
2009/12/30 Jacek Caban ja...@codeweavers.com: Hi Rob, Thanks for your work on jscript. Unfortunately fixing Valgrind on jscript is not going to be easy. Most of your fixes are good, except clear_global part and jscript: Fix a circular reference caused by object and function objects being able to have properties that are objects and functions. patch. I've sent a test showing that Windows doesn't do that. I've resent the memory leak patch without the clear_global part. To fix it properly we need to implement a garbage collector. I've designed it in the very early days of jscript, but I never had time to implement it. That sounds like an excellent piece of work for someone interested in algorithms as opposed to the usual bug fixing. I don't think I have the time to follow through on this myself at the moment. I can see how this would work for solving the general problem of circular reference counts in jscript. However, I don't see how this helps for managing the lifetime of global objects. It seems that either the global constructors have state in which case they need to be around for as long as the script context is, while not holding a reference to the script context to avoid preventing it from dying, or they don't have any state in which case they can be constructed lazily and destroyed when the last reference is removed. Thanks, -- Rob Shearman
Re: [3/4] jscript: Fix various memory and reference count leaks.
On 12/31/09 1:17 PM, Rob Shearman wrote: 2009/12/30 Jacek Cabanja...@codeweavers.com: Hi Rob, Thanks for your work on jscript. Unfortunately fixing Valgrind on jscript is not going to be easy. Most of your fixes are good, except clear_global part and jscript: Fix a circular reference caused by object and function objects being able to have properties that are objects and functions. patch. I've sent a test showing that Windows doesn't do that. I've resent the memory leak patch without the clear_global part. Thanks. To fix it properly we need to implement a garbage collector. I've designed it in the very early days of jscript, but I never had time to implement it. That sounds like an excellent piece of work for someone interested in algorithms as opposed to the usual bug fixing. I don't think I have the time to follow through on this myself at the moment. I can see how this would work for solving the general problem of circular reference counts in jscript. However, I don't see how this helps for managing the lifetime of global objects. It seems that either the global constructors have state in which case they need to be around for as long as the script context is, while not holding a reference to the script context to avoid preventing it from dying, or they don't have any state in which case they can be constructed lazily and destroyed when the last reference is removed. The general idea is that script_ctx_t structure is treated just like other objects by garbage collectors. This way, if there are no references stored outside script engine, as soon as engine is closed, it will be released because all its references will be circular. script_ctx_t will have to be treated a bit differently than regular objects in practice, but it's an implementation detail. Thanks, Jacek
Re: [3/4] jscript: Fix various memory and reference count leaks.
Hi Rob, Thanks for your work on jscript. Unfortunately fixing Valgrind on jscript is not going to be easy. Most of your fixes are good, except clear_global part and jscript: Fix a circular reference caused by object and function objects being able to have properties that are objects and functions. patch. I've sent a test showing that Windows doesn't do that. To fix it properly we need to implement a garbage collector. I've designed it in the very early days of jscript, but I never had time to implement it. Thanks, Jacek
Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
2009/12/28 Matteo Bruni matteo.myst...@gmail.com:
Why do you need the fake parser? Can't you just not support those
shader versions yet? There's also (in general) not much of a point in
adding structure fields that aren't used yet.
+/* This file needs the original d3d9 definitions. The bwriter ones
+ * aren't useable because they are wine-internal things. We're writing
+ * d3d8/9 shaders here, so we need the d3d9 definitions (which are
+ * equal to the d3d8 ones)
+ */
This doesn't seem to match what the code actually does.
+/* Debug utility routines. Some are not reentrant, check asmutils.c */
Same as above.
+const char *debug_print_dstreg(const struct shader_reg *reg, shader_type st)
{
+return wine_dbg_sprintf(%s, get_regname(reg, st));
+}
+
+const char *debug_print_srcreg(const struct shader_reg *reg, shader_type st)
{
+switch(reg-srcmod) {
+case BWRITERSPSM_NONE:
+return wine_dbg_sprintf(%s, get_regname(reg, st));
+}
+return Unknown modifier;
+}
return get_regname(reg, st); should work at least as well.
+/* Mutex used to guarantee a single invocation
+ of the D3DXAssembleShader function (or its variants) at a time.
+ This is needed as wpp isn't thread-safe */
+extern CRITICAL_SECTION wpp_mutex;
It's probably easier to just statically initialize the critical
section in shader.c.
As for splitting things up, I think it's ok to e.g. add the
pre-processor first, and just return E_NOTIMPL from assemble_shader().
Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
Matteo Bruni matteo.myst...@gmail.com writes: + +%option reentrant bison-bridge These won't work on old flex versions, and will get you in trouble with the flex police (aka Michael Stefaniuc ;-) -- Alexandre Julliard julli...@winehq.org
Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
2009/12/29 Henri Verbeet hverb...@gmail.com: 2009/12/28 Matteo Bruni matteo.myst...@gmail.com: Why do you need the fake parser? Can't you just not support those shader versions yet? There's also (in general) not much of a point in adding structure fields that aren't used yet. I added the fake parser for two reasons: because I have to initialize an asm_parser structure anyway in the create__parser functions (and I prefer to use parser_fake for the unimplemented shader versions instead of parser_vs_3) and I want to avoid having some tests for the other shader versions pass by chance inside the todo_wine (this is accomplished by the asmparser_end_fake function... probably this can be seen as a hack). Agreed on the structure fields. +/* This file needs the original d3d9 definitions. The bwriter ones + * aren't useable because they are wine-internal things. We're writing + * d3d8/9 shaders here, so we need the d3d9 definitions (which are + * equal to the d3d8 ones) + */ This doesn't seem to match what the code actually does. That #include is only used from the next patch. I'll move the include and the comment in the right place (maybe rephrasing it somewhat). +/* Debug utility routines. Some are not reentrant, check asmutils.c */ Same as above. Yep, that's not true anymore, courtesy of wine_dbg_sprintf. As for splitting things up, I think it's ok to e.g. add the pre-processor first, and just return E_NOTIMPL from assemble_shader(). You mean a patch which adds only the first half of the D3DXAssembleShader implementation (returning just after the preprocessing)? That seems reasonable. Btw, ok for your other points also.
Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
2009/12/29 Alexandre Julliard julli...@winehq.org: Matteo Bruni matteo.myst...@gmail.com writes: + +%option reentrant bison-bridge These won't work on old flex versions, and will get you in trouble with the flex police (aka Michael Stefaniuc ;-) Alexandre, now you have another person on your side against RHEL 5 and old flex versions :) Joking aside, that's not a big issue as wpp is not reentrant anyway and I'm using the mutex to allow a single call at a time to D3DXAssembleShader. Just a question: as now I have to keep the parser context in a global structure defined in asmshader.y, how should I give access to it from asmshader.l? Do you prefer to give visibility to the global struct from asmshader.l or to create a getter (like a struct asm_parser *asm_get_context(void) function)?
Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
2009/12/29 Matteo Bruni matteo.myst...@gmail.com: I added the fake parser for two reasons: because I have to initialize an asm_parser structure anyway in the create__parser functions (and I prefer to use parser_fake for the unimplemented shader versions instead of parser_vs_3) and I want to avoid having some tests for the other shader versions pass by chance inside the todo_wine (this is accomplished by the asmparser_end_fake function... probably this can be seen as a hack). Agreed on the structure fields. Can't you just let the parser fail when it encounters an unsupported shader version? That's more or less what happens anyway, but I don't think you have to wait until after parsing all the instructions for that.
Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
On 12/29/2009 04:54 PM, Matteo Bruni wrote: 2009/12/29 Alexandre Julliardjulli...@winehq.org: Matteo Brunimatteo.myst...@gmail.com writes: + +%option reentrant bison-bridge These won't work on old flex versions, and will get you in trouble with the flex police (aka Michael Stefaniuc ;-) All that because the Wine maintainer puts a lot of emphasis on portability ... Alexandre, now you have another person on your side against RHEL 5 and old flex versions :) I have just submitted a patch for configure.ac to require a newer flex version to build Wine. People on RHEL5 can just rebuild a SRPM from Fedora (those from F7 to F10 are just a rpmbuild --rebuild). Now the Wine maintainer needs to just accept that patch ;) bye michael
Re: [3/4] d3dx9: Implement D3DXAssembleShader function, really basic shader assembler.
2009/12/29 Henri Verbeet hverb...@gmail.com: 2009/12/29 Matteo Bruni matteo.myst...@gmail.com: I added the fake parser for two reasons: because I have to initialize an asm_parser structure anyway in the create__parser functions (and I prefer to use parser_fake for the unimplemented shader versions instead of parser_vs_3) and I want to avoid having some tests for the other shader versions pass by chance inside the todo_wine (this is accomplished by the asmparser_end_fake function... probably this can be seen as a hack). Agreed on the structure fields. Can't you just let the parser fail when it encounters an unsupported shader version? That's more or less what happens anyway, but I don't think you have to wait until after parsing all the instructions for that. Yep, you are right. I believe previously I had found no way to halt the parser early, so I simply let it go until the end and I needed the fake backend to cope with that. Seems like the YYABORT macro does just what I need...
Re: [3/4] user32/cursoricon.c: Multiple Fixes/Improvements for DrawIconEx
Hi Wilfried, minor comment on this patch: hBitTemp = SelectObject( hMemDC, hXorBits ); + +UINT bltFlag = 0; C89 doesn't allow you to mix code and declarations like this. You'll have to declare bltFlag at the top of the block. --Juan
Re: [3/4] user32/cursoricon.c: Multiple Fixes/Improvements for DrawIconEx
Wilfried Pasquazzo wrote: Best regards, Subject: Make DrawIconEx 2 additional tests (now passes all tests) Subject: Make DrawIconEx pass a test You should really combine these 2 patches. You are modifying the same exact code and breaking it between patches (for some programs). Vitaliy.
Re: [3/4] WineD3D: Perform the srgb copy with fbo_blit if possible
2009/9/10 Stefan Dösinger ste...@codeweavers.com:
+if (context-current_fbo) {
+context_bind_fbo(context, GL_FRAMEBUFFER_EXT,
context-current_fbo-id);
+} else {
+context_bind_fbo(context, GL_FRAMEBUFFER_EXT, NULL);
+}
This shouldn't be needed.
+/* Need FBOs obviously */
+if(wined3d_settings.offscreen_rendering_mode != ORM_FBO) return FALSE;
+/* Need framebuffer blit */
+if(!GL_SUPPORT(EXT_FRAMEBUFFER_BLIT)) return FALSE;
+/* Need to be able to attach both RGB and sRGB copy */
+if((fmt_flags attach_flags) != attach_flags) return FALSE;
+/* Need to be able to blit from and to my format */
+if(!surface_can_stretch_rect(This, This)) return FALSE;
+/* Need either RGB or sRGB copy up to date */
+if((This-Flags (SFLAG_INSRGBTEX | SFLAG_INTEXTURE)) == 0) return
FALSE;That's redundant,
Some of those are redundant. A format is never FBO attachable if FBOs
are disabled, surface_can_stretch_rect() will already check for
regular attachment.
Re: [3/4] d3d10: Implement ID3D10Effect::GetVariableByIndex.
2009/8/21 Rico Schüller kgbric...@web.de:
+if( index This-local_buffer_variable_count )
Please use consistent style.
+for (i = 0; i This-local_buffer_count; ++i)
+{
+struct d3d10_effect_local_buffer *l = This-local_buffers[i];
+unsigned int j;
+
+for (j = 0; j l-variable_count; ++j)
+{
+struct d3d10_effect_variable *v = l-variables[j];
+
+if (nr == index)
+{
+TRACE(Returning variable %p.\n, v);
+return (ID3D10EffectVariable *)v;
+}
+nr++;
+}
+}
This is unnecessary, you can just keep subtracting l-variable_count
from index until l-variable_count index, at which point index
should have the right value to index l-variables.
Re: [3/4]winemaker: add workspace-parse function
This one is wrapped also (which is only logical). There are two other problems: * there's the same case sensitivity issue as in the project parsing code. Using search_from() in the same way should fix it. * also it assumes that the project file is in the current directory. In other words, the following does not work: winemaker Chap03/HelloWin/HelloWin.dsw -- Francois Gouget fgou...@free.fr http://fgouget.free.fr/ Demander si un ordinateur peut penser revient à demander si un sous-marin peut nager.
Re: [3/4]
2008/12/8 Stefan Dösinger [EMAIL PROTECTED]: Does the patch have any other advantages besides making the fixed function state handlers somewhat simpler though? I'm not sure if introducing a dependency on the state tracker in the shader backend is worth this. (I've got a better way to avoid redundant constant loads). The double loading fix was the main intention The other thing I intend with the patch is to move the GLSL shader-constant dependency checks to the GLSL code where it belongs The patch also starts to untie vertex and pixel shaders where we can(read ARB). I think it is wrong that we tie vertex and pixel shaders together in the shader backend interface because one implementation needs it.(On a hypothetical point, we need this for a nvts or atifs shader backend(*)). This might conflict with your intentions with the patch you mentioned above though, in that case we should not do that. The patch essentially adds a version to the constants and throws them in a queue. The disadvantage is that setting a constant becomes a bit more expensive, and cache usage becomes a bit worse if you have to load constants from deeper in the tree, but it still gives me about 10 extra fps in the CSS stress test. I don't see how setting vertex and fragment shaders at the same time hurts us that much, it just gives the shader backend a certain degree of freedom. Please also keep issue 20 from ARB_geometry_shader4 in mind. While it's possible to do crazy stuff like mixing ARB_vertex_program with fragment shaders, you can't mix ARB_vertex_program with a geometry shader. On the other hand, if you really wanted to do that kind of mixing, you could just create two backends and always pass FALSE for usePS to one backend, and always pass FALSE for useVS to the other. It's not impossible, just ugly. Splitting the shader backend doesn't change anything there. That it might not be trivial isn't really an argument against it. Either way, this is pretty much a separate issue from the original patch. The thing is just that this topic comes back every other patch, and I feel like running in a cicrcle having the same arguments over that point again and again... I do have an opinion on how the shader backend should fit in with the rest of the code. So far, I haven't heard anything that would make me change that opinion.
RE: [3/4]
The patch essentially adds a version to the constants and throws them in a queue. The disadvantage is that setting a constant becomes a bit more expensive, and cache usage becomes a bit worse if you have to load constants from deeper in the tree, but it still gives me about 10 extra fps in the CSS stress test. Whoa, that's an impressive improvement! I didn't see that much room for improvement in the CS:S performance at all. How expensive is a shader_glsl_load_constant call if no constant was changed? I assume that no GL call is made in that case, I don't see how setting vertex and fragment shaders at the same time hurts us that much, it just gives the shader backend a certain degree of freedom. I'd argue that my patch gives the shader backend more freedom. A backend implementation is free to use the same function for vertexshader_select and fragmentshader_select, as we use the same prototype. All it has to do(or better, should do) is checking wether it will be called again via a isStateDirty check. However, this check is done in the current code already, just outside of the shader backend(ie, it is forced onto the backend even if it is not needed) Please also keep issue 20 from ARB_geometry_shader4 in mind. While it's possible to do crazy stuff like mixing ARB_vertex_program with fragment shaders, you can't mix ARB_vertex_program with a geometry shader. I don't care too much about mixing ARB with GLSL. It might be a neat trick for MacOS, but it doesn't work in the way we need it in the current architecture anyway. To work around those MacOS GLSL bugs we'd have to select the backend based on the concrete shader. Having a ARBvp+GLSLfrag shader backend selected at library load isn't any better than what we have now in terms of MacOS bugdodgeability. On the other hand, if you really wanted to do that kind of mixing, you could just create two backends and always pass FALSE for usePS to one backend, and always pass FALSE for useVS to the other. It's not impossible, just ugly. Splitting the shader backend doesn't change anything there. I'll come back to this should I ever send in an ATIfs or NVTS shader backend :-) . That solution is fine with me. I do have an opinion on how the shader backend should fit in with the rest of the code. So far, I haven't heard anything that would make me change that opinion. I think we have a different understanding of what the gobal state table infrastructure is supposed to be. I didn't intend it as a fixed function state table, but rather a tool for managing all sorts of states. It includes things like blending, the scissor test or even the index buffer, which are not affected by shaders at all(since they're neither part of the vertex, fragment or geometry pipeline). So I don't think limiting the state table to the fixed function pipeline would be correct, even if shaders sat on top of it. (yes, there are many fixed function states and just a few shader states, but that's due to the nature of the two pipelines) I don't think that the vertex shader bypasses the fixed function vertex pipeline argument works here either. We have such a relationship between states elsewhere, e.g. D3DRS_LIGHTING overrides the lights, D3DRS_ALPHABLENDENABLE overrides various alpha states etc. I'm not convinced that shaders are special in any way. There's also the point about skipping applying fixed function states when the shader is active. I really think this should be up to the fragment pipeline implementation. The ARB fragment pipeline replacement skips the states(because it has to), the fixed function GL pipeline doesn't. Functionally it doesn't make any difference for the ffp pipeline. Performance wise you can argue both. If we apply the states it makes switching the shader on and off cheaper, if we do not apply them it helps broken games.
Re: [3/4]
2008/12/8 Stefan Dösinger [EMAIL PROTECTED]:
+/* TODO: If we're called by shader_glsl_select, we know that both we get
past both checks.
+ * is it cheaper to have a wrapper function check those? Inline some
common code? Extra
+ * parameter?
+ */
if (!prog) {
/* No GLSL program set - nothing to do. */
return;
+} else if(isStateDirty(context, STATE_PIXELSHADER) ||
+ isStateDirty(context, STATE_VDECL) ||
+ isStateDirty(context, STATE_PIXELSHADERCONSTANT) ||
+ isStateDirty(context, STATE_VERTEXSHADERCONSTANT)) {
+/* Will be called again later */
+return;
If you're going to introduce a dependency on state management in the
shader backends anyway, you don't need to export constant loading
functions from the shader backends at all. You might as well just call
shader_select(), and let that figure out if it needs to change the
shader or load any constants. That would also avoid this not-so-pretty
code here.
Re: [3/4]
2008/12/8 Stefan Dösinger [EMAIL PROTECTED]: I don't think that would work well. We would need some shader model specific private data in each context(e.g. last vertexshader and last vertexdeclaration) to allow the shader backend to find out what to do, since the dirty state information won't suffice if the shader backend doesn't know where it was called from. Why not? How does e.g. ARB find out wether it has to reapply the pixelshader? Or how does GLSL find out if it has to apply a new GLSL program. I would expect the relevant state to be dirty, but you say this depends on where the function is called from? I would argue that since shaders replace vertex and/or fragment states, we shouldn't have to go through the state management for replaced pipeline stages in the first place. And I argue that this should be the implementation's decision. (Read: The state manager and context.c should not treat some states specially) Well, you can hardly argue that eg. vertex shader enable is a proper vertex state to begin with.
Re: [3/4]
2008/12/8 Stefan Dösinger [EMAIL PROTECTED]: If you're going to introduce a dependency on state management in the shader backends anyway, you don't need to export constant loading functions from the shader backends at all. You might as well just call shader_select(), and let that figure out if it needs to change the shader or load any constants. That would also avoid this not-so-pretty code here. I don't think that would work well. We would need some shader model specific private data in each context(e.g. last vertexshader and last vertexdeclaration) to allow the shader backend to find out what to do, since the dirty state information won't suffice if the shader backend doesn't know where it was called from. Why not? It would also tie shader and shader constants, as well as different shader types and constant types together in ARB. That's an issue of course, but we should only reload dirty constants anyway. I've got some patches I could cleanup that do this. There are however two follow-up ideas of this patch I am toying with: 1: Make the load_constant and shader_select prototypes the same, so GLSL can use one call for all and ARB remains flexible 2: Make the shader backend provide a pipeline template instead of shader_select and shader_load_constant. That would move shader selection out of the fixed function pipeline code and allow the shader to use the state management linking facilities for these sorts of dependencies I would argue that since shaders replace vertex and/or fragment states, we shouldn't have to go through the state management for replaced pipeline stages in the first place.
RE: [3/4]
If you're going to introduce a dependency on state management in the shader backends anyway, you don't need to export constant loading functions from the shader backends at all. You might as well just call shader_select(), and let that figure out if it needs to change the shader or load any constants. That would also avoid this not-so-pretty code here. I don't think that would work well. We would need some shader model specific private data in each context(e.g. last vertexshader and last vertexdeclaration) to allow the shader backend to find out what to do, since the dirty state information won't suffice if the shader backend doesn't know where it was called from. It would also tie shader and shader constants, as well as different shader types and constant types together in ARB. There are however two follow-up ideas of this patch I am toying with: 1: Make the load_constant and shader_select prototypes the same, so GLSL can use one call for all and ARB remains flexible 2: Make the shader backend provide a pipeline template instead of shader_select and shader_load_constant. That would move shader selection out of the fixed function pipeline code and allow the shader to use the state management linking facilities for these sorts of dependencies
RE: [3/4]
I don't think that would work well. We would need some shader model specific private data in each context(e.g. last vertexshader and last vertexdeclaration) to allow the shader backend to find out what to do, since the dirty state information won't suffice if the shader backend doesn't know where it was called from. Why not? How does e.g. ARB find out wether it has to reapply the pixelshader? Or how does GLSL find out if it has to apply a new GLSL program. I would argue that since shaders replace vertex and/or fragment states, we shouldn't have to go through the state management for replaced pipeline stages in the first place. And I argue that this should be the implementation's decision. (Read: The state manager and context.c should not treat some states specially)
RE: [3/4]
I would expect the relevant state to be dirty, but you say this
depends on where the function is called from?
States are marked clean before the application function is called. This is
needed to handle e.g. the relationship between the vertex declaration and the
lighting enable state.
What I mean is this:
pixelshader(bla bla bla) {
/* I know I have to apply the pixelshader */
shader_backend-shader_select();
}
pixelshader(bla bla bla) {
/* I know I have to apply the vertex shader */
shader_backend-shader_select();
}
shader_whatever_select() {
/* What do I have to do? Apply pixel shader? Or the vertex shader? or
both? */
}
Well, you can hardly argue that eg. vertex shader enable is a proper
vertex state to begin with.
The vertex shader enable doesn't have any say without the vertex declaration.
And the vertex declaration has a similar effect on e.g. D3DRS_FOGVERTEXMODE.
This state is treated differently in different shader model versions and
different GL pipeline implementations on the fragment side. Is FOGVERTEXMODE a
proper state or not?
What I want to say is: We open a can of worms if we try to separate first
class(programmable/ffp switch) states and second class states(ffp states) in
the general state manager. This should really be left up to the actual pipeline
implementation.
(sidenote: The current fog code is a mess)
Re: [3/4]
2008/12/8 Stefan Dösinger [EMAIL PROTECTED]:
I would expect the relevant state to be dirty, but you say this
depends on where the function is called from?
States are marked clean before the application function is called. This is
needed to handle e.g. the relationship between the vertex declaration and the
lighting enable state.
What I mean is this:
pixelshader(bla bla bla) {
/* I know I have to apply the pixelshader */
shader_backend-shader_select();
}
pixelshader(bla bla bla) {
/* I know I have to apply the vertex shader */
shader_backend-shader_select();
}
shader_whatever_select() {
/* What do I have to do? Apply pixel shader? Or the vertex shader? or
both? */
}
It would be up to the shader backend to determine what to apply. That
might imply storing the current vertex and fragment shader id's in the
context, but considering we also store those in the d3d vertex and
pixel shaders I think that's acceptable for now.
Does the patch have any other advantages besides making the fixed
function state handlers somewhat simpler though? I'm not sure if
introducing a dependency on the state tracker in the shader backend is
worth this. (I've got a better way to avoid redundant constant loads).
Well, you can hardly argue that eg. vertex shader enable is a proper
vertex state to begin with.
The vertex shader enable doesn't have any say without the vertex declaration.
And the vertex declaration has a similar effect on e.g. D3DRS_FOGVERTEXMODE.
This state is treated differently in different shader model versions and
different GL pipeline implementations on the fragment side. Is FOGVERTEXMODE
a proper state or not?
Fog is not a pure vertex or fragment state, no (neither is the vertex
declaration, strictly speaking, since a large part of what it does is
about resource loading rather than how vertices are rendered).
state_fog() makes this obvious in a rather painful way.
What I want to say is: We open a can of worms if we try to separate first
class(programmable/ffp switch) states and second class states(ffp states) in
the general state manager. This should really be left up to the actual
pipeline implementation.
That it might not be trivial isn't really an argument against it.
Either way, this is pretty much a separate issue from the original
patch.
RE: [3/4]
Does the patch have any other advantages besides making the fixed function state handlers somewhat simpler though? I'm not sure if introducing a dependency on the state tracker in the shader backend is worth this. (I've got a better way to avoid redundant constant loads). The double loading fix was the main intention The other thing I intend with the patch is to move the GLSL shader-constant dependency checks to the GLSL code where it belongs The patch also starts to untie vertex and pixel shaders where we can(read ARB). I think it is wrong that we tie vertex and pixel shaders together in the shader backend interface because one implementation needs it.(On a hypothetical point, we need this for a nvts or atifs shader backend(*)). This might conflict with your intentions with the patch you mentioned above though, in that case we should not do that. It would be up to the shader backend to determine what to apply. That might imply storing the current vertex and fragment shader id's in the context, but considering we also store those in the d3d vertex and pixel shaders I think that's acceptable for now. Agreed, I can live with it. I however think that having this instead of the dirty state checks(which aren't completely nice either) moots the point. That it might not be trivial isn't really an argument against it. Either way, this is pretty much a separate issue from the original patch. The thing is just that this topic comes back every other patch, and I feel like running in a cicrcle having the same arguments over that point again and again... (*) I may need pixel shader support on my old laptop for personal reasons in a few weeks, so maybe I invest a rainy weekend of my time into it. Otoh just installing Windows may be an easier fix
Re: [3/4] ntdll: Implement the timer queue thread.
Hi Dan,
2008/7/23 Dan Hipschman [EMAIL PROTECTED]:
This isn't the most efficient implementation, but it works, and it should
not be difficult to tweak. Hence, I'd rather get this version in and add
the optimizations one at a time, in little patches. Get it working first...
Yes, I think you're right. I don't think it will be too hard to change
the architecture of this to that of my suggestions.
However, I have one minor nit.
+static void WINAPI timer_queue_thread_proc(LPVOID p)
+{
+struct timer_queue *q = p;
+ULONG timeout_ms;
+BOOL done;
+
+timeout_ms = INFINITE;
+while (!q-quit)
+{
+LARGE_INTEGER timeout;
+DWORD ret = NtWaitForSingleObject(q-event, FALSE,
+ get_nt_timeout(timeout,
timeout_ms));
This should be NTSTATUS.
+
+if (ret == STATUS_TIMEOUT)
+queue_timer_expire_next(q);
+
+RtlEnterCriticalSection(q-cs);
+timeout_ms = timer_queue_update(q);
+RtlLeaveCriticalSection(q-cs);
+}
--
Rob Shearman
Re: [3/4] ntdll: Implement the timer queue thread.
Rob Shearman [EMAIL PROTECTED] writes: 2008/7/23 Dan Hipschman [EMAIL PROTECTED]: This isn't the most efficient implementation, but it works, and it should not be difficult to tweak. Hence, I'd rather get this version in and add the optimizations one at a time, in little patches. Get it working first... Yes, I think you're right. I don't think it will be too hard to change the architecture of this to that of my suggestions. Actually I think the code will be much simpler with a sorted timer list, so I'd suggest to start with that. -- Alexandre Julliard [EMAIL PROTECTED]
Re: [3/4] user32: Set the vkey value to VK_PROCESSKEY when IME process key.
Thank you for these investigations This is really nice. I support those
patches.
I just wanted to also communicate a bit of additional information from
my investigations just in case someone asks. I placed hooks on both
WH_KEYBOARD_LL and WH_KEYBOARD. Even when the IME is active both of
those hooks showed the VKEY of the actual keystroke.
This showed me that all of this processing happened after
process_keyboard_message.
great work and thanks,
-aric
ByeongSik Jeon wrote:
When the MS Windows set the vkey value(MSG.wParam) to VK_PROCESSKEY?
I have tested following code:
while ( GetMessage(msg, 0, 0, 0) ) {
if ( msg.message == WM_KEYDOWN )
printf(OLD: %x %x %x\n, msg.message, msg.wParam, msg.lParam);
TranslateMessage( msg );
if ( msg.message == WM_KEYDOWN )
printf(NEW: %x %x %x\n, msg.message, msg.wParam, msg.lParam);
DispatchMessage( msg );
}
-
The result is:
-
OLD: 100 e5 130001
NEW: 100 e5 130001
OLD: 100 e5 250001
NEW: 100 e5 250001
OLD: 100 e5 130001
NEW: 100 e5 130001
-
e5 is VK_PROCESSKEY.
Before GetMessage, PeekMessage return, msg.wPram ahs seted to VK_PROCESSKEY.
---
dlls/user32/message.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
Re: [3/4].Wbemprox.dll. Main modules for then library.
Ivan Sinitsin wrote: --- /dev/null 2007-03-10 18:36:24 +0300 +++ Makefile.in 2008-02-21 15:57:24 +0300 @@ -0,0 +1,23 @@ +TOPSRCDIR = @top_srcdir@ +TOPOBJDIR = ../.. +SRCDIR= @srcdir@ +VPATH = @srcdir@ +MODULE= wbemprox.dll +IMPORTLIB = libwbemprox.$(IMPLIBEXT) +IMPORTS = ole32 oleaut32 user32 advapi32 kernel32 shell32 +EXTRALIBS = -luuid + +C_SRCS = \ + main.c \ + regsvr.c \ + i_numwbemclassobject.c \ + i_wbemservices.c \ + mainclass.c \ + factory.c The files i_numwbemclassobject.c and i_wbemservices.c aren't included with this patch, so Wine would not compile if this patch was applied. Please also reconsider the naming of these files - I presume i_ standards for interface, but you aren't declaring an interface in these files but implementing an object. + + +IDL_I_SRCS = wbemprox.idl + [EMAIL PROTECTED]@ + [EMAIL PROTECTED]@ # everything below this line is overwritten by make depend -- Rob Shearman
Re: [3/4] WineX11: Ignore the alpha if all pixels are 0x00
Hi Stefan, a few minor nits:
+ * one pixel has alpha != 0x00, and the mask ignored as described in the
Do you mean, the mask is ignored..., or the AND mask is ignored... ?
+ * written at 8 / 16 bpp times do not knwow about the 32 bit alpha, so
s/knwow/know/
+for (y = 0; y ymax; ++y)
+{
+xor_ptr = xor_bits + (y * xor_width_bytes);
+for (x = 0; x xmax; ++x)
+{
+if(xor_ptr[3] != 0x00) {
+alpha_zero = FALSE;
+break;
For the first for loop, you probably want something like (for y = 0;
alpha_zero y ymax...
Also, the whitespace around the if is a bit funny.
--Juan
Re: [3/4] msxml: Implement removeAttribute
A problem with this patch is that the user may have an IXMLDOMNode * reference to the attribute node being removed. The xmlFreeProp will free the xmlnode so accessing the IXMLDOMNode could result in memory corruption. Under MSXML it's possible to access such node and even reconnect it e.g. to a different documents (at least I seem to remember such a thing for element nodes). I don't think supporting it is possible with our current reference counting only for the whole document. I was thinking about also keeping reference counts for each connected component and each node. The connected component count could be used to determine if the node can be freed in such a situation and the node count could be used to update the connected component counts if the tree gets split. But so far I haven't written any code (Another possibility is to keep in each node the refcount of the whole subtree starting in this node. This is more elegant but it would take a linear time with respect to the tree height to update the refcount). Mikołaj Zalewski
Re: [3/4] msxml: Implement removeAttribute
Mikołaj Zalewski wrote: A problem with this patch is that the user may have an IXMLDOMNode * reference to the attribute node being removed. The xmlFreeProp will free the xmlnode so accessing the IXMLDOMNode could result in memory corruption. Under MSXML it's possible to access such node and even reconnect it e.g. to a different documents (at least I seem to remember such a thing for element nodes). I don't think supporting it is possible with our current reference counting only for the whole document. I was thinking about also keeping reference counts for each connected component and each node. The connected component count could be used to determine if the node can be freed in such a situation and the node count could be used to update the connected component counts if the tree gets split. But so far I haven't written any code (Another possibility is to keep in each node the refcount of the whole subtree starting in this node. This is more elegant but it would take a linear time with respect to the tree height to update the refcount). Mikołaj Zalewski Thanks for pointing this out. Ill write some tests, and retry again shortly. Alistair Leslie-Hughes
Re: 3/4 setupapi: Correctly handle an empty filename in SetupGetSourceFileLocationA.
On 5/18/07, Hans Leidekker [EMAIL PROTECTED] wrote: -Hans Changelog Correctly handle an empty filename in SetupGetSourceFileLocationA. Please add a test case. -- James Hawkins
Re: 3/4 setupapi: Correctly handle an empty filename in SetupGetSourceFileLocationA.
On Friday 18 May 2007 11:43:29 James Hawkins wrote: Correctly handle an empty filename in SetupGetSourceFileLocationA. Please add a test case. The BITS installer was enough of a test case for me here, but since you ask, I'll see what I can do. -Hans
Re: [3/4] WineD3D: Store the pixel format in the texture
Shouldn't this be done for volume textures as well?
Re: [3/4] WineD3D: Store the pixel format in the texture
Am Mittwoch 21 März 2007 17:53 schrieb H. Verbeet: Shouldn't this be done for volume textures as well? It is done, unless I missed something: @@ -896,6 +898,7 @@ static HRESULT WINAPI IWineD3DDeviceImpl_CreateVolumeTexture(IWineD3DDevice *ifa object-width = Width; object-height = Height; object-depth = Depth; +object-baseTexture.format = Format; pgpi2Ng96IsE8.pgp Description: PGP signature
Re: [3/4] WineD3D: Blit the offscreen texture into the drawable if needed
Am Mittwoch 07 März 2007 02:43 schrieb Stefan Dösinger: Do not apply this patch yet. Cube textures aren't that easy pgpJyUMrUXycs.pgp Description: PGP signature
Re: [3/4 revised] msi: Add partial, expandable OLE automation support.
On 2/25/07, Misha Koshelev [EMAIL PROTECTED] wrote:
This patch adds OLE automation support to MSI. It does this by creating
a wrapper class, which implements any of the OLE automation classes as
long as a function is given which is called from within the wrapper
classes invoke method after appropriate error checking. Basic
functionality is implemented, and it is fairly straightforward to add
the rest as the methods are all already implemented as functions in the
DLL.
+/*
+ * If you would like to implement a new automation function/object,
look towards the bottom of this
+ * file for the meat and potatoes section.
+ */
+
+/* FIXME: I don't know how big this should be */
+#define MAX_MSI_STRING 1000
+
+/*
+ * AutomationObject - base class for all automation objects so we
don't have to repeat functions. Just
+ *need to implement Invoke function for each
dispinterface that is called from within
+ *the AutomationObject Invoke function which
performs error checking, and pass the new
+ *function to create_automation_object.
+ */
+
+typedef interface AutomationObject AutomationObject;
+
+interface AutomationObject {
+/*
+ * VTables - We provide IDispatch, IProvideClassInfo,
IProvideClassInfo2, IProvideMultipleClassInfo
+ */
+const IDispatchVtbl *lpVtbl;
+const IProvideClassInfoVtbl *lpvtblIProvideClassInfo;
+const IProvideClassInfo2Vtbl *lpvtblIProvideClassInfo2;
+const IProvideMultipleClassInfoVtbl *lpvtblIProvideMultipleClassInfo;
+
+/* Object reference count */
+LONG ref;
+
+/* Clsid for this class and it's appropriate ITypeInfo object */
+LPCLSID clsid;
+ITypeInfo *iTypeInfo;
+
+/* The MSI handle of the current object */
+MSIHANDLE msiHandle;
+
+/* A function that is called from IDispatch::Invoke, specific to
this type of object. By the
+ * time this function is called, basic error checking has been
done in the AutomationObject
+ * Invoke function */
+HRESULT (STDMETHODCALLTYPE *funcInvoke)(
+AutomationObject* This,
+DISPID dispIdMember,
+REFIID riid,
+LCID lcid,
+WORD wFlags,
+DISPPARAMS* pDispParams,
+VARIANT* pVarResult,
+EXCEPINFO* pExcepInfo,
+UINT* puArgErr);
+};
You went a little comment crazy here (and elsewhere). You should tone
it down a bit. Good code explains itself and requires little or no
commenting.
+/*
+ * Perform a sanity check on the parameters.
+ */
+if ( (This==0) || (ppvObject==0) )
+ return E_INVALIDARG;
+
+/*
+ * Initialize the return parameter.
+ */
+*ppvObject = 0;
+
+/*
+ * Compare the riid with the interface IDs implemented by this object.
+ */
+if (IsEqualGUID(riid, IID_IUnknown) || IsEqualGUID(riid,
IID_IDispatch) || IsEqualGUID(riid, This-clsid))
+*ppvObject = This;
+else if (IsEqualGUID(riid, IID_IProvideClassInfo))
+ *ppvObject = (IProvideClassInfo*)(This-lpvtblIProvideClassInfo);
+else if (IsEqualGUID(riid, IID_IProvideClassInfo2))
+ *ppvObject = (IProvideClassInfo2*)(This-lpvtblIProvideClassInfo2);
+else if (IsEqualGUID(riid, IID_IProvideMultipleClassInfo))
+ *ppvObject =
(IProvideMultipleClassInfo*)(This-lpvtblIProvideMultipleClassInfo);
+
+/*
+ * Check that we obtained an interface.
+ */
+if ((*ppvObject)==0)
+{
+ TRACE(() : asking for unsupported interface %s\n,debugstr_guid(riid));
+ return E_NOINTERFACE;
+}
+
+/*
+ * Query Interface always increases the reference count by one when it is
+ * successful
+ */
+IClassFactory_AddRef(iface);
+
+return S_OK;
+}
Way too many comments. It may seem like nitpicking, but the comments
clutter up the code and don't provide any useful information.
--
James Hawkins
Re: 3/4 patches for compilation of ATL MFC
Boaz Harrosh [EMAIL PROTECTED] writes:
+#define _I64_MIN(-9223372036854775807i64 - 1)
+#define _I64_MAX 9223372036854775807i64
+#define _UI64_MAX 0xui64
The i64 suffix is not portable.
--- include/msvcrt/math.h 2 Sep 2003 00:58:21 - 1.1
+++ include/msvcrt/math.h 10 Mar 2004 08:36:47 -
@@ -1,12 +1,14 @@
-#ifndef __WINE_MATH_H
-#define __WINE_MATH_H
-#ifdef __cplusplus
-extern C {
-#endif
+#include ../include/math.h
This is wrong, we need a proper math.h.
--
Alexandre Julliard
[EMAIL PROTECTED]
