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