December 2009 - Wine-Devel - list.winehq.org

Re: [PATCH 1/7] dsound: Remove support for IKsPropertySet for now

by Maarten Lankhorst

Hi Robert, Robert Reif schreef: > Maarten Lankhorst wrote: >> -- >> As I said, I'm confident nobody uses it, since wine never implemented >> it properly anyway >> --- >> > I think you will find that you will have to add it back later because > both interfaces are commonly used. > > The private interface is used for enumerating devices and is the only > way of getting some information about the actual hardware. Yeah, but I haven't found a single program that seems to use it, and since the reworking is already hard enough right now to split up, I felt that leaving it in the patchset would make my work even harder. It will be added back, but I haven't found any program using the IKsPropertySet, either buffer or interface, so I don't really have any non-wine testcases. > The buffer interface is commonly used to determine special hardware > effects. Programs do check even if wine doesn't implement them > because most windows sound cards don't either. In fact, now that you > will be using openal you might have the opportunity to actually add > real effects! I'm aware, but working on reworking things at the moment, since the whole directsound interface will be replaced, this code would be reimplemented as well. Does the removal really break anything? I thought that on acceleration < full those interfaces wouldn't be available anyhow, so programs should be able to deal with it. If not, I'm very interested in seeing those programs, since none of the programs I tested really seemed to care, even programs that used dsound3d. If a program breaks on this, I would be really interested, since I thought IKsPropertySet isn't even supposed to be available on a software loc buffer (which is all our directsound supports anyhow). Cheers, Maarten.

Dec. 3, 2009

Re: [PATCH 1/7] dsound: Remove support for IKsPropertySet for now

by Robert Reif

Maarten Lankhorst wrote: > -- > As I said, I'm confident nobody uses it, since wine never implemented it properly anyway > --- > I think you will find that you will have to add it back later because both interfaces are commonly used. The private interface is used for enumerating devices and is the only way of getting some information about the actual hardware. The buffer interface is commonly used to determine special hardware effects. Programs do check even if wine doesn't implement them because most windows sound cards don't either. In fact, now that you will be using openal you might have the opportunity to actually add real effects!

Dec. 3, 2009

Re: [4/6] WineD3D: Infrastructure to render swapchains to a FBO

by Stefan Dösinger

Am 03.12.2009 um 11:19 schrieb Henri Verbeet: > 2009/12/3 Stefan Dösinger <stefan(a)codeweavers.com>: >> It matters when reading back an offscreen render target if FBOs are enabled, but FBO_blit is not supported. The readback function calls glReadBuffer(device->offscreenBuffer) in this case, and is correct to do so because its needed with other ORM methods. Without this change copying the backbuffer from an FBO into a texture without FBO_blit support fails. >> > You should never need to do that, FBO ORM implies that you already > have the data in a texture. That doesn't help much if I want it in another texture(ie, StretchRect from offscreen target to offscreen target). Of course I could download and upload it, but a glCopyTexImage from an FBO to a texture works faster. > I think the more interesting part to share would be setting up the > pipeline states for blitting, actually. CTXUSAGE_BLIT does that to > some extent, but it assumes fixed function blits, and ignores the case > where we support shaders, or can do the blit with fbo_blit. It can be > done afterwards if you think it's too much work, but I think it's > nicer to do first. Well, we have the blit backend - I guess we should consistently use that rather than call glEnable(GL_TEXTURE_2D), and probably move the depth blit shader and the P8 unpack shader in there.

Dec. 3, 2009

Re: [5/6] imagehlp: Recalculate PE checksum after file modification

by Alexandre Julliard

