November 2013 - Wine-Devel - list.winehq.org

Re: mshtml: Avoid memory leaks (coverity)

by Jacek Caban

Hi Frédéric, On 11/30/13 15:16, Frédéric Delanoy wrote: > CID 1131417 > --- > dlls/mshtml/htmlbody.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/dlls/mshtml/htmlbody.c b/dlls/mshtml/htmlbody.c > index fcd8766..163f0ba 100644 > --- a/dlls/mshtml/htmlbody.c > +++ b/dlls/mshtml/htmlbody.c > @@ -648,7 +648,8 @@ static HRESULT WINAPI HTMLBodyElement_get_scroll(IHTMLBodyElement *iface, BSTR * > > if(!overflow || !*overflow) { > *p = NULL; > - return S_OK; > + hres = S_OK; > + goto cleanup; > }else if(!strcmpW(overflow, visibleW) || !strcmpW(overflow, autoW)) { > ret = autoW; > }else if(!strcmpW(overflow, scrollW)) { > @@ -658,11 +659,16 @@ static HRESULT WINAPI HTMLBodyElement_get_scroll(IHTMLBodyElement *iface, BSTR * > }else { > TRACE("Defaulting %s to NULL\n", debugstr_w(overflow)); > *p = NULL; > - return S_OK; > + hres = S_OK; > + goto cleanup; > } > > *p = SysAllocString(ret); > - return *p ? S_OK : E_OUTOFMEMORY; > + hres = *p ? S_OK : E_OUTOFMEMORY; > + > +cleanup: > + SysFreeString(overflow); > + return hres; > } > Good catch, but those gotos make the code more complicated that it needs to be. You may just remove early returns and free overflow before allocating the result. Jacek

Nov. 30, 2013

Re: qmgr/tests: Skip tests on Windows instead of failing

by Nikolay Sivov

On 11/29/2013 23:12, André Hentschel wrote: > Some w2k and win7 machines... > --- > dlls/qmgr/tests/enum_files.c | 2 +- > dlls/qmgr/tests/file.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/dlls/qmgr/tests/enum_files.c b/dlls/qmgr/tests/enum_files.c > index 7e2ab1e..a77f1e7 100644 > --- a/dlls/qmgr/tests/enum_files.c > +++ b/dlls/qmgr/tests/enum_files.c > @@ -256,7 +256,7 @@ START_TEST(enum_files) > /* Keep state separate between tests. */ > if (!setup()) > { > - ok(0, "tests:%d: Unable to setup test\n", i); > + win_skip("tests:%d: Unable to setup test, skipping tests\n", i); > break; > } I guess it's just fails to create an manager instance, please add a copy of test_create_manager() for that. > +++ b/dlls/qmgr/tests/file.c > @@ -197,7 +197,7 @@ START_TEST(file) > /* Keep state separate between tests. */ > if (!setup()) > { > - ok(0, "tests:%d: Unable to setup test\n", i); > + win_skip("tests:%d: Unable to setup test, skipping tests\n", i); > break; > } > (*test)(); This one is different as file.c already skips everything if initial object creation fails. We need to know where setup() fails exactly.

Nov. 29, 2013

Re: [PATCH 2/3] wined3d: Pass a struct wined3d_bo_address to read_from_framebuffer.

by Stefan Dösinger

Am 28.11.2013 17:07 schrieb "Stefan Dösinger" <stefandoesinger(a)gmail.com>: > > > Am 28.11.2013 um 15:56 schrieb Henri Verbeet <hverbeet(a)gmail.com>: > > > On 28 November 2013 10:48, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > >> /* Read the framebuffer back into the surface */ > >> -static void read_from_framebuffer(struct wined3d_surface *surface, void *dest, UINT pitch) > >> +static void read_from_framebuffer(struct wined3d_surface *surface, > >> + const struct wined3d_bo_address *data, UINT pitch) > > It seems a bit silly to pass these as parameters when the only caller > > gets them from the surface. > It keeps the switch between resource->allocatedMemory and PBO in a central place. My goal is to remove resource->allocatedMemory and have surface_get_data and surface_map choose between PBO, heap_memory, user-memory set by surface_set_memory and the DIB. > I guess you meant that I can call surface_get_memory in the function itself...

Nov. 28, 2013

Re: [PATCH 2/3] wined3d: Pass a struct wined3d_bo_address to read_from_framebuffer.

by Stefan Dösinger

Am 28.11.2013 um 15:56 schrieb Henri Verbeet <hverbeet(a)gmail.com>: > On 28 November 2013 10:48, Stefan Dösinger <stefan(a)codeweavers.com> wrote: >> /* Read the framebuffer back into the surface */ >> -static void read_from_framebuffer(struct wined3d_surface *surface, void *dest, UINT pitch) >> +static void read_from_framebuffer(struct wined3d_surface *surface, >> + const struct wined3d_bo_address *data, UINT pitch) > It seems a bit silly to pass these as parameters when the only caller > gets them from the surface. It keeps the switch between resource->allocatedMemory and PBO in a central place. My goal is to remove resource->allocatedMemory and have surface_get_data and surface_map choose between PBO, heap_memory, user-memory set by surface_set_memory and the DIB.

Nov. 28, 2013

Re: AUTHORS: Revert commit 5da31c8817293ff98b4defceb962f7d0fbc25829

by Tae Wong

Here's a version of the AUTHORS list with many omissions of developer names and patch writers started.

Nov. 28, 2013

Re: [PATCH 2/3] wined3d: Pass a struct wined3d_bo_address to read_from_framebuffer.

by Henri Verbeet

On 28 November 2013 10:48, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > /* Read the framebuffer back into the surface */ > -static void read_from_framebuffer(struct wined3d_surface *surface, void *dest, UINT pitch) > +static void read_from_framebuffer(struct wined3d_surface *surface, > + const struct wined3d_bo_address *data, UINT pitch) It seems a bit silly to pass these as parameters when the only caller gets them from the surface.

Nov. 28, 2013

Re: [PATCH 2/3] wined3d: Pass a struct wined3d_bo_address to read_from_framebuffer.

by Henri Verbeet

On 28 November 2013 10:48, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > --- > dlls/wined3d/surface.c | 54 +++++++++++++++++++++----------------------------- > 1 file changed, 23 insertions(+), 31 deletions(-) > > diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c > index 744d6b9..e574956 100644 > --- a/dlls/wined3d/surface.c > +++ b/dlls/wined3d/surface.c > @@ -3438,7 +3438,8 @@ HRESULT CDECL wined3d_surface_flip(struct wined3d_surface *surface, struct wined > } > > /* Read the framebuffer back into the surface */ > -static void read_from_framebuffer(struct wined3d_surface *surface, void *dest, UINT pitch) > +static void read_from_framebuffer(struct wined3d_surface *surface, > + const struct wined3d_bo_address *data, UINT pitch) > { > struct wined3d_device *device = surface->resource.device; > const struct wined3d_gl_info *gl_info; > @@ -3489,7 +3490,7 @@ static void read_from_framebuffer(struct wined3d_surface *surface, void *dest, U > /* In case of P8 render targets the index is stored in the alpha component */ > fmt = GL_ALPHA; > type = GL_UNSIGNED_BYTE; > - mem = dest; > + mem = data->addr; > bpp = surface->resource.format->byte_count; > } > else > @@ -3518,21 +3519,18 @@ static void read_from_framebuffer(struct wined3d_surface *surface, void *dest, U > break; > > default: > - mem = dest; > + mem = data->addr; > fmt = surface->resource.format->glFormat; > type = surface->resource.format->glType; > bpp = surface->resource.format->byte_count; > } > > - if (surface->flags & SFLAG_PBO) > + if (data->buffer_object) > { > - GL_EXTCALL(glBindBufferARB(GL_PIXEL_PACK_BUFFER_ARB, surface->pbo)); > + GL_EXTCALL(glBindBufferARB(GL_PIXEL_PACK_BUFFER_ARB, data->buffer_object)); > checkGLcall("glBindBufferARB"); > if (mem) > - { > ERR("mem not null for pbo -- unexpected\n"); > - mem = NULL; > - } > } > > /* Save old pixel store pack state */ > @@ -3564,23 +3562,14 @@ static void read_from_framebuffer(struct wined3d_surface *surface, void *dest, U > gl_info->gl_ops.gl.p_glPixelStorei(GL_PACK_SKIP_ROWS, skipRow); > checkGLcall("glPixelStorei"); > > - if (surface->flags & SFLAG_PBO) > + if (data->buffer_object && !srcIsUpsideDown) > { > - GL_EXTCALL(glBindBufferARB(GL_PIXEL_PACK_BUFFER_ARB, 0)); > - checkGLcall("glBindBufferARB"); > - > /* Check if we need to flip the image. If we need to flip use glMapBufferARB > * to get a pointer to it and perform the flipping in software. This is a lot > * faster than calling glReadPixels for each line. In case we want more speed > * we should rerender it flipped in a FBO and read the data back from the FBO. */ > - if (!srcIsUpsideDown) > - { > - GL_EXTCALL(glBindBufferARB(GL_PIXEL_UNPACK_BUFFER_ARB, surface->pbo)); > - checkGLcall("glBindBufferARB"); > - > - mem = GL_EXTCALL(glMapBufferARB(GL_PIXEL_UNPACK_BUFFER_ARB, GL_READ_WRITE_ARB)); > - checkGLcall("glMapBufferARB"); > - } > + mem = GL_EXTCALL(glMapBufferARB(GL_PIXEL_PACK_BUFFER_ARB, GL_READ_WRITE_ARB)); > + checkGLcall("glMapBufferARB(GL_PIXEL_PACK_BUFFER_ARB, GL_READ_WRITE_ARB)"); > } > > /* TODO: Merge this with the palettization loop below for P8 targets */ > @@ -3611,15 +3600,19 @@ static void read_from_framebuffer(struct wined3d_surface *surface, void *dest, U > bottom -= pitch; > } > HeapFree(GetProcessHeap(), 0, row); > + } > > - /* Unmap the temp PBO buffer */ > - if (surface->flags & SFLAG_PBO) > - { > - GL_EXTCALL(glUnmapBufferARB(GL_PIXEL_UNPACK_BUFFER_ARB)); > - GL_EXTCALL(glBindBufferARB(GL_PIXEL_UNPACK_BUFFER_ARB, 0)); > - } > + if (data->buffer_object) > + { > + if (!srcIsUpsideDown) > + GL_EXTCALL(glUnmapBufferARB(GL_PIXEL_PACK_BUFFER_ARB)); > + > + GL_EXTCALL(glBindBufferARB(GL_PIXEL_PACK_BUFFER_ARB, 0)); > + checkGLcall("glBindBufferARB(GL_PIXEL_PACK_BUFFER_ARB, 0)"); > } > > + context_release(context); > + > /* For P8 textures we need to perform an inverse palette lookup. This is > * done by searching for a palette index which matches the RGB value. > * Note this isn't guaranteed to work when there are multiple entries for > @@ -3657,7 +3650,7 @@ static void read_from_framebuffer(struct wined3d_surface *surface, void *dest, U > && *green == pal[c].peGreen > && *blue == pal[c].peBlue) > { > - *((BYTE *) dest + y * width + x) = c; > + *((BYTE *)data->addr + y * width + x) = c; > break; > } > } > @@ -3665,8 +3658,6 @@ static void read_from_framebuffer(struct wined3d_surface *surface, void *dest, U > } > HeapFree(GetProcessHeap(), 0, mem); > } > - > - context_release(context); > } > > /* Read the framebuffer contents into a texture. Note that this function > @@ -5046,8 +5037,9 @@ static void surface_load_sysmem(struct wined3d_surface *surface, > > if (surface->flags & SFLAG_INDRAWABLE) > { > - read_from_framebuffer(surface, surface->resource.allocatedMemory, > - wined3d_surface_get_pitch(surface)); > + struct wined3d_bo_address data; > + surface_get_memory(surface, &data); > + read_from_framebuffer(surface, &data, wined3d_surface_get_pitch(surface)); > return; > } > > -- > 1.8.3.2 > > >

Nov. 28, 2013

Re: [PATCH 3/6] wined3d: Move format conversion to surface_upload_data.

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2013-11-27 18:56, schrieb Henri Verbeet: > Doing conversions in the blitter allows them to be done on either > the GPU or the CPU, depending on what's appropriate. I don't think it is worth the trouble. Since we introduced the SNORM and RG conversion code it has never been a performance problem. I haven't seen any application that streams SNORM or RG data. The return of investment would be nonexistent from a user point of view. While optionally doing the conversion on the GPU sounds nice on paper, it means adding another codepath where one of them won't get proper testing, no matter how well it is integrated. > I also think it's just the right place conceptually. It's correct > that this would require some work on the blitter to be able to > correctly express some of the conversions. I disagree, for a number of reasons: The GL format conversion has to work on volumes as well. Do we want to draw them into the blitter? The similarities between the GL format conversion and the surface->surface blitter are limited. Both convert formats, but the former doesn't have to care about sizes, stretching etc. The amount of conversion logic that can be shared is limited. Maybe the RG->RGB expansion code can be shared, if d3d9::StretchRect allows a RG_FLOAT->RGBA_FLOAT blit. But the algorithm that does that is really simple. surface_blt is a god call already by Microsoft's design. I don't think we should add more to it. On the other hand, the patch we are discussing isolates the GL format conversion in surface_upload_data and volume_upload_data. The conv-update.diff patch frees the location management from conversion constraints. I believe those changes are reasonable improvements. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJSlvutAAoJEN0/YqbEcdMwsdAQAIiF/hge67ctBd1ZiHsjz3/K HvJyAN9Umrh020xQC1kQvB3S7d9fZn+n3vf0sQdcQpw5L42zs2XVo2Y462HIo9Aq 9KURr8o+UCsr5o3DVtwzO4PElX7FwImU3N/lC0tJaIe+4lGcI+EUxcBtQ6LLWCfF mp9orSU1SP5VYwG/lmLzgLBF3y8ZJZsbE2C59wnnzQUDYCXX0wGDUj1PgdOwpdh0 HT0JLl8aSACwIMCsqrGKUVIBsVLI4j35qRmSse+Dd/+lJGsAJsRNR7uSpIgWoxo8 s/xNJRTl7orllwpY20yWyvG0TZJhtX0tGX15TsE9q/jwvLeEoSzJE3qpMkNd0b0l kHfDv/eoIdrXyw7tNrE61Z/OY/ep8Ap4VYFvZYKsRcI6jHY5Tcw29LHWQEaY+C2G pHOGzZHfpoBgkmTZN4wC9zKuqMBR4R4Imy6pslz8/0q0Sw5fj6IFEMJpRLRX9c1G AQqeMdPnR0QpmpFj5kJBPP9hcQPEI93spDu9KWw+lL/6PI6vv1PGhSlShwVR08ZD 4SmbeByuBeEyWSnv0MU/z+4FAXKBFLydwzmTK0j5BDydP8IDcG9Xnw1pqW53e4Hz iPsmF8qQU+8f7vYiKZquU7biF/HtBaEpzRU06foIM7VRSpjb6HfNImIwcUXT0DJh /74jszDqhx2A4H7Kx3Ah =bzHp -----END PGP SIGNATURE-----

Nov. 28, 2013

Wine Mono 4.5.2 Release

by Vincent Povirk

This is a small bugfix update because Andrew Eikum found a regression in upstream mono that I thought warranted it. As always, the msi can be installed in current Wine without any modification. Any testing before this gets into Wine would be appreciated. The source tarball is at https://sourceforge.net/projects/wine/files/Wine%20Mono/4.5.2/wine-mono-4.5… The binary is at https://sourceforge.net/projects/wine/files/Wine%20Mono/4.5.2/wine-mono-4.5… For developers, the source repository is at https://github.com/madewokherd/wine-mono Changes since 4.5.0: * Fixed CultureInfo.CurrentCultureInfo always returning InvariantCulture. (Andrew Eikum) * Fixed build on recent mingw versions that define __readfsdword. (Andrew Eikum) * Added the policy/v4.0 registry key to reflect support for .NET 4.

Nov. 27, 2013

Re: [PATCH 3/6] wined3d: Move format conversion to surface_upload_data.

by Henri Verbeet

On 27 November 2013 18:08, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote: > I don’t see how that helps here. This isn’t about a WINED3DFMT_R8G8B8A8_UNORM to WINED3DFMT_R32G32B32A32_FLOAT blit, this about uploading formats that have a conversion function to handle missing GL support (e.g. WINED3DFMT_R16G16_FLOAT without TEXTURE_RG or WINED3DFMT_R16G16_SNORM). Unless we add some WINED3DFMT enum values to express those conversions I don’t see how a staging resource helps here. > Doing conversions in the blitter allows them to be done on either the GPU or the CPU, depending on what's appropriate. I also think it's just the right place conceptually. It's correct that this would require some work on the blitter to be able to correctly express some of the conversions. > I don’t think there are any d3d8/9 blit operations that our GPU blitters cannot handle. But not all blitters are necessarily always available. E.g. d3d8 level hardware generally doesn't have useful fragment shaders.

Nov. 27, 2013

Re: [PATCH 3/6] wined3d: Move format conversion to surface_upload_data.

by Stefan Dösinger

Am 27.11.2013 um 17:46 schrieb Henri Verbeet <hverbeet(a)gmail.com>: > Staging resources. If we're going to enforce access restrictions we're > going to need those anyway for blits that only the CPU blitter can do. I don’t see how that helps here. This isn’t about a WINED3DFMT_R8G8B8A8_UNORM to WINED3DFMT_R32G32B32A32_FLOAT blit, this about uploading formats that have a conversion function to handle missing GL support (e.g. WINED3DFMT_R16G16_FLOAT without TEXTURE_RG or WINED3DFMT_R16G16_SNORM). Unless we add some WINED3DFMT enum values to express those conversions I don’t see how a staging resource helps here. I don’t think there are any d3d8/9 blit operations that our GPU blitters cannot handle. There’s one exception - Partial CopyRects copies from a render target to a sysmem surface. With a small modification to the NP2 repacking code surface_download_data can handle that. A staging resource is a viable alternative here, but this is unrelated to this patch. Ddraw blits don’t matter because all ddraw surfaces are mappable. The conv-update.diff patch has one potential problem: An application could create a R16G16_SNORM surface in D3DPOOL_DEFAULT with D3DUSAGE_DYNAMIC, update it with update_surface and then map it. If that ever happens we can easily handle this with something like if(converted && destination->resource.valid_access & ACCESS_CPU) wined3d_surface_blt(destination, source, ...); else { surface_get_data(source, &data); surface_upload_from_surface(destination, data); } But until we find an application with such an odd behavior I’m fine with the existing FIXME.

Nov. 27, 2013

Re: [PATCH 3/6] wined3d: Move format conversion to surface_upload_data.

by Henri Verbeet

On 27 November 2013 17:37, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote: > > Am 27.11.2013 um 17:29 schrieb Henri Verbeet <hverbeet(a)gmail.com>: >> I think we almost always want to do format conversion in the blitter. > How do you intend to handle the sysmem access restrictions? I don’t think we want to load a non-dynamic DEFAULT pool surface or volume into LOCATION_SYSMEM or LOCATION_BUFFER. > Staging resources. If we're going to enforce access restrictions we're going to need those anyway for blits that only the CPU blitter can do.

Nov. 27, 2013

Re: [PATCH 3/6] wined3d: Move format conversion to surface_upload_data.

by Stefan Dösinger

Am 27.11.2013 um 17:29 schrieb Henri Verbeet <hverbeet(a)gmail.com>: > I think we almost always want to do format conversion in the blitter. How do you intend to handle the sysmem access restrictions? I don’t think we want to load a non-dynamic DEFAULT pool surface or volume into LOCATION_SYSMEM or LOCATION_BUFFER.

Nov. 27, 2013

Re: [PATCH 3/6] wined3d: Move format conversion to surface_upload_data.

by Henri Verbeet

On 27 November 2013 17:16, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote: > Am 27.11.2013 um 17:05 schrieb Henri Verbeet <hverbeet(a)gmail.com>: >> I'm not sure about this, why do we want this? > To upload converted formats directly to the destination in update_texture without blitting to the unlockable default pool destination, something along the lines of the attached diff. Patch 4 interacts with this code move, this is why I’ve moved this patch ahead a bit. > I think we almost always want to do format conversion in the blitter.

Nov. 27, 2013

Re: [PATCH 3/6] wined3d: Move format conversion to surface_upload_data.

by Stefan Dösinger

Am 27.11.2013 um 17:05 schrieb Henri Verbeet <hverbeet(a)gmail.com>: > I'm not sure about this, why do we want this? To upload converted formats directly to the destination in update_texture without blitting to the unlockable default pool destination, something along the lines of the attached diff. Patch 4 interacts with this code move, this is why I’ve moved this patch ahead a bit.

Nov. 27, 2013

Re: [PATCH 3/6] wined3d: Move format conversion to surface_upload_data.

by Henri Verbeet

I'm not sure about this, why do we want this?

Nov. 27, 2013

Re: winhttp/tests: Use BOOL type where appropriate

by Michael Stefaniuc

On 11/27/2013 01:43 PM, Dmitry Timoshkov wrote: > Michael Stefaniuc <mstefani(a)redhat.com> wrote: > >>>>>> --- a/dlls/winhttp/tests/notification.c >>>>>> +++ b/dlls/winhttp/tests/notification.c >>>>>> @@ -44,9 +44,9 @@ struct notification >>>>>> { >>>>>> enum api function; /* api responsible for notification */ >>>>>> unsigned int status; /* status received */ >>>>>> - int todo; >>>>>> - int ignore; >>>>>> - int skipped_for_proxy; >>>>>> + BOOL todo; >>>>>> + BOOL ignore; >>>>>> + BOOL skipped_for_proxy; >>>>> >>>>> Do you realize that BOOL is typedef'ed to int and therefore your patches >>>>> do basically nothing? Is there any real reason behind these patches? >>>> >>>> Hi, Dmitry. This has been discussed before, here is an example: >>>> http://osdir.com/ml/wine-devel/2013-10/msg00131.html >>> >>> I don't think that qualifies as a "discussion" :) Besides the using >>> Windows-only BOOL type these patches break the git history and don't >>> add any advertized clarity IMHO. >> Again this is an optimization for the human reader. BOOL, TRUE and FALSE >> make it easier to read the code without needing too much context. > > Is breaking git history really justified with this kind of patches? > Isn't this from the same (or close enough) area as changing hungarian > notation or replacing LP(C)STR by (const) char * on the whole Wine code > base? Of course it is in the eye of the beholder. For a few of us the benefits outweigh the git history churn on the BOOL side. The LPJUNK removal I do it only in the lines I'm already touching with the exception of the LPIFACE typedefs cause those are even more annoying. bye michael

Nov. 27, 2013

Re: winhttp/tests: Use BOOL type where appropriate

by Dmitry Timoshkov

Michael Stefaniuc <mstefani(a)redhat.com> wrote: > >>>> --- a/dlls/winhttp/tests/notification.c > >>>> +++ b/dlls/winhttp/tests/notification.c > >>>> @@ -44,9 +44,9 @@ struct notification > >>>> { > >>>> enum api function; /* api responsible for notification */ > >>>> unsigned int status; /* status received */ > >>>> - int todo; > >>>> - int ignore; > >>>> - int skipped_for_proxy; > >>>> + BOOL todo; > >>>> + BOOL ignore; > >>>> + BOOL skipped_for_proxy; > >>> > >>> Do you realize that BOOL is typedef'ed to int and therefore your patches > >>> do basically nothing? Is there any real reason behind these patches? > >> > >> Hi, Dmitry. This has been discussed before, here is an example: > >> http://osdir.com/ml/wine-devel/2013-10/msg00131.html > > > > I don't think that qualifies as a "discussion" :) Besides the using > > Windows-only BOOL type these patches break the git history and don't > > add any advertized clarity IMHO. > Again this is an optimization for the human reader. BOOL, TRUE and FALSE > make it easier to read the code without needing too much context. Is breaking git history really justified with this kind of patches? Isn't this from the same (or close enough) area as changing hungarian notation or replacing LP(C)STR by (const) char * on the whole Wine code base? -- Dmitry.

Nov. 27, 2013

Re: winhttp/tests: Use BOOL type where appropriate

by Michael Stefaniuc

On 11/27/2013 12:52 PM, Dmitry Timoshkov wrote: > Bruno Jesus <00cpxxx(a)gmail.com> wrote: > >>>> --- a/dlls/winhttp/tests/notification.c >>>> +++ b/dlls/winhttp/tests/notification.c >>>> @@ -44,9 +44,9 @@ struct notification >>>> { >>>> enum api function; /* api responsible for notification */ >>>> unsigned int status; /* status received */ >>>> - int todo; >>>> - int ignore; >>>> - int skipped_for_proxy; >>>> + BOOL todo; >>>> + BOOL ignore; >>>> + BOOL skipped_for_proxy; >>> >>> Do you realize that BOOL is typedef'ed to int and therefore your patches >>> do basically nothing? Is there any real reason behind these patches? >> >> Hi, Dmitry. This has been discussed before, here is an example: >> http://osdir.com/ml/wine-devel/2013-10/msg00131.html > > I don't think that qualifies as a "discussion" :) Besides the using > Windows-only BOOL type these patches break the git history and don't > add any advertized clarity IMHO. Again this is an optimization for the human reader. BOOL, TRUE and FALSE make it easier to read the code without needing too much context. But it isn't restricted to that. Other tools can do more strict checks like warning about questionable opertations like arithmetic operations with BOOLs. Even before Frederic started I've stumbled over Wine code doing exactly that (oleaut32 in VarFormatNumber()) but the respective code is "ugly" so I couldn't force myself to rewrite it and get rid of the issue. bye michael

Nov. 27, 2013

Re: winhttp/tests: Use BOOL type where appropriate

by Dmitry Timoshkov

Bruno Jesus <00cpxxx(a)gmail.com> wrote: > >> --- a/dlls/winhttp/tests/notification.c > >> +++ b/dlls/winhttp/tests/notification.c > >> @@ -44,9 +44,9 @@ struct notification > >> { > >> enum api function; /* api responsible for notification */ > >> unsigned int status; /* status received */ > >> - int todo; > >> - int ignore; > >> - int skipped_for_proxy; > >> + BOOL todo; > >> + BOOL ignore; > >> + BOOL skipped_for_proxy; > > > > Do you realize that BOOL is typedef'ed to int and therefore your patches > > do basically nothing? Is there any real reason behind these patches? > > Hi, Dmitry. This has been discussed before, here is an example: > http://osdir.com/ml/wine-devel/2013-10/msg00131.html I don't think that qualifies as a "discussion" :) Besides the using Windows-only BOOL type these patches break the git history and don't add any advertized clarity IMHO. -- Dmitry.

Nov. 27, 2013

Re: winhttp/tests: Use BOOL type where appropriate

by Bruno Jesus

On Wed, Nov 27, 2013 at 6:16 AM, Dmitry Timoshkov <dmitry(a)baikal.ru> wrote: > Frédéric Delanoy <frederic.delanoy(a)gmail.com> wrote: > >> --- a/dlls/winhttp/tests/notification.c >> +++ b/dlls/winhttp/tests/notification.c >> @@ -44,9 +44,9 @@ struct notification >> { >> enum api function; /* api responsible for notification */ >> unsigned int status; /* status received */ >> - int todo; >> - int ignore; >> - int skipped_for_proxy; >> + BOOL todo; >> + BOOL ignore; >> + BOOL skipped_for_proxy; > > Do you realize that BOOL is typedef'ed to int and therefore your patches > do basically nothing? Is there any real reason behind these patches? Hi, Dmitry. This has been discussed before, here is an example: http://osdir.com/ml/wine-devel/2013-10/msg00131.html > -- > Dmitry. Best wishes, Bruno

Nov. 27, 2013

Re: Daemonizing wine (fix for issue #15462)

by Alexandre Julliard

Micha Nelissen <micha(a)neli.hopto.org> writes: > On 27-11-13 09:46, Alexandre Julliard wrote: >> You can always start the wineserver separately if you want to send its >> output somewhere else. I don't think there's a need for an elaborate >> mechanism for this. > > I don't know wine very well, but there is also wineboot, and I've also > seen services.exe being started, and Run and RunServices from the > registry, so it's about more than only wineserver. Sure, you just have to run some wine binary first, like winepath, to start all system processes with the stderr you want. That's easy to do with a simple shell alias. The point is that getting diagnostics on stderr is a valuable tool for figuring out problems, and we cannot sacrifice that for the very uncommon case of running a script with redirected stderr. So any solution to this issue has to preserve the display of errors on stderr by default. -- Alexandre Julliard julliard(a)winehq.org

Nov. 27, 2013

Re: Daemonizing wine (fix for issue #15462)

by Micha Nelissen

On 27-11-13 09:46, Alexandre Julliard wrote: > You can always start the wineserver separately if you want to send its > output somewhere else. I don't think there's a need for an elaborate > mechanism for this. I don't know wine very well, but there is also wineboot, and I've also seen services.exe being started, and Run and RunServices from the registry, so it's about more than only wineserver.

Nov. 27, 2013

Re: Daemonizing wine (fix for issue #15462)

by Alexandre Julliard

Micha Nelissen <micha(a)neli.hopto.org> writes: > On 26-11-13 19:29, Alexandre Julliard wrote: >>> The patch creates a daemon.err logfile in the wine server dir >>> (/tmp/.wine-XXX/YYY/), where the output of wineserver.exe, >>> services.exe, and all other background processes go. >> >> I don't think that's a good idea. In the debugging case, you want all >> the traces to go to the same place, and when displaying errors, no user >> is ever going to see them in that file. Not to mention that the log file >> is going to grow endlessly. > > Ok, do you have a counter proposal? Maybe use syslog, except if some > debug flag is set? (but then how does this debug flag get into > kernel32 etc?) > > Note that the current situation is also a bit strange, because the > stderr of e.g. wineserver is directed to the first process it was > started by. So... AFAICS, if process A and process B start, and B > causes an error in wineserver (or services.exe or...) then the error > is output on the stderr of process A. You can always start the wineserver separately if you want to send its output somewhere else. I don't think there's a need for an elaborate mechanism for this. >>> I've removed the inheritance of stdin/stdout/stderr in >>> server/process.c (that took some time to find!) because in windows, >>> either you set bInheritHandles to true with CreateProcess and you >>> inherit stdin etc too, or you set it false, and you get nothing. So >>> this patch should also increase winapi compatibility. >> >> This should be a separate patch, with test cases. > > Where can I look for testcases? Should it be in kernel32/tests/process.c? Probably. -- Alexandre Julliard julliard(a)winehq.org

Nov. 27, 2013

Re: winhttp/tests: Use BOOL type where appropriate

by Dmitry Timoshkov

Frédéric Delanoy <frederic.delanoy(a)gmail.com> wrote: > --- a/dlls/winhttp/tests/notification.c > +++ b/dlls/winhttp/tests/notification.c > @@ -44,9 +44,9 @@ struct notification > { > enum api function; /* api responsible for notification */ > unsigned int status; /* status received */ > - int todo; > - int ignore; > - int skipped_for_proxy; > + BOOL todo; > + BOOL ignore; > + BOOL skipped_for_proxy; Do you realize that BOOL is typedef'ed to int and therefore your patches do basically nothing? Is there any real reason behind these patches? -- Dmitry.

Nov. 27, 2013

Re: kernel32/tests: Add tests for job objects

by 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=3566 Your paranoid android. === w7pro64 (32 bit process) === process.c:2193: Test failed: Unexpected completion event: 7, 000001FC, 00000AB0 process.c:2210: Test failed: Unexpected completion event: 4, 000001FC, 00000000 process.c:2211: Test failed: Unexpected completion event: 6, 000001FC, 00000934 process.c:2212: Test failed: Unexpected completion event: 6, 000001FC, 00000AB8 process.c:2281: Test failed: Unexpected completion event: 7, 000001FC, 00000AB8 process.c:2309: Test failed: Unexpected completion event: 4, 0000023C, 00000000 process.c:2310: Test failed: Unexpected completion event: 6, 000001FC, 00000AE8 process.c:2311: Test failed: Unexpected completion event: 6, 000001FC, 00000AF0

Nov. 27, 2013

Re: Daemonizing wine (fix for issue #15462)

by Micha Nelissen

On 26-11-13 19:29, Alexandre Julliard wrote: >> The patch creates a daemon.err logfile in the wine server dir >> (/tmp/.wine-XXX/YYY/), where the output of wineserver.exe, >> services.exe, and all other background processes go. > > I don't think that's a good idea. In the debugging case, you want all > the traces to go to the same place, and when displaying errors, no user > is ever going to see them in that file. Not to mention that the log file > is going to grow endlessly. Ok, do you have a counter proposal? Maybe use syslog, except if some debug flag is set? (but then how does this debug flag get into kernel32 etc?) Note that the current situation is also a bit strange, because the stderr of e.g. wineserver is directed to the first process it was started by. So... AFAICS, if process A and process B start, and B causes an error in wineserver (or services.exe or...) then the error is output on the stderr of process A. >> I've removed the inheritance of stdin/stdout/stderr in >> server/process.c (that took some time to find!) because in windows, >> either you set bInheritHandles to true with CreateProcess and you >> inherit stdin etc too, or you set it false, and you get nothing. So >> this patch should also increase winapi compatibility. > > This should be a separate patch, with test cases. Where can I look for testcases? Should it be in kernel32/tests/process.c? Thanks, Micha

Nov. 26, 2013

Re: libport: Add a finite() portability macro and use it.

by Alexandre Julliard

Francois Gouget <fgouget(a)free.fr> writes: > --- > > This fixes compilation on Solaris machines. It should probably be a function along the lines of isinf and isnan. -- Alexandre Julliard julliard(a)winehq.org

Nov. 26, 2013

Re: Daemonizing wine (fix for issue #15462)

by Alexandre Julliard

Micha Nelissen <micha(a)neli.hopto.org> writes: > The patch creates a daemon.err logfile in the wine server dir > (/tmp/.wine-XXX/YYY/), where the output of wineserver.exe, > services.exe, and all other background processes go. I don't think that's a good idea. In the debugging case, you want all the traces to go to the same place, and when displaying errors, no user is ever going to see them in that file. Not to mention that the log file is going to grow endlessly. > I've removed the inheritance of stdin/stdout/stderr in > server/process.c (that took some time to find!) because in windows, > either you set bInheritHandles to true with CreateProcess and you > inherit stdin etc too, or you set it false, and you get nothing. So > this patch should also increase winapi compatibility. This should be a separate patch, with test cases. -- Alexandre Julliard julliard(a)winehq.org

Nov. 26, 2013

Re: shell32: change return value of SHFileOperation when deleting files

by Alexandre Julliard

Nikolay Sivov <bunglehead(a)gmail.com> writes: > On 11/26/2013 20:35, Qian Hong wrote: >> On Tue, Nov 26, 2013 at 11:44 PM, Nikolay Sivov <bunglehead(a)gmail.com> wrote: >>> Does it actually fix it? Cause a hack attached to this bug differs from what >>> you're doing. >> I wrote the original hack, it does fix the app, as well as Zhenbo's patch. >> >>> Another thing to test is to try this application on Vista or later to see if >>> it works. >> Yes it works, the app is smart enough to handle both 0x402 and 0x2. > Ok, it's better to return 0x2 value then as this patch does. It would be even better to return the actual error from the corresponding file operations... -- Alexandre Julliard julliard(a)winehq.org

Nov. 26, 2013

Re: shell32: change return value of SHFileOperation when deleting files

by Nikolay Sivov

On 11/26/2013 20:35, Qian Hong wrote: > On Tue, Nov 26, 2013 at 11:44 PM, Nikolay Sivov <bunglehead(a)gmail.com> wrote: >> Does it actually fix it? Cause a hack attached to this bug differs from what >> you're doing. > I wrote the original hack, it does fix the app, as well as Zhenbo's patch. > >> Another thing to test is to try this application on Vista or later to see if >> it works. > Yes it works, the app is smart enough to handle both 0x402 and 0x2. Ok, it's better to return 0x2 value then as this patch does.

Nov. 26, 2013

Re: shell32: change return value of SHFileOperation when deleting files

by Qian Hong

On Tue, Nov 26, 2013 at 11:44 PM, Nikolay Sivov <bunglehead(a)gmail.com> wrote: > Does it actually fix it? Cause a hack attached to this bug differs from what > you're doing. I wrote the original hack, it does fix the app, as well as Zhenbo's patch. > Another thing to test is to try this application on Vista or later to see if > it works. Yes it works, the app is smart enough to handle both 0x402 and 0x2. -- Regards, Qian Hong - http://www.winehq.org

Nov. 26, 2013

Re: shell32: change return value of SHFileOperation when deleting files

by Qian Hong

Hi Zhenbo, thanks for the work. On Tue, Nov 26, 2013 at 11:37 PM, Zhenbo Li <litimetal(a)gmail.com> wrote: > When deleting files, I can't find a situation to return ERROR_PATH_NOT_FOUND Hmm, testing deleting an nonexistent path would be interesting too :) Try shfo.pFrom = "nonexistent\\nonexistent.txt\0"; and shfo.pFrom = "testdir2\0testdir2\0"; In these case, Windows may return ERROR_PATH_NOT_FOUND or ERROR_INVALID_LEVEL. -- Regards, Qian Hong - http://www.winehq.org

Nov. 26, 2013

Re: [PATCH 4/4] wined3d: Don't store PBO pointers in allocatedMemory.

by Henri Verbeet

On 25 November 2013 23:41, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > @@ -817,6 +813,12 @@ static void surface_map(struct wined3d_surface *surface, const RECT *rect, DWORD > > context_release(context); > } > + else > + { > + ret = surface->resource.allocatedMemory; > + } > + > + return ret; > } > You don't really need an else-branch here, you can just return early in the PBO case.

Nov. 26, 2013

Re: shell32: change return value of SHFileOperation when deleting files

by Nikolay Sivov

On 11/26/2013 19:37, Zhenbo Li wrote: > This patch should fix bug 34324. Does it actually fix it? Cause a hack attached to this bug differs from what you're doing. Another thing to test is to try this application on Vista or later to see if it works. > When deleting files, I can't find a situation to return > ERROR_PATH_NOT_FOUND > --- > dlls/shell32/shlfileop.c | 2 +- > dlls/shell32/tests/shlfileop.c | 3 --- > 2 files changed, 1 insertion(+), 4 deletions(-) >

Nov. 26, 2013

Re: [PATCH 6/6] qcap: Fix Avi Mux output pin Connect implementation

by 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=3550 Your paranoid android. === w8 (32 bit qcap) === qcap.c:1293: Test failed: ReceiveConnection returned 80040200 qcap.c:1296: Test failed: ConnectedTo returned 80040209 qcap.c:1297: Test failed: incorrect pin: 00000000, expected 0023FC68 qcap.c:1315: Test failed: expected Reconnect === w864 (32 bit qcap) === qcap.c:1293: Test failed: ReceiveConnection returned 80040200 qcap.c:1296: Test failed: ConnectedTo returned 80040209 qcap.c:1297: Test failed: incorrect pin: 00000000, expected 0028FC58 qcap.c:1315: Test failed: expected Reconnect === w864 (64 bit qcap) === qcap.c:1293: Test failed: ReceiveConnection returned 80040200 qcap.c:1296: Test failed: ConnectedTo returned 80040209 qcap.c:1297: Test failed: incorrect pin: 0000000000000000, expected 000000000023FB30 qcap.c:1315: Test failed: expected Reconnect

Nov. 26, 2013

Re: [PATCH 5/6] qcap: Add helper to initalize test_filter structure

by 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=3549 Your paranoid android. === w8 (32 bit qcap) === qcap.c:1162: Test failed: ReceiveConnection returned 80040200 qcap.c:1165: Test failed: ConnectedTo returned 80040209 qcap.c:1166: Test failed: incorrect pin: 00000000, expected 0023FD64 === w864 (32 bit qcap) === qcap.c:1162: Test failed: ReceiveConnection returned 80040200 qcap.c:1165: Test failed: ConnectedTo returned 80040209 qcap.c:1166: Test failed: incorrect pin: 00000000, expected 0028FD54 === w864 (64 bit qcap) === qcap.c:1162: Test failed: ReceiveConnection returned 80040200 qcap.c:1165: Test failed: ConnectedTo returned 80040209 qcap.c:1166: Test failed: incorrect pin: 0000000000000000, expected 000000000023FB70

Nov. 26, 2013

Re: [PATCH 4/6] qcap: Constify vtables in Avi Mux tests

by 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=3548 Your paranoid android. === w8 (32 bit qcap) === qcap.c:1149: Test failed: ReceiveConnection returned 80040200 qcap.c:1152: Test failed: ConnectedTo returned 80040209 qcap.c:1153: Test failed: incorrect pin: 00000000, expected 0023FD64 === w864 (32 bit qcap) === qcap.c:1149: Test failed: ReceiveConnection returned 80040200 qcap.c:1152: Test failed: ConnectedTo returned 80040209 qcap.c:1153: Test failed: incorrect pin: 00000000, expected 0028FD54 === w864 (64 bit qcap) === qcap.c:1149: Test failed: ReceiveConnection returned 80040200 qcap.c:1152: Test failed: ConnectedTo returned 80040209 qcap.c:1153: Test failed: incorrect pin: 0000000000000000, expected 000000000023FB60

Nov. 26, 2013

Re: [PATCH 3/6] qcap: Add Avi Mux input pin ReceiveConnection test

by 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=3547 Your paranoid android. === w8 (32 bit qcap) === qcap.c:1149: Test failed: ReceiveConnection returned 80040200 qcap.c:1152: Test failed: ConnectedTo returned 80040209 qcap.c:1153: Test failed: incorrect pin: 00000000, expected 0023FD64 === w864 (32 bit qcap) === qcap.c:1149: Test failed: ReceiveConnection returned 80040200 qcap.c:1152: Test failed: ConnectedTo returned 80040209 qcap.c:1153: Test failed: incorrect pin: 00000000, expected 0028FD54 === w864 (64 bit qcap) === qcap.c:1149: Test failed: ReceiveConnection returned 80040200 qcap.c:1152: Test failed: ConnectedTo returned 80040209 qcap.c:1153: Test failed: incorrect pin: 0000000000000000, expected 000000000023FB60

Nov. 26, 2013

Re: [2/4] wininet: Reset data stream for existing request connections. (try 4)

by 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=3543 Your paranoid android. === w2008s64 (64 bit http) === http.c:3615: Test failed: HttpQueryInfo failed 0 === w7pro64 (64 bit http) === http.c:3615: Test failed: HttpQueryInfo failed 0

Nov. 25, 2013

Re: [1/4] wininet/tests: Test successive HttpSendRequest calls. (try 4)

by 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=3542 Your paranoid android. === w2008s64 (64 bit http) === http.c:3615: Test failed: HttpQueryInfo failed 0 === w7pro64 (64 bit http) === http.c:3615: Test failed: HttpQueryInfo failed 0

Nov. 25, 2013

Re: [PATCH 3/5] wined3d: Temporarily remove surface PBO support.

by Henri Verbeet

On 25 November 2013 19:23, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote: > You know the AMD GPU and r600g driver pretty well. Is there anything > in the hw or driver that suggests that the usage really matters and/or > that using a PBO for PACK_BUFFER and UNPACK_BUFFER causes problems? > The Mesa Gallium drivers just care about the STATIC/DYNAMIC/DRAW part of the usage hint, and not so much about READ/DRAW/COPY. I can't say if that will necessarily always stay that way though. I know the proprietary NVIDIA driver does at least complain through ARB_debug_output if the detected usage doesn't match the usage hint.

Nov. 25, 2013

Re: [PATCH 3/5] wined3d: Temporarily remove surface PBO support.

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2013-11-25 18:21, schrieb Henri Verbeet: > I haven't quite decided yet. One consideration is that if we want > the usage hint to glBufferDataARB() to make sense, we can't > necessarily use the same PBO both for downloading and uploading > data. The usage hint is gone from GL_ARB_buffer_storage. Do you really think it matters? Do we have any evidence that suggests that using one PBO to download and upload data causes problems? Even if it does cause problems we should check if it causes more problems than using one surface to download and upload in d3d9. You know the AMD GPU and r600g driver pretty well. Is there anything in the hw or driver that suggests that the usage really matters and/or that using a PBO for PACK_BUFFER and UNPACK_BUFFER causes problems? > Another is essentially the various variants of d3d10's > UpdateSubResource(), and d3d9's GetRenderTargetData() / > GetFrontBufferData(). We'd obviously like to avoid stalling on > those, but it's not clear to me if that's always possible. For > example, always creating PBO's for sysmem surfaces and then > uploading from there probably isn't going to work very well if the > application just creates a single sysmem surface and uses that for > updating different default pool surfaces with UpdateSurface(), > unless perhaps it also uses DISCARD on the sysmem surface. Perhaps > that's ok, but I don't think we really know at this point. Most applications that use UpdateSurface / UpdateTexture that I've worked with in the command Stream development use DISCARD on the sysmem surface. I'd expect the application to stall on native d3d as well if it doesn't use DISCARD, but it is difficult to test. The exception here are intro videos. Apps do all sorts of stupid things, including just using a D3DPOOL_MANAGED texture and calling it a day. It doesn't really matter because pure video playback doesn't queue up lots of commands anyway. The one thing that matters performance-wise is not stalling the pipeline on map. I have not seen a single application that uses D3DPOOL_DEFAULT D3DUSAGE_DYNAMIC textures with DISCARD, except for one case of intro video playback. Not sure which game this was, I'd have to re-check them all. Even the applications that use UpdateSurface / UpdateTexture in real rendering don't stream a lot of data compared to the use of dynamic buffers, usually it's a texture update every other frame. According to my testing, the only applications that profit from PBOs(in their current state, and in my cs tree) are Warcraft 3 and UT2004. They abuse backbuffer->map(READONLY) as a glFinish replacement, and loading the data into a PBO instead of sysmem gives them a ~10% performance boost. > At some point I considered a scheme where the device has a pool of > PBOs and the resource update function grab one from there when > needed. That probably has its own problems, but I haven't thought > it all the way through either. I plan to add a BO pool to the command stream patches to handle DISCARD, but I think that's only partially related to what you've considered. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJSk5WsAAoJEN0/YqbEcdMwr7AP+QGOK7WWNbe07foHaa1G4GPq 2EWpJUnLbxVySZISO70gIrbrE8LPlJf5gQpi1b8UdyUrYTIkp9AFiSpcQYKTtwLE RO4SmwDA3h2OfSbAdjFiVFeZQ1C2oBBdxq1sGx/s9cZcgtXZ/dcaOkyxeEAqkJT2 KhPt4Aaoibv4i3kUXhhBo7x87Rl4qqFRPxe8XLFvkTXJ2OpibyQEV9pYkxmvSkjY BrtV1b3Nt3WXRoSfKsgqWUBx+QlxRVNhO3BaTq6tXvdVq5sF+Mr9gS5hIeyPnkC1 H2UQv79bYybGVlupcCvZV6mxaEVig+krlIGxTyge90wPWuxqwNWJzyl03Jg2dqpk /5uIZyuj877yciNnOY8NhU/5vBIFESZrNH4zWcV6n1TB6Jc2pkGlfw20uj8Zs5Ei BkRMD3EuLhoTL6zG6k+Etmzwrex2qielEM7GNBGlbxxeLEJ+otCQkTTCeItTk62w yRtoIWZTp76a0N82XHKXHVl2C99Wxxr4N5lfu7XJeqJ6SPPxi/XRz0Q9o/l1kPQC 98Buc1FYQtR19go3ogETlKCIp7hB6FXzzIgebdPcoG0YBV+11dIiCzT1gAu5r9BO 3mE53y3NiEl5TAzeuEdPZOiPtKBVTj3QmV6texOTE/hU/lJyKal3FqqZTHXT0QCD TKlJhfAzO1tGEptUf9GQ =G+/U -----END PGP SIGNATURE-----

Nov. 25, 2013

Re: First try to sync translations with Transifex.com

by Henri Verbeet

On 25 November 2013 18:38, Francois Gouget <fgouget(a)free.fr> wrote: > > The attachment seems to have been mangled: when I looked at it here I > only got question marks in the translations. So unless the mangling only > happened here, that would be one reason why the patch has not been > applied. > It looks fine here. Aside from introducing some trailing whitespace, the patch applies fine on top of wine-1.7.5 at least. (But using git send-email is still a good idea.) I'd expect the main issue to be the authorship information.

Nov. 25, 2013

Re: First try to sync translations with Transifex.com

by Francois Gouget

The attachment seems to have been mangled: when I looked at it here I only got question marks in the translations. So unless the mangling only happened here, that would be one reason why the patch has not been applied. Also, although the translation comes from Transifex.com, it looks like you're the only author behind the changes in that patch, i.e. there's presumably no copyright attribution issue. That may be worth noting in the patch submission. Could you resubmit your patch? -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ 1 + e ^ ( i * pi ) = 0

Nov. 25, 2013

Re: [PATCH 3/5] wined3d: Temporarily remove surface PBO support.

by Henri Verbeet

On 25 November 2013 17:01, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote: > I've attached the current state of the surface patches for reference. > Patches 2 and 26 are simpler without PBOs, and I'll have to duplicate > patches 27-29, 33, 41 and 50 in surface.c and then delete or move the > code in separate patches. > > I'd appreciate it if you could take a look at the entire patchset and > see if there's anything else you don't like conceptually or if there's > another major reordering you want. > Ok, but it may take a while. >> I think it's also worth pointing out that the way the volume code >> uses PBOs isn't a whole lot better than what the surface code >> does. > How would you prefer PBOs to work? > I haven't quite decided yet. One consideration is that if we want the usage hint to glBufferDataARB() to make sense, we can't necessarily use the same PBO both for downloading and uploading data. Another is essentially the various variants of d3d10's UpdateSubResource(), and d3d9's GetRenderTargetData() / GetFrontBufferData(). We'd obviously like to avoid stalling on those, but it's not clear to me if that's always possible. For example, always creating PBO's for sysmem surfaces and then uploading from there probably isn't going to work very well if the application just creates a single sysmem surface and uses that for updating different default pool surfaces with UpdateSurface(), unless perhaps it also uses DISCARD on the sysmem surface. Perhaps that's ok, but I don't think we really know at this point. At some point I considered a scheme where the device has a pool of PBOs and the resource update function grab one from there when needed. That probably has its own problems, but I haven't thought it all the way through either.

Nov. 25, 2013

Re: [PATCH 3/5] wined3d: Get rid of surface_set_dirty().

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2013-11-25 17:31, schrieb Stefan Dösinger: > Am 2013-11-25 17:16, schrieb Henri Verbeet: >> Actually, would that do that right thing if the application >> first calls AddDirtyRect(), and then LockRect() with >> D3DLOCK_DISCARD | D3DLOCK_NO_DIRTY_UPDATE? > Good question. It obviously needs a test, but I think yes because > only SYSMEM and DEFAULT pool surfaces support D3DLOCK_DISCARD, and > I expect that D3DLOCK_NO_DIRTY_UPDATE is ignored on DEFAULT pool > surfaces and on SYSMEM surfaces it has a different meaning. That said, I don't think you even need D3DLOCK_DISCARD here. AddDirtyRect on a managed texture followed by a D3DLOCK_NO_DIRTY_UPDATE map is enough to cause trouble for my proposal. I still recommend a test. Native d3d9 may ignore AddDirtyRect in this case for a similar reason. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJSk30xAAoJEN0/YqbEcdMwGlIP/2w8MZNzxUCtG3pqLjQasXie Rk8NqYYRKcJIJZRudZnu6b7Tdxpj4yKnYyp93rSsUBQY1xV76ePCZs0puePOfkdc T9TwV8Rn5jp2Y2IX1xMSQeqoFSyZUPhZ1vr+289CZ2Mp04cofhTuTc2trqmhbciH j5XJFjt6yYvoldh77h7mrtRMXic5iKe9SmQRvV6zfzY3NNMFMuWLVVwxQvcu+ulr jFydDi4HU6kF/pv4OOLVDsjy24DeDV3kL0G0XREN6ixWs41Fci7y09B8amUBne92 zmiARdzAUZ7ZvOq+pPmADrAEmZQy7uwWd4vKD/49UhDVpTRPrYhfjNw9Y+NdfGuX zOPfMXXjGwV7cv1cGH2AKMj8FNJSU0OgoINlpnB3b/VmOTuheXzgL4HIm7pr3AK2 HeK/hGWrDjAS7BG/7CsCi5CW/lsXbeanh0zNZA/1AhKMQp3w0HM9AZp5/Ty8XFaM BWPEqb8TseimVl4DH/mrHoKNptMuvpr/hTG82yVAaSEzshbRXEeeMXDp55RjACGJ 7Ux5DRf19aDj1GNl1lTlMO5X0HqjSY5cqn9zdAqk1fajt7qTPT4ux4A0Xqe7riXa YBylEEQPl8gZCFPf1uVA1F5YP5KeL7VrG/VpBX+7dGMCBT+Z4jjGWF6/Fg+VHwtO SljmdECQV+XsEOYcu6L7 =rgSe -----END PGP SIGNATURE-----

Nov. 25, 2013

Re: [PATCH 3/5] wined3d: Get rid of surface_set_dirty().

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2013-11-25 17:16, schrieb Henri Verbeet: > Actually, would that do that right thing if the application first > calls AddDirtyRect(), and then LockRect() with D3DLOCK_DISCARD | > D3DLOCK_NO_DIRTY_UPDATE? Good question. It obviously needs a test, but I think yes because only SYSMEM and DEFAULT pool surfaces support D3DLOCK_DISCARD, and I expect that D3DLOCK_NO_DIRTY_UPDATE is ignored on DEFAULT pool surfaces and on SYSMEM surfaces it has a different meaning. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJSk3tNAAoJEN0/YqbEcdMwCXsQAI2mrkuXMg+yBFl+Qy8dbtxD obA3KQFtF4lQWHL+sMtZfmIxRbyLWLQmULK1qzBNEYaQK0aJpsxkVPFzN4AxpHsq I68IWS3nBGz1I1ZlyOU6E5Q3DWVKVj6eoz06kLiWxoovpbBr6yWjcWzPDVdFG8nC shpaJfhOPieZLUH8vBYgtvc+hoS48rQ4qK+wPq+syuQX6igCEOivUmzzfZWluzI2 luhvYYes9600k/g/pQtWgyE8IL2hFRLRryU86NLgFSf4Jpb9O/EC0zzsdNc0iSFp +jVg8tSSR70cbopbKJ5pjTjC7qbofL3HkPaevovNQsSqaNqfJ9C3cbAhJo7zQhVv xkIwQTEEtcacnNnHyOQCAeGDyKx5HntlRPsDWjWMnnqTA1Ngmq2GGZe+aDcwOJBA b7UjJCqQngAW/ittWMnOOy2xZt0EIvSaCdm4tvTCvqT8+uQjhhTQoedUbUnQe2y3 mz+As2j+nGn8Xlk2PrJwcCBU9aZuk6Y90yVu2D16rZFXgIY9aauw6LgLXMpeEskO V4a3ps76JqeIKUjakX2rSsddYXdLS7AjZ/Fg5KH0QJ46JjjPThHieC4pxCHDU5ur FTEJT7bHhCczfUcckm3lMiI86jCYIeMFVyLEq4TDLsG+s5Ty4VRjoI7lgsgI5nA9 J6knIEteY0TDtrgODfUf =WSHe -----END PGP SIGNATURE-----

Nov. 25, 2013

Re: [PATCH 3/5] wined3d: Get rid of surface_set_dirty().

by Henri Verbeet

On 25 November 2013 14:22, Henri Verbeet <hverbeet(a)gmail.com> wrote: > On 25 November 2013 13:47, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote: >> Why not something like >> >> if (surface->flags & SFLAG_INSYSMEM) >> surface_invalidate_location(surface, ~SFLAG_INSYSMEM); >> >> If the surface doesn't have SFLAG_INSYSMEM set already downloading >> texture / framebuffer data to sysmem and re-uploading it won't do >> anything. >> > That's probably ok, but I don't think it should necessarily go into this patch. Actually, would that do that right thing if the application first calls AddDirtyRect(), and then LockRect() with D3DLOCK_DISCARD | D3DLOCK_NO_DIRTY_UPDATE?

Nov. 25, 2013

Re: [PATCH 3/5] wined3d: Temporarily remove surface PBO support.

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2013-11-25 12:02, schrieb Henri Verbeet: > On 24 November 2013 23:45, Stefan Dösinger > <stefandoesinger(a)gmail.com> wrote: >> Am 2013-11-24 11:30, schrieb Henri Verbeet: >>> Is there a good reason this can't just be fixed first? >> It's certainly possible, but it comes with a greater risk of >> regressions and I estimate that it will require a week of >> duplicated > That shouldn't be true, I think. I'll give it another try and see where it leads me. I've attached the current state of the surface patches for reference. Patches 2 and 26 are simpler without PBOs, and I'll have to duplicate patches 27-29, 33, 41 and 50 in surface.c and then delete or move the code in separate patches. I'd appreciate it if you could take a look at the entire patchset and see if there's anything else you don't like conceptually or if there's another major reordering you want. > I think it's also worth pointing out that the way the volume code > uses PBOs isn't a whole lot better than what the surface code > does. How would you prefer PBOs to work? My patchset replaces the xFLAG_PBO flags with a map_binding field and adds resource_get_memory similar to buffer_get_memory. This will also take care of user memory and dib sections. Eventually we want to use PBOs for sysmem surfaces and volumes so the driver doesn't have to create an extra data copy in UpdateTexture / UpdateSurface. This requires ARB_buffer_storage, and I don't think it makes sense to implement this before buffer, surface and volume location and GL BO handling is merged. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJSk3RXAAoJEN0/YqbEcdMwbR0P/2627l6lJFpCVzxRaeOl9GkR d9SERwsYHPYjAEkos6YIkiujRmTi7KakIFkUtzP5rZoB9lhguJUDXpFZw2tBhKVY An4LB/cQDRML2i2F0DdCvfdvgC5kH2206KWTst9icOE8ovqNhbUbBYDszsiG0GIA ft/VjfI7R3+0QvbwsPL4PcgKaI/G5GBzrMzUU7MgdMHB9zcVynakPS7AxY+H2XCQ 8lG2f1AmJ50PLF4UUw2xXgUluF9yzTyqijnvwmsrRaDurL6Bly7DG5IZj83A48J7 9qVJq53LUIeATg8ZBJPKqJy0bs8wfyhXpGMRAmgTEgqaxfJc5eZvI8ppSqZUUWHL PriHNmaBm5hEyGbIvOdRjHhzXET9eIGKQBnTm04EEkRSAneLtmTtuSE75tQgEQiN RSPjvIbYmuXBda8SVn4ZyqFyusUvbSo5HZoFlEgDwyl54tKPcF/IICklcOk/YA/+ 7OAYXmgfkkIZLTOo2IobpWKu1PTU2yIb9ivq4RZp8dWhsemje4v+vseZjEBrIZaG zQHyGd3S3ii/fssQRu+vHNrMQKOtbR9Au9rKeBVSPoU9kuHA0uwoJQPFLu4dw51z zvXKRkwMTBXf5OeLRlw4/YHoRsTzIlKsaZ1tElUugx0gfFIC+TrJe17BeLVgFWnt ex/VAj17impWVBtgmQTJ =XN+B -----END PGP SIGNATURE-----

Nov. 25, 2013