Re: iphlpapi: Add AllocateAndGetTcpExTableFromStack()

2013-09-04 Thread Marvin
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=1995

Your paranoid android.


=== wxppro (32 bit iphlpapi) ===
iphlpapi: unhandled exception c005 at 76D6EFE7




Re: [PATCH try3] atl110: Added new DLL.

2013-09-04 Thread Jacek Caban
On 09/03/13 13:51, Qian Hong wrote:
> On Tue, Sep 3, 2013 at 7:42 PM, Jacek Caban  wrote:
>> Not really, good catch. We should make them consistent. Honestly, I'm
>> not sure which one is better. Both have their problems. Some functions
>> are forwarded, others are not, so having one debug channel would be
>> guarantee that we don't miss some calls while debugging a bug. However,
>> some functions have the same names and are not forwarded, so one debug
>> channel would be ambiguous.
>>
>> I'm open for opinions.
> How about something like this:
>
> atl80.c:
> -BOOL WINAPI AtlAxWinInit(void)
> +BOOL WINAPI ATL80_AtlAxWinInit(void)
>
> atl80.spec:
> -42 stdcall AtlAxWinInit()
> +42 stdcall AtlAxWinInit() ATL80_AtlAxWinInit
>
> So we can always use one debug channel for all atlXX dlls, at the same
> time different exported function with the same name will generate
> different trace log.

Yes, that could work as well. I don't have strong opinion, feel free to
submit a patch with solution of your choice.

Thanks,
Jacek




Re: wininet: Don't start the next chunk if the read is satisfied.

2013-09-04 Thread Jacek Caban
Hi Hans,

On 09/04/13 09:22, Hans Leidekker wrote:
> ---
>  dlls/wininet/http.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c
> index 1832b4f..521e4aa 100644
> --- a/dlls/wininet/http.c
> +++ b/dlls/wininet/http.c
> @@ -2795,7 +2795,7 @@ static DWORD chunked_read(data_stream_t *stream, 
> http_request_t *req, BYTE *buf,
>  chunked_stream_t *chunked_stream = (chunked_stream_t*)stream;
>  DWORD read_bytes = 0, ret_read = 0, res = ERROR_SUCCESS;
>  
> -if(chunked_stream->chunk_size == ~0u) {
> +if(!chunked_stream->chunk_size || chunked_stream->chunk_size == ~0u) {
>  res = start_next_chunk(chunked_stream, req);
>  if(res != ERROR_SUCCESS)
>  return res;
> @@ -2835,7 +2835,7 @@ static DWORD chunked_read(data_stream_t *stream, 
> http_request_t *req, BYTE *buf,
>  chunked_stream->chunk_size -= read_bytes;
>  size -= read_bytes;
>  ret_read += read_bytes;
> -if(!chunked_stream->chunk_size) {
> +if(size && !chunked_stream->chunk_size) {
>  assert(read_mode != READMODE_NOBLOCK);
>  res = start_next_chunk(chunked_stream, req);
>  if(res != ERROR_SUCCESS)

This patch breaks two assumptions:
- chunk_size outside chunked_read means the end of the stream
- READMODE_SYNC should read as much data as possible

There is longer story about the way it's implemented the way it is.
Native IE in the past considered zero-length reporting data in
INTERNET_STATUS_REQUEST_COMPLETE to be an error. If you stop reading in
the end of chunk and the next chunk is the last one, then you will send
such report asynchronously. That's why we read the whole chunk only if
we can start reading the next one (and we can't if we don't want to
block). Now I think that the failure could possibly be caused by a
combination of returning zero size and some other bug and the behaviour
that I described could just hide the problem. Commit
851866e22a731141da7e3cbd2550c67c59968959 fixed a problem that could
potentially be the other one.

This means that I'm not entirely sure that we really shouldn't report
zero size data. This is illogical to begin with, but we even have tests
for that:
ok(!on_async, "Returned zero size in response to request complete\n");
I think we should try harder to have a test reporting zero size data
(this may be tricky to write one for Wine tests, but if we could just
observe such occasional notifications, that would be enough). And this
would need more testing with native IE.

This all means that intention of the patch may be right, but it needs
more work.

Thanks,
Jacek




Re: atl90: Add new DLL

2013-09-04 Thread Zhenbo Li
Sorry, I've made a typo.
This patch is for bug 34432


2013/9/4 Zhenbo Li 

> This patch could fix bug 34431, which is caused by missing atl90.dll
>
> ---
>  configure  |  1 +
>  configure.ac   |  1 +
>  dlls/atl90/Makefile.in |  7 
>  dlls/atl90/atl90.c | 93 ++**
> 
>  dlls/atl90/atl90.spec  | 52 
>  include/atlbase.h  |  1 +
>  tools/make_specfiles   |  1 +
>  7 files changed, 156 insertions(+)
>



-- 
Have a nice day!
Zhenbo Li



Re: iphlpapi: Add AllocateAndGetTcpExTableFromStack()

2013-09-04 Thread Bruno Jesus
On Wed, Sep 4, 2013 at 8:02 AM, Ralf Habacker  wrote:
>
> - added test case (update)
>
> Signed-off-by: Ralf Habacker 
> ---
>  dlls/iphlpapi/iphlpapi.spec|1 +
>  dlls/iphlpapi/ipstats.c|   33 +
>  dlls/iphlpapi/tests/iphlpapi.c |   32 
>  3 Dateien geändert, 66 Zeilen hinzugefügt(+)

> +// WinXP SP3 crashes on this test
> +//ret = pAllocateAndGetTcpExTableFromStack( 0, FALSE, GetProcessHeap(), 
> HEAP_ZERO_MEMORY, AF_INET );
> +//ok( ret == ERROR_INVALID_PARAMETER, "got %u\n", ret );

C++ comments are not allowed. You could use if(0) {} to make sure the
code compiles.

You are still missing the (try X) at the end of the email subject.

Regards,
Bruno




Re: wininet: Don't start the next chunk if the read is satisfied.

2013-09-04 Thread Hans Leidekker
On Wed, 2013-09-04 at 13:11 +0200, Jacek Caban wrote:
> On 09/04/13 09:22, Hans Leidekker wrote:
> > ---
> >  dlls/wininet/http.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c
> > index 1832b4f..521e4aa 100644
> > --- a/dlls/wininet/http.c
> > +++ b/dlls/wininet/http.c
> > @@ -2795,7 +2795,7 @@ static DWORD chunked_read(data_stream_t *stream, 
> > http_request_t *req, BYTE *buf,
> >  chunked_stream_t *chunked_stream = (chunked_stream_t*)stream;
> >  DWORD read_bytes = 0, ret_read = 0, res = ERROR_SUCCESS;
> >  
> > -if(chunked_stream->chunk_size == ~0u) {
> > +if(!chunked_stream->chunk_size || chunked_stream->chunk_size == ~0u) {
> >  res = start_next_chunk(chunked_stream, req);
> >  if(res != ERROR_SUCCESS)
> >  return res;
> > @@ -2835,7 +2835,7 @@ static DWORD chunked_read(data_stream_t *stream, 
> > http_request_t *req, BYTE *buf,
> >  chunked_stream->chunk_size -= read_bytes;
> >  size -= read_bytes;
> >  ret_read += read_bytes;
> > -if(!chunked_stream->chunk_size) {
> > +if(size && !chunked_stream->chunk_size) {
> >  assert(read_mode != READMODE_NOBLOCK);
> >  res = start_next_chunk(chunked_stream, req);
> >  if(res != ERROR_SUCCESS)
> 
> This patch breaks two assumptions:
> - chunk_size outside chunked_read means the end of the stream

That's strange, considering that there could always be another chunk. We can't
know if we're at the end of the stream in chunked mode, so I wonder if we should
always try the next chunk (and block) if we're being asked to read more than the
current chunk gives us.

The bug I'm seeing is when the current chunk is exactly the size of the read
request (or has that many bytes left). In that case the current code still tries
to start the next chunk and potentially blocks.

> - READMODE_SYNC should read as much data as possible

But no more than asked for? I don't see how my patch breaks this assumption.






Re: wininet: Don't start the next chunk if the read is satisfied.

2013-09-04 Thread Jacek Caban
On 09/04/13 14:38, Hans Leidekker wrote:
> On Wed, 2013-09-04 at 13:11 +0200, Jacek Caban wrote:
>> On 09/04/13 09:22, Hans Leidekker wrote:
>>> ---
>>>  dlls/wininet/http.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c
>>> index 1832b4f..521e4aa 100644
>>> --- a/dlls/wininet/http.c
>>> +++ b/dlls/wininet/http.c
>>> @@ -2795,7 +2795,7 @@ static DWORD chunked_read(data_stream_t *stream, 
>>> http_request_t *req, BYTE *buf,
>>>  chunked_stream_t *chunked_stream = (chunked_stream_t*)stream;
>>>  DWORD read_bytes = 0, ret_read = 0, res = ERROR_SUCCESS;
>>>  
>>> -if(chunked_stream->chunk_size == ~0u) {
>>> +if(!chunked_stream->chunk_size || chunked_stream->chunk_size == ~0u) {
>>>  res = start_next_chunk(chunked_stream, req);
>>>  if(res != ERROR_SUCCESS)
>>>  return res;
>>> @@ -2835,7 +2835,7 @@ static DWORD chunked_read(data_stream_t *stream, 
>>> http_request_t *req, BYTE *buf,
>>>  chunked_stream->chunk_size -= read_bytes;
>>>  size -= read_bytes;
>>>  ret_read += read_bytes;
>>> -if(!chunked_stream->chunk_size) {
>>> +if(size && !chunked_stream->chunk_size) {
>>>  assert(read_mode != READMODE_NOBLOCK);
>>>  res = start_next_chunk(chunked_stream, req);
>>>  if(res != ERROR_SUCCESS)
>> This patch breaks two assumptions:
>> - chunk_size outside chunked_read means the end of the stream

Ops, I meant to write chunk_size==0, which means the end of the stream.

> That's strange, considering that there could always be another chunk. We can't
> know if we're at the end of the stream in chunked mode, so I wonder if we 
> should
> always try the next chunk (and block) if we're being asked to read more than 
> the
> current chunk gives us.
> The bug I'm seeing is when the current chunk is exactly the size of the read
> request (or has that many bytes left). In that case the current code still 
> tries
> to start the next chunk and potentially blocks.

See chunked_end_of_data. It will return TRUE after the call you
mentioned, no matter if there are more chunks to read.

Obviously we could change chunked_end_of_data and the whole assumptions,
but we need better understanding of desired behaviour here. Consider
this sequence:

- chunked_read reads the whole chunk
- app calls InternetQueryDataAvailable, which has no data buffered, so
it schedules async read
- async read reads zero-size chunk (indicating the end of the stream)
- we call INTERNET_REQUEST_COMPLETE, reporting 0 bytes to read

The last call, as I explained previously, has been problematic in the
past and according to all tests we have, it never happens with navitve
wininet (possibly we simply need more testing to see this happening).
This needs to be investigated, otherwise we risk breaking native IE.

>> - READMODE_SYNC should read as much data as possible
> But no more than asked for? I don't see how my patch breaks this assumption.

You're right, I misread that part, sorry.


Jacek




Re: [2/2] rpcrt4: Clear the connection pool when closing an http connection.

2013-09-04 Thread Jacek Caban
On 09/04/13 15:36, Hans Leidekker wrote:
> On Wed, 2013-09-04 at 15:28 +0200, Jacek Caban wrote:
>> On 09/04/13 15:01, Hans Leidekker wrote:
>>> ---
>>>  dlls/rpcrt4/rpc_transport.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/dlls/rpcrt4/rpc_transport.c b/dlls/rpcrt4/rpc_transport.c
>>> index d0857fa..1eb0151 100644
>>> --- a/dlls/rpcrt4/rpc_transport.c
>>> +++ b/dlls/rpcrt4/rpc_transport.c
>>> @@ -3234,6 +3234,8 @@ static int rpcrt4_ncacn_http_close(RpcConnection 
>>> *Connection)
>>>HeapFree(GetProcessHeap(), 0, httpc->servername);
>>>httpc->servername = NULL;
>>>  
>>> +  /* don't allow this connection to be reused */
>>> +  InternetSetOptionW(NULL, INTERNET_OPTION_SETTINGS_CHANGED, NULL, 0);
>> There must be a better way to do that than removing all existing wininet
>> cached connections to all hosts. Removing INTERNET_FLAG_KEEP_CONNECTION
>> from HttpOpenRequest call would be a good start...
> We can't do that, it breaks NTLM and Negotiate.

Hmm, right. Thinking more about this, I don't see how this connection
could be reused. RPC over HTTP shouldn't reach the end of the stream, so
it can't be reused once the connection is established, even if client
closes the handle. If it is, that's probably a bug in wininet. When did
you see this to happen?

Jacek




Re: [PATCH 1/3] riched20: Use codepage in ME_ToUnicode. (try 2)

2013-09-04 Thread Nikolay Sivov

On 9/4/2013 07:17, Jactry Zeng wrote:

-ME_EndToUnicode(unicode, wszText);
+ME_EndToUnicode(unicode ? 1200 : CP_ACP, wszText);
It's still ugly to use magic numbers like that, in a similar situation I 
had some time

ago I use ~0 to mark WCHAR data encoding that does need special handling.
Even with a value like ~0 it needs to be documented in code comments.




Re: [PATCH] opencl/tests: Add initial tests

2013-09-04 Thread Marvin
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=2003

Your paranoid android.


=== build (build) ===
Unable to regenerate dlls/opencl/Makefile
Recreation of tests/Makefile failed




Re: [2/2] rpcrt4: Clear the connection pool when closing an http connection.

2013-09-04 Thread Hans Leidekker
On Wed, 2013-09-04 at 15:28 +0200, Jacek Caban wrote:
> On 09/04/13 15:01, Hans Leidekker wrote:
> > ---
> >  dlls/rpcrt4/rpc_transport.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/dlls/rpcrt4/rpc_transport.c b/dlls/rpcrt4/rpc_transport.c
> > index d0857fa..1eb0151 100644
> > --- a/dlls/rpcrt4/rpc_transport.c
> > +++ b/dlls/rpcrt4/rpc_transport.c
> > @@ -3234,6 +3234,8 @@ static int rpcrt4_ncacn_http_close(RpcConnection 
> > *Connection)
> >HeapFree(GetProcessHeap(), 0, httpc->servername);
> >httpc->servername = NULL;
> >  
> > +  /* don't allow this connection to be reused */
> > +  InternetSetOptionW(NULL, INTERNET_OPTION_SETTINGS_CHANGED, NULL, 0);
> 
> There must be a better way to do that than removing all existing wininet
> cached connections to all hosts. Removing INTERNET_FLAG_KEEP_CONNECTION
> from HttpOpenRequest call would be a good start...

We can't do that, it breaks NTLM and Negotiate.






Re: [2/2] rpcrt4: Clear the connection pool when closing an http connection.

2013-09-04 Thread Jacek Caban
On 09/04/13 15:01, Hans Leidekker wrote:
> ---
>  dlls/rpcrt4/rpc_transport.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/dlls/rpcrt4/rpc_transport.c b/dlls/rpcrt4/rpc_transport.c
> index d0857fa..1eb0151 100644
> --- a/dlls/rpcrt4/rpc_transport.c
> +++ b/dlls/rpcrt4/rpc_transport.c
> @@ -3234,6 +3234,8 @@ static int rpcrt4_ncacn_http_close(RpcConnection 
> *Connection)
>HeapFree(GetProcessHeap(), 0, httpc->servername);
>httpc->servername = NULL;
>  
> +  /* don't allow this connection to be reused */
> +  InternetSetOptionW(NULL, INTERNET_OPTION_SETTINGS_CHANGED, NULL, 0);

There must be a better way to do that than removing all existing wininet
cached connections to all hosts. Removing INTERNET_FLAG_KEEP_CONNECTION
from HttpOpenRequest call would be a good start...

Jacek




Re: [PATCH 1/3] riched20: Use codepage in ME_ToUnicode. (try 2)

2013-09-04 Thread Jactry Zeng
Hi Nikolay,

2013/9/5 Nikolay Sivov 
>
> On 9/4/2013 07:17, Jactry Zeng wrote:
>>
>> -ME_EndToUnicode(unicode, wszText);
>> +ME_EndToUnicode(unicode ? 1200 : CP_ACP, wszText);
>
> It's still ugly to use magic numbers like that, in a similar situation I
had some time
> ago I use ~0 to mark WCHAR data encoding that does need special handling.
> Even with a value like ~0 it needs to be documented in code comments.
>

Thanks for your review!
But I still can't understand it. :(  Can you show me a commit as an example.

Thank you!

(sorry for forgeting cc wine-devel)
--
Regards,
Jactry Zeng



Re: [2/2] rpcrt4: Clear the connection pool when closing an http connection.

2013-09-04 Thread Hans Leidekker
On Wed, 2013-09-04 at 18:16 +0200, Jacek Caban wrote:
> On 09/04/13 15:36, Hans Leidekker wrote:
> > On Wed, 2013-09-04 at 15:28 +0200, Jacek Caban wrote:
> >> On 09/04/13 15:01, Hans Leidekker wrote:
> >>> +  /* don't allow this connection to be reused */
> >>> +  InternetSetOptionW(NULL, INTERNET_OPTION_SETTINGS_CHANGED, NULL, 0);
> >> There must be a better way to do that than removing all existing wininet
> >> cached connections to all hosts. Removing INTERNET_FLAG_KEEP_CONNECTION
> >> from HttpOpenRequest call would be a good start...
> > We can't do that, it breaks NTLM and Negotiate.
> 
> Hmm, right. Thinking more about this, I don't see how this connection
> could be reused. RPC over HTTP shouldn't reach the end of the stream, so
> it can't be reused once the connection is established, even if client
> closes the handle. If it is, that's probably a bug in wininet. When did
> you see this to happen?

It happens when Outlook connects to Exchange, closes the connection and then
reconnects. I didn't trust wininet either and wrote a standalone test to mimic
the consecutive connects. It showed that wininet is right in reusing the
connection, though I didn't perform any reads or writes.

You say that reuse depends on reaching end of stream, so this might actually
be a variation of the other bug, which is that in chunked mode we can't know
whether we reached end of stream.






Re: [3/6] ntdll: NtWriteFile should fail for overlapped IO on files if offset is negative.

2013-09-04 Thread Alexandre Julliard
Dmitry Timoshkov  writes:

> diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c
> index cde299c..c802432 100644
> --- a/dlls/ntdll/tests/file.c
> +++ b/dlls/ntdll/tests/file.c
> @@ -2124,11 +2124,8 @@ todo_wine
>  iob.Information = -1;
>  offset.QuadPart = (LONGLONG)-2 /* FILE_USE_FILE_POINTER_POSITION */;
>  status = pNtWriteFile(hfile, 0, NULL, NULL, &iob, contents, 
> sizeof(contents), &offset, NULL);
> -todo_wine
>  ok(status == STATUS_INVALID_PARAMETER, "expected 
> STATUS_INVALID_PARAMETER, got %#x\n", status);
> -todo_wine
>  ok(iob.Status == -1, "expected -1, got %#x\n", iob.Status);
> -todo_wine
>  ok(iob.Information == -1, "expected -1, got %ld\n", iob.Information);

A test for a non-magic negative value would be a good idea.

-- 
Alexandre Julliard
julli...@winehq.org




RE: D3D command stream patches for testing

2013-09-04 Thread Vedran Rodic
Hello,

I was not subscribed to wine-devel, so I can't reply to Stefan's mail
directly, but here are some of my observations with Dota 2 on an Intel
HD4000 GPU.

Intel Mesa driver doesn't seem to like glBufferSubData in a way this
patchset uses it.
I get many many messages like these per frame when I enable
INTEL_DEBUG=perf performance feedback of the Intel Mesa driver:

Using a blit copy to avoid stalling on 588b glBufferSubData() to a
busy buffer object.
Using a blit copy to avoid stalling on 480b glBufferSubData() to a
busy buffer object.
Using a blit copy to avoid stalling on 480b glBufferSubData() to a
busy buffer object.
Using a blit copy to avoid stalling on 1104b glBufferSubData() to a
busy buffer object.
Using a blit copy to avoid stalling on 252b glBufferSubData() to a
busy buffer object.
Using a blit copy to avoid stalling on 204b glBufferSubData() to a
busy buffer object.
Using a blit copy to avoid stalling on 192b glBufferSubData() to a
busy buffer object.
Using a blit copy to avoid stalling on 192b glBufferSubData() to a
busy buffer object.
Using a blit copy to avoid stalling on 12b glBufferSubData() to a busy
buffer object.
Using a blit copy to avoid stalling on 12b glBufferSubData() to a busy
buffer object.

I'm not sure how much this really impacts performance, but even with
my Dota 2 wine perf hacks [1] it's not faster than current Wine git
(performance is very similar).

On what hardware did you test this patchset?

[1]: 
http://vrodic.blogspot.com/2013/08/dota-2-wine-optimization-for-intel-gpus.html




Re: 0001-Fix-race-for-IOCTL_SERIAL_WAIT_ON_MASK.patch

2013-09-04 Thread Alexandre Julliard
Wolfgang Walter  writes:

> @@ -1265,8 +1265,12 @@ static inline NTSTATUS io_control(HANDLE hDevice,
>  case IOCTL_SERIAL_WAIT_ON_MASK:
>  if (lpOutBuffer && nOutBufferSize == sizeof(DWORD))
>  {
> +piosb->u.Status = STATUS_PENDING;
> +piosb->Information = 0;
>  if (!(status = wait_on(hDevice, fd, hEvent, piosb, lpOutBuffer)))
>  sz = sizeof(DWORD);
> +else if (status == STATUS_PENDING)
> +return status;

Status is not supposed to be set until the operation is done.

-- 
Alexandre Julliard
julli...@winehq.org




Re: [PATCH] wined3d: Fix per-stage constant in GLSL fixed function pipeline + add tests. (try 2)

2013-09-04 Thread Matteo Bruni
2013/9/4 Christian Costa :
> This patch fixes part of bug 33606 and 31918.
>
> The current code generates shaders that support per-stage constant but does 
> not declare the variables used causing thus compilation failures.
> With this patch, these variables are declared and updated when needed. Tests 
> are also added.
>
> Thanks to Matteo Bruni for review and support.
>
> Try 2:
>  - update with latest git

Hi, I have a few small nits still, please bear with me...

> ---
>  dlls/d3d9/tests/visual.c   |   80 
> 
>  dlls/wined3d/glsl_shader.c |   34 ++-
>  2 files changed, 112 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c
> index b856c93..6072c23 100644
> --- a/dlls/d3d9/tests/visual.c
> +++ b/dlls/d3d9/tests/visual.c
> @@ -10742,6 +10742,85 @@ static void dp3_alpha_test(IDirect3DDevice9 *device) 
> {
>  ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 
> 0x%08x\n", hr);
>  }
>
> +static void perstage_constant_test(IDirect3DDevice9 *device)
> +{
> +HRESULT hr;
> +D3DCAPS9 caps;
> +DWORD color;
> +struct vertex quad[] = {
> +{ -1.0,-1.0,0.1,0x408080c0 },
> +{  1.0,-1.0,0.1,0x408080c0 },
> +{ -1.0, 1.0,0.1,0x408080c0 },
> +{  1.0, 1.0,0.1,0x408080c0 },
> +};
> +

Can you please make quad[] static const? Also, add the 'f' suffix to
the float constants and move the opening brace to newline.

> +memset(&caps, 0, sizeof(caps));
> +hr = IDirect3DDevice9_GetDeviceCaps(device, &caps);
> +ok(SUCCEEDED(hr), "GetDeviceCaps failed with 0x%08x\n", hr);
> +if (!(caps.PrimitiveMiscCaps & D3DPMISCCAPS_PERSTAGECONSTANT))
> +{
> +skip("D3DPMISCCAPS_PERSTAGECONSTANT not supported\n");

Please put a '.' at the end of the message. Same for the following prints.

> +return;
> +}
> +
> +hr = IDirect3DDevice9_SetFVF(device, D3DFVF_XYZ | D3DFVF_DIFFUSE);
> +ok(hr == D3D_OK, "IDirect3DDevice9_SetFVF failed with 0x%08x\n", hr);
> +
> +hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_CONSTANT, 
> 0x80a1b2c3);
> +ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 
> 0x%08x\n", hr);
> +
> +/* Check color values */
> +hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_COLOROP, 
> D3DTOP_SELECTARG1);
> +ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 
> 0x%08x\n", hr);
> +hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_COLORARG1, 
> D3DTA_CONSTANT);
> +ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 
> 0x%08x\n", hr);
> +

Although you don't strictly need it here, I'd explicitly set
D3DTSS_COLOROP for stage 1 to D3DTOP_DISABLE. Also maybe set
D3DTSS_ALPHAOP for stage 0 to disabled?

> +hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET | 
> D3DCLEAR_ZBUFFER, 0x, 1.0f, 0);
> +ok(hr == D3D_OK, "IDirect3DDevice9_Clear failed with 0x%08x\n", hr);
> +
> +hr = IDirect3DDevice9_BeginScene(device);
> +ok(hr == D3D_OK, "IDirect3DDevice9_BeginScene failed with 0x%08x\n", hr);
> +hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2, 
> quad, sizeof(*quad));
> +ok(SUCCEEDED(hr), "DrawPrimitiveUP failed with 0x%08x\n", hr);
> +hr = IDirect3DDevice9_EndScene(device);
> +ok(hr == D3D_OK, "IDirect3DDevice9_EndScene failed with 0x%08x\n", hr);
> +
> +color = getPixelColor(device, 320, 240);
> +ok(color_match(color, 0x00a1b2c3, 4), "perstage constant test 0x%08x, 
> expected 0x00a1b2c3\n", color);
> +hr = IDirect3DDevice9_Present(device, NULL, NULL, NULL, NULL);
> +ok(SUCCEEDED(hr), "IDirect3DDevice9_Present failed with 0x%08x\n", hr);
> +
> +/* Check alpha value */
> +hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_COLOROP, 
> D3DTOP_SELECTARG1);
> +ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 
> 0x%08x\n", hr);
> +hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_COLORARG1, 
> D3DTA_CONSTANT | D3DTA_ALPHAREPLICATE);
> +ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 
> 0x%08x\n", hr);
> +hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_ALPHAOP, 
> D3DTOP_SELECTARG1);
> +ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 
> 0x%08x\n", hr);
> +hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_ALPHAARG1, 
> D3DTA_CONSTANT);
> +ok(hr == D3D_OK, "IDirect3DDevice9_SetTextureStageState failed with 
> 0x%08x\n", hr);
> +
> +hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET | 
> D3DCLEAR_ZBUFFER, 0x, 1.0f, 0);
> +ok(hr == D3D_OK, "IDirect3DDevice9_Clear failed with 0x%08x\n", hr);
> +
> +hr = IDirect3DDevice9_BeginScene(device);
> +ok(hr == D3D_OK, "IDirect3DDevice9_BeginScene failed with 0x%08x\n", hr);
> +

Re: oledb32: Implement IDataSourceLocator get/put hWnd

2013-09-04 Thread Alexandre Julliard
Alistair Leslie-Hughes  writes:

> Hi,
>
>
> Changelog:
> oledb32: Implement IDataSourceLocator get/put hWnd

It doesn't work here:

../../../tools/runtest -q -P wine -M oledb32.dll -T ../../.. -p 
oledb32_test.exe.so database.c && touch database.ok
database.c:252: Test failed: got 80040154
make[1]: *** [database.ok] Error 1

-- 
Alexandre Julliard
julli...@winehq.org




Re: [PATCH 3/5] wined3d: Don't set WINED3DUSAGE_RENDERTARGET on the front buffer.

2013-09-04 Thread Stefan Dösinger
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Am 2013-09-04 08:56, schrieb Henri Verbeet:
> We never render directly to the front buffer, and in case of e.g. a
> P8 front buffer, we would fail surface creation if we were to
> enforce format restrictions.
This patch uncovers a problem I am not quite sure how to solve.

We set the frontbuffer as render target in wined3d_device_init_3d if
there's no backbuffer, i.e., when ddraw is used. In theory this
doesn't matter because we're not rendering to it, just blitting.

Similarly, dlls/ddraw/device.c calls set_render_target to set the
ddraw frontbuffer on release. The call fails now, but that doesn't
cause any visible problems yet. There is a zombie wined3d surface
left, but it goes away on ddraw shutdown or when a new ddraw
IDirect3DDevice is created.

However, when we create wined3d textures for standalone surfaces, the
ddraw surface isn't destroyed until the wined3d surface goes away
because we're waiting for the callback. This keeps the ddraw surface
alive as well.

The ddraw tests notice this because keeping the IDirectDrawSurface
alive means keeping an IDirect3DDevice1 alive as well. The device
holds a reference to the attached viewport(s), which triggers a
failure in the ddraw1 tests.

The test failure was lingering before, I fixed this by adding a
similar set_render_target call to ddraw_surface_release_iface_destroy.
See the attached patches for reference.

The easiest solution seems to create a separate dummy render target
that ddraw can use to unset application targets. That still leaves the
problem that we're initializing wined3d with an invalid render target now.

-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.20 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSJ6NiAAoJEN0/YqbEcdMwqEsP/3We5UyaUsuUmh8DuqpUDozy
OPFdfg0defIGBB3xdcRPYtwz265jsThxK8ek/vb24E1fJXJsm7zE1CJq1jUh/Beo
X1zkewvL29WpriNvh11AW0+FIrE2XMwmr/5ekHQMXM4Evv2aRWG6zmWj0Wv1zR+x
EcFgHE7J6boOIHtogPoge8KHiVULjxm91Dpnariab0YzUNDUfaHUXopWH0N4hb3T
W5ds4cnHLOapOk8ihDfyAcBXOH4JlvConKKG4zDtbcuovp01yLR+eF03m/eBqt5m
0x3VpPyyaIJpFtOERAi9CqbN0Xuy3wOLmtgHsjy8Qh1wKr/I+DmkuakcsJYELmrY
Ou7n+fDRceJyxdqP8vGtwlsP7Skkyxet/KYt7xbWsMcIcMjFDv6KW6YMyOGg2aJM
O4o8gEFH3zgBQqLBsOf+O85GbQ2ZTLFoRaWkbRYFYYBaypd+S4TBRn9i0gL0Qw/q
KImvYV47HcBK0uTuuV0dG/2YzUs/JraKGMU0sVmUNiDqfOm7+R+NAlU86jQ4PNo+
G0cV2IpEt9Bk/06XmSWI944X7NTe/ZDXs8Fa5MveLk87wETzg54F3RK2AoyHPY59
UK/3nisrRmUDeRZHiRp53E6eSW7VSXoHUgqeHt04fvLheMXpAnuibKJcKGTYqYMe
kfkNuq73chpb9uZhe0EJ
=PCrO
-END PGP SIGNATURE-
>From 14219846c7e4b8b2b95fb26221a99c5144437fec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20D=C3=B6singer?= 
Date: Mon, 2 Sep 2013 17:46:42 +0200
Subject: [PATCH 10/13] ddraw: Unset the render target when it is destroyed
Reply-To: wine-devel 

---
 dlls/ddraw/surface.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c
index 62d3218..037ada9 100644
--- a/dlls/ddraw/surface.c
+++ b/dlls/ddraw/surface.c
@@ -495,6 +495,7 @@ ULONG ddraw_surface_release_iface(struct ddraw_surface *This)
 if (iface_count == 0)
 {
 IUnknown *release_iface = This->ifaceToRelease;
+struct wined3d_surface *rt;
 
 /* Complex attached surfaces are destroyed implicitly when the root is released */
 wined3d_mutex_lock();
@@ -504,6 +505,14 @@ ULONG ddraw_surface_release_iface(struct ddraw_surface *This)
 wined3d_mutex_unlock();
 return iface_count;
 }
+
+rt = wined3d_device_get_render_target(This->ddraw->wined3d_device, 0);
+if (rt == This->wined3d_surface)
+{
+wined3d_device_set_render_target(This->ddraw->wined3d_device, 0,
+This->ddraw->wined3d_frontbuffer, TRUE);
+}
+
 if (This->wined3d_texture) /* If it's a texture, destroy the wined3d texture. */
 wined3d_texture_decref(This->wined3d_texture);
 else
-- 
1.8.1.5

>From 9cb01bbda830b0bc548d254503aac53c437d284c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20D=C3=B6singer?= 
Date: Mon, 2 Sep 2013 17:51:19 +0200
Subject: [PATCH 12/13] ddraw: Create textures for all surfaces
Reply-To: wine-devel 

---
 dlls/ddraw/ddraw.c   | 16 +---
 dlls/ddraw/surface.c |  9 +
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/dlls/ddraw/ddraw.c b/dlls/ddraw/ddraw.c
index 26f1984..99ec4f7 100644
--- a/dlls/ddraw/ddraw.c
+++ b/dlls/ddraw/ddraw.c
@@ -2863,12 +2863,22 @@ static HRESULT CreateSurface(struct ddraw *ddraw, DDSURFACEDESC2 *DDSD,
 }
 }
 
+hr = ddraw_surface_create_texture(object, flags);
+if (FAILED(hr))
+{
+if (version == 7)
+IDirectDrawSurface7_Release(&object->IDirectDrawSurface7_iface);
+else if (version == 4)
+IDirectDrawSurface4_Release(&object->IDirectDrawSurface4_iface);
+else
+IDirectDrawSurface_Release(&object->IDirectDrawSurface_iface);
+
+retu

Re: [PATCH 3/5] wined3d: Don't set WINED3DUSAGE_RENDERTARGET on the front buffer.

2013-09-04 Thread Henri Verbeet
On 4 September 2013 23:18, Stefan Dösinger  wrote:
> We set the frontbuffer as render target in wined3d_device_init_3d if
> there's no backbuffer, i.e., when ddraw is used. In theory this
> doesn't matter because we're not rendering to it, just blitting.
>
> Similarly, dlls/ddraw/device.c calls set_render_target to set the
> ddraw frontbuffer on release. The call fails now, but that doesn't
> cause any visible problems yet. There is a zombie wined3d surface
> left, but it goes away on ddraw shutdown or when a new ddraw
> IDirect3DDevice is created.
>
Right. I had noticed the FIXME, but didn't quite realize we already
enforced it. My long term plan for fixing that FIXME was to get rid of
the restriction on setting render target 0 to NULL, since afaik that's
valid in d3d10 anyway.




Re: D3D command stream patches for testing

2013-09-04 Thread Stefan Dösinger
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Am 2013-09-04 16:30, schrieb Vedran Rodic:
> Using a blit copy to avoid stalling on 12b glBufferSubData() to a
> busy buffer object.
The long term plan is to use GL_ARB_buffer_storage to create buffers
that can be used for rendering while mapped. That avoids the
doublebuffered buffer thing and glBufferSubData calls.

I don't know exactly what the driver means with "blit copy", but I
guess it creates an additional copy of the new data rather than
writing it to the mmap()ed GPU memory directly. It shouldn't hurt too
much.

> I'm not sure how much this really impacts performance, but even
> with my Dota 2 wine perf hacks [1] it's not faster than current
> Wine git (performance is very similar).
Mostly the nvidia blob on Linux, but also some testing on OSX. I got a
lot of reports of crashes on Mesa. I think there are race conditions
in the driver that won't go away until I've moved all GL calls into
the worker thread.

Are you GPU or CPU limited? If you're GPU limited, those patches won't
change anything.


-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.20 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSJ74YAAoJEN0/YqbEcdMwtOEP/R70M0MDrQcwwYVfE6JO5oqN
ilBs6sWjcYing2ynTwD7jjHjEK9HuO89gOmVW1khD/gBEPAwSomrCW9AHQKhDUra
ycx9smrE9zqejBvpXSPYQQDgy9u2D6o9pY243+veAhNcuk+27GmsLBnZOWs2ggAP
P93pNPCS3KTqd2aGSILqZ0fmHipommvA08ftNKfVTC7eqY7p+w6gatuJVKtjBDBd
ZVo5Em4x+UCwjv0Vm/qIHiA2iaspl3aZr+TKIlmU8KuBoO/O9z4iDhHwa70YlBss
mATg9yrh9eFhtIX5aIolS62GfpbmFApjzllULjJzJ0PYz51oknBAlVEG2emKvqmg
JrINQJrv/u4OPBU5rOTH8A4XdAgFjN5nVVrF2Yhw6su6z7tyy2xS11kcOQiTTs9R
U2s/EaqiKIrK9wu1Sq2Vwb8afoBgGB6eBTOJJTaxi7+IkbRh9KBy5EN5s38LyvMe
JtxUS4zMeaIDIePM76nBYZAulo62p2vDDtrlTIigt1FZEpth3oCV9WT1l23SUV3C
8dniiSZoxOh8G2I88J3FjbWd3CaBktS6zsAUuw3r8kL/TzeOwXXVIQeZSZxd5TsN
64GmvFm7RpOncnOb09EQmlgx150jRBi9kTfDfokryfnHdNVAhamIuPxX6P8QY4bP
IJL8AEIU9lLa/z4DVzWU
=vcwM
-END PGP SIGNATURE-




Re: [PATCH 1/5] use a correct timeout in test_waittxempty

2013-09-04 Thread Dmitry Timoshkov
Wolfgang Walter  wrote:

> When we send 17 bytes with 150 baud, no parity, one stop bit then
> we need to wait at least 17*10/150 seconds > 1000 ms

Under Windows with both real COM-port and USB-serial cable this test takes
no longer than 890 ms, usually it's even shorter like 875 ms. Testbot VMs
also don't fail this test. If under Wine it takes > 1000 ms for you then
probably there is a bug somewhere.

-- 
Dmitry.




Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

2013-09-04 Thread Dmitry Timoshkov
Wolfgang Walter  wrote:

> +if (res || (!res && GetLastError() == ERROR_IO_PENDING))
> +/* if data has been sent: wait for termination */
> +Sleep(timeout);

I don't see such a problem with real COM-port and serial-USB cable
under Windows or Linux here and under testbot VMs.

-- 
Dmitry.




Re: [PATCH 5/5] kernel32/tests: remove several todos in test_waittxempty

2013-09-04 Thread Dmitry Timoshkov
Wolfgang Walter  wrote:

> -ok(!res && GetLastError() == ERROR_IO_PENDING, "%d: WaitCommEvent 
> error %d\n", i, GetLastError());
> +ok(res || (!res && GetLastError() == ERROR_IO_PENDING), "%d: 
> WaitCommEvent error %d\n", i, GetLastError());

This change looks spurious and unrelated. Also todos must be removed
in the patch that fixes appropriate behaviour.

-- 
Dmitry.




Re: D3D command stream patches for testing

2013-09-04 Thread Prot Secret
Hi, I am interested in your work, but after patching wine171 of this patch
and enabled csmt, i have seen this in SC2
http://postimg.org/gallery/1w8y6jbg
I use nvidia GTX 680 with proprietary i386 nvidia driver 319.49.

Where I could be wrong, or what else can be set that would be normalized
image using the patch?

Thanks!