Owen Rudge <orudge(a)codeweavers.com> writes: > +static BOOL IMAGEHLP_RecalculateChecksum(HANDLE handle) > +{ > + IMAGE_DOS_HEADER dos_hdr; > + IMAGE_NT_HEADERS nt_hdr; > + HANDLE hMapping; > + LPVOID BaseAddress; > + DWORD FileLength, count, HeaderSum, CheckSum = 0; > + BOOL r; > + > + TRACE("handle %p\n", handle); > + > + /* read the DOS header */ > + count = SetFilePointer(handle, 0, NULL, FILE_BEGIN); > + > + if (count == INVALID_SET_FILE_POINTER) > + return FALSE; > + > + count = 0; > + > + r = ReadFile(handle, &dos_hdr, sizeof dos_hdr, &count, NULL); > + > + if (!r) > + return FALSE; > + > + if (count != sizeof dos_hdr) > + return FALSE; > + > + /* verify magic number of 'MZ' */ > + if (dos_hdr.e_magic != 0x5A4D) > + return FALSE; > + > + /* read the PE header */ > + count = SetFilePointer(handle, dos_hdr.e_lfanew, NULL, FILE_BEGIN); > + > + if (count == INVALID_SET_FILE_POINTER) > + return FALSE; > + > + count = 0; > + r = ReadFile(handle, &nt_hdr, sizeof nt_hdr, &count, NULL); > + > + if (!r) > + return FALSE; > + > + if (count != sizeof nt_hdr) > + return FALSE; > + > + /* verify NT signature */ > + if (nt_hdr.Signature != IMAGE_NT_SIGNATURE) > + return FALSE; Please write a helper for this, we don't need 3 implementations of the same thing. -- Alexandre Julliard julliard(a)winehq.org

Dec. 3, 2009

Re: [1/6] imagehlp: Verify magic numbers and pad certificates to 8 byte boundaries

by Alexandre Julliard

Owen Rudge <orudge(a)codeweavers.com> writes: > @@ -64,6 +64,10 @@ static BOOL IMAGEHLP_GetSecurityDirOffset( HANDLE handle, > if( count != sizeof dos_hdr ) > return FALSE; > > + /* verify magic number of 'MZ' */ > + if( dos_hdr.e_magic != 0x5A4D ) > + return FALSE; > + > /* read the PE header */ > count = SetFilePointer( handle, dos_hdr.e_lfanew, NULL, FILE_BEGIN ); > if( count == INVALID_SET_FILE_POINTER ) > @@ -75,6 +79,10 @@ static BOOL IMAGEHLP_GetSecurityDirOffset( HANDLE handle, > if( count != sizeof nt_hdr ) > return FALSE; > > + /* verify NT signature */ > + if( nt_hdr.Signature != IMAGE_NT_SIGNATURE ) > + return FALSE; > + > sd = &nt_hdr.OptionalHeader. > DataDirectory[IMAGE_FILE_SECURITY_DIRECTORY]; You also need to check the optional header magic to differentiate between 32-bit and 64-bit PE files. -- Alexandre Julliard julliard(a)winehq.org

Dec. 3, 2009

Re: ntdll: add partial support for HEAP_FREE_CHECKING_ENABLED heap overrun detection (take 2)

by Alexandre Julliard

Dan Kegel <dank(a)kegel.com> writes: > @@ -467,6 +475,156 @@ static void test_HeapQueryInformation(void) > ok(info == 0 || info == 1 || info == 2, "expected 0, 1 or 2, got %u\n", info); > } > > +static LPVOID (WINAPI * pRtlAllocateHeap)(HANDLE,ULONG,ULONG); > +static HANDLE (WINAPI * pRtlCreateHeap)(ULONG,PVOID,SIZE_T,SIZE_T,PVOID,PRTL_HEAP_DEFINITION); > +static HANDLE (WINAPI * pRtlDestroyHeap)(HANDLE); > +static NTSTATUS (WINAPI * pRtlFreeHeap)(HANDLE, ULONG, PVOID); > +static ULONG (WINAPI * pRtlGetNtGlobalFlags)(void); > +static LPVOID (WINAPI * pRtlReAllocateHeap)(HANDLE, ULONG, PVOID, ULONG); > +static SIZE_T (WINAPI * pRtlSizeHeap)( HANDLE, ULONG, LPCVOID); > +static BOOLEAN (WINAPI * pRtlValidateHeap)(HANDLE, ULONG, LPCVOID); > + > +static HMODULE hntdll; Now that it's in kernel32 you should of course use the kernel32 heap functions instead. > @@ -348,7 +349,10 @@ HANDLE thread_init(void) > peb->NtGlobalFlag = get_global_flag(); > > /* create the process heap */ > - if (!(peb->ProcessHeap = RtlCreateHeap( HEAP_GROWABLE, NULL, 0, 0, NULL, NULL ))) > + heapflags = HEAP_GROWABLE; > + if (peb->NtGlobalFlag & FLG_HEAP_ENABLE_FREE_CHECK) > + heapflags |= HEAP_FREE_CHECKING_ENABLED; > + if (!(peb->ProcessHeap = RtlCreateHeap( heapflags, NULL, 0, 0, NULL, NULL ))) Is there any reason for not doing this for all created heaps, instead of just the main one? -- Alexandre Julliard julliard(a)winehq.org

Dec. 3, 2009

Re: [4/6] WineD3D: Infrastructure to render swapchains to a FBO

by Henri Verbeet

2009/12/3 Stefan Dösinger <stefan(a)codeweavers.com>: > Am 02.12.2009 um 22:10 schrieb Henri Verbeet: >> This change is probably correct, but note that if it matters you're >> probably doing something wrong. > It matters when reading back an offscreen render target if FBOs are enabled, but FBO_blit is not supported. The readback function calls glReadBuffer(device->offscreenBuffer) in this case, and is correct to do so because its needed with other ORM methods. Without this change copying the backbuffer from an FBO into a texture without FBO_blit support fails. > You should never need to do that, FBO ORM implies that you already have the data in a texture. >>> +static inline void swapchain_blit(IWineD3DSwapChainImpl *This, struct wined3d_context *context) >> We have a number of functions now that do something very similar to >> what this function does. Perhaps it's time to spend some time on our >> blitter in order to unify those? > The part that's shared between those is the glBegin(); 4*texcoord, 4*vertex; glEnd() block, but I'm not convinced that putting this pretty simple piece of code into a shared function is worth the trouble. Most of the code in the blit functions is to find the proper coordinates for the specific problem. > I think the more interesting part to share would be setting up the pipeline states for blitting, actually. CTXUSAGE_BLIT does that to some extent, but it assumes fixed function blits, and ignores the case where we support shaders, or can do the blit with fbo_blit. It can be done afterwards if you think it's too much work, but I think it's nicer to do first.

Dec. 3, 2009

Re: [2/6] WineD3D: Add a function for initializing surface sysmem

by Henri Verbeet

2009/12/3 Stefan Dösinger <stefandoesinger(a)gmx.at>: > > Am 02.12.2009 um 22:10 schrieb Henri Verbeet: >> IWineD3DDeviceImpl_Reset() suffers from similar issues in general, so >> maybe we just don't care, but if one of the updateSurfaceDesc() calls >> fails you're left with inconsistent surface sizes. > We don't care because by Microsoft's design Reset() leaves the device in a broken state if it fails, and all you can do with it is call Reset() again or destroy it. > Ok. >>> + This->Flags |= SFLAG_INSYSMEM; >> Why is this in surface_init_sysmem(), instead of a ModifyLocation() >> call after the call to surface_init_sysmem()? > My sense is that surface_init_sysmem should set it since it allocates and writes to the system memory copy. Probably surface_init_sysmem should call ModifyLocation itself to remove the other location flags. > That would work as well, my main point was that I don't like functions other than ModifyLocation() touching the location flags.

Dec. 3, 2009

Re: Fixing the kernel32:actctx tests on Vista+

by improve pctools

I hope this article will help you fix the http://www.improve-pc-tools.com/index.php/how-to-fix-kernel32dll-errors/ kernel32 error on vista. Jacek Caban-2 wrote: > > Paul Vriens wrote: >> On 11/25/2009 05:39 PM, Jacek Caban wrote: >>> Hi Paul, >>> >>> Paul Vriens wrote: >>>> Hi Jacek, >>>> >>>> You are the original author of these tests. Is there any reason the >>>> "test3.manifest" name is reused? >>>> >>>> The tests currently crash (intermittently on some) on Vista+ and with >>>> the attached patch things seem to get better. I'm just renaming one of >>>> the manifest files so there are no duplicates in these tests. >>> >>> I don't remember much about these tests, but looking at the code I don't >>> see any reason to reuse the name, so your patch looks good for me. >> >> Well, it was 2007 ;) >> >> Btw. another patch (see attached patch2) works as well and is >> basically the same thing that Ge did back in September to these same >> tests. >> >> So both work fine. Any favors for one over the other? > > Both seem good for me. I think that a new file name is slightly better, > because it feels safer than cheating cache mechanisms. > > > Jacek > > > > -- View this message in context: http://old.nabble.com/Fixing-the-kernel32%3Aactctx-tests-on-Vista%2B-tp2651… Sent from the Wine - Devel mailing list archive at Nabble.com.

Dec. 3, 2009

Re: RFC: dsound: Add support for enumerating capture devices through openal, try 4

by Maarten Lankhorst

Hi Ken, Ken Thomases schreef: > On Dec 2, 2009, at 12:11 PM, Maarten Lankhorst wrote: > >> This patch doesn't apply, just a rfc email to see if this approach is >> correct :) > >> +static void *openal_handle = RTLD_DEFAULT; > >> +#ifdef SONAME_LIBOPENAL >> + char error[128]; >> + openal_handle = wine_dlopen(SONAME_LIBOPENAL, RTLD_NOW, error, >> sizeof(error)); >> + if (!openal_handle) >> + ERR("Couldn't load " SONAME_LIBOPENAL ": %s\n", error); >> +#endif >> +#define LOAD_FUNCPTR(f) \ >> + if((p##f = wine_dlsym(openal_handle, #f, NULL, 0)) == NULL) { \ >> + ERR("Couldn't lookup %s in libopenal\n", #f); \ >> + failed = 1; \ >> + } >> + >> + LOAD_FUNCPTR(alcCreateContext); > > By defaulting to RTLD_DEFAULT on the Mac (where SONAME_LIBOPENAL won't > be defined), is there any risk that wine_dlsym will find the symbols > from Wine's openal32.dll.so rather than the linked OpenAL framework? > If it does, is that a problem? > > I had proposed using RTLD_NEXT for the default to avoid that > possibility, although it seems that RTLD_NEXT isn't universally > available. We might use RTLD_NEXT if it's defined and RTLD_DEFAULT > otherwise. Those symbols are prefixed with wine_ and libopenal is static linked to openal32, so there cannot be duplicate names between the 2. We don't have to worry about the win32 symbol names, since they're not handled by wine_dlsym. Just to be sure I tested, and confirmed that. Cheers, Maarten.

Dec. 3, 2009