January 2011 - Wine-Devel - list.winehq.org

Re: winex11.drv: Make sure that default clipping region exists before calling ExcludeUpdateRgn().

by Dmitry Timoshkov

Alexandre Julliard <julliard(a)winehq.org> wrote: > > If there is no current clipping region ExtSelectClipRgn(hdc, hrgn, RGN_DIFF) > > creates its own default clipping region which in most cases doesn't match > > neither a visible region nor the window extents attached to the DC. > > It will get intersected with the visible region anyway, so it shouldn't > matter as long as it's large enough. Why does it make a difference? I looks like the default clip region created by ExtSelectClipRgn() uses the dimensions of the selected bitmap, and it's not large enough. -- Dmitry.

Jan. 31, 2011

Re: winex11.drv: Make sure that default clipping region exists before calling ExcludeUpdateRgn().

by Alexandre Julliard

Dmitry Timoshkov <dmitry(a)codeweavers.com> writes: > If there is no current clipping region ExtSelectClipRgn(hdc, hrgn, RGN_DIFF) > creates its own default clipping region which in most cases doesn't match > neither a visible region nor the window extents attached to the DC. It will get intersected with the visible region anyway, so it shouldn't matter as long as it's large enough. Why does it make a difference? -- Alexandre Julliard julliard(a)winehq.org

Jan. 31, 2011

Re: [PATCH] [patch] Already in cache means nothing more to do...

by Alexandre Julliard

max(a)mtew.isa-geek.net writes: > @@ -2519,11 +2519,7 @@ static BOOL CommitUrlCacheEntryInternal( > > if (URLCache_FindHash(pHeader, lpszUrlNameA, &pHashEntry)) > { > - FIXME("entry already in cache - don't know what to do!\n"); > -/* > - * SetLastError(ERROR_FILE_NOT_FOUND); > - * return FALSE; > - */ > + error = ERROR_SUCCESS; The entry is not necessarily the same even if the hash matches. -- Alexandre Julliard julliard(a)winehq.org

Jan. 31, 2011

Re: user32/gdi32: Fix for a crash in create_alpha_bitmap()

by Alexandre Julliard

Peter Schlaile <peter(a)schlaile.de> writes: > At least, wine reacts very differently than windows here :) > Read: it crashes the application I mentioned in my previous mail: > http://www.phononet.de/downloads/support/downloads/Aktueller_PNClient/PNCLI… > > And: from a design perspective it sounds very strange that a fast > track optimisation *silently* changes protection bits! There's nothing silent about it, the protection bits have to match the DIB state. > That wouldn't be a problem, if windows behaved exactly the same way. > (which it most likely does not, otherwise PNClient wouldn't crash, > right?). I wouldn't be surprised if there was a way to crash it on Windows too, that app is broken, it never gives the code a chance to handle the exception. This is not how exception handlers are supposed to work. Of course this specific issue would be fixed by a DIB engine, but there are other places where exceptions can happen internally, even on Windows. > Are you sure? 0 means: everything is transparent, and that sounds > like: we need an alpha channel, right? 0 means there's no alpha channel, which is what we are checking here. -- Alexandre Julliard julliard(a)winehq.org

Jan. 31, 2011

Re: user32/gdi32: Fix for a crash in create_alpha_bitmap()

by Peter Schlaile

Hi Alexandre, On Mon, 31 Jan 2011, Alexandre Julliard wrote: > Peter Schlaile <peter(a)schlaile.de> writes: > >> Fix: X11DRV_SetDIBits() shouldn't silently change Bitmap protection, so >> we now restore the protection bits, that were present before instead of >> always changing to READONLY. > > No, it needs to be read-only so that we can detect further changes. so: what is your proposal to fix the problem? At least, wine reacts very differently than windows here :) Read: it crashes the application I mentioned in my previous mail: http://www.phononet.de/downloads/support/downloads/Aktueller_PNClient/PNCLI… And: from a design perspective it sounds very strange that a fast track optimisation *silently* changes protection bits! That wouldn't be a problem, if windows behaved exactly the same way. (which it most likely does not, otherwise PNClient wouldn't crash, right?). And: my problem is, that I already spent three days in tracking this bug down, so, I'm not sure, if I find the time to write test code, that will demonstrate the difference between windows and wine. Should we move to bug tracker then? >> Additional minor fix: bmi_has_alpha() tests for alpha channel != 0, but >> should test for alpha channel != 255. > > No it shouldn't, 0 is correct. Are you sure? 0 means: everything is transparent, and that sounds like: we need an alpha channel, right? Don't get me wrong, if windows behaves like that, it's ok for me, too, but that really sounds strange. Cheers, Peter ---- Peter Schlaile

Jan. 31, 2011

Re: user32/gdi32: Fix for a crash in create_alpha_bitmap()

by Alexandre Julliard

Peter Schlaile <peter(a)schlaile.de> writes: > Fix: X11DRV_SetDIBits() shouldn't silently change Bitmap protection, so > we now restore the protection bits, that were present before instead of > always changing to READONLY. No, it needs to be read-only so that we can detect further changes. > Additional minor fix: bmi_has_alpha() tests for alpha channel != 0, but > should test for alpha channel != 255. No it shouldn't, 0 is correct. -- Alexandre Julliard julliard(a)winehq.org

Jan. 31, 2011

RE: Correction to crash inside RtlCaptureStackBackTrace() + test case

by Janne Hakonen

> > If I need to make changes to, say, test cases, should I also send bug fix > > patch again even if there is no changes to that? > > Please. If it is two separate patches, be sure to make it a series > (label them 1/2, 2/2) and that the tests pass before/after each patch. > Ok, thanks for the info. :) Here's the job run by Testbot, when I posted the patches to the mailing list: http://testbot.winehq.org/JobDetails.pl?Key=8716 Looking at the logs only WNT4WSSP6 and W2KPROSP4 shows message: rtl.c:1340: Tests skipped: RtlCaptureStackBackTrace threw SEH exception, most likely running on Windows NT or 2000 Best Regards, Janne

Jan. 31, 2011

Re: Correction to crash inside RtlCaptureStackBackTrace() + test case

by Austin English

On Sun, Jan 30, 2011 at 22:15, Janne Hakonen <joyer83(a)live.fi> wrote: >>Also, it is custom to mark your resubmissions with corrections as [try x]. >> Makes it easier to determine what you are doing. > I posted my patches to patches mailing list only once so far, so there > hasn't been any retries yet. > But, I shall add that tag if/when I need to adjust those or any future > patches. > If I need to make changes to, say, test cases, should I also send bug fix > patch again even if there is no changes to that? Please. If it is two separate patches, be sure to make it a series (label them 1/2, 2/2) and that the tests pass before/after each patch. -- -Austin

Jan. 31, 2011

RE: Correction to crash inside RtlCaptureStackBackTrace() + test case

by Janne Hakonen

> Did you try this on WindowsXP or Windows7 to make sure that the win_skip did not function on those platforms? My understanding is that win_skip works on all Windows platforms not just WindowsNT or Windows2000. Well, I did test that skip doesn't happen on my home computer, which is Windows 7. I also tested that the skip do happen on Windows NT4 and 2000 on Testbot. >Also, it is custom to mark your resubmissions with corrections as [try x]. Makes it easier to determine what you are doing. I posted my patches to patches mailing list only once so far, so there hasn't been any retries yet. But, I shall add that tag if/when I need to adjust those or any future patches. If I need to make changes to, say, test cases, should I also send bug fix patch again even if there is no changes to that? Best Regards, Janne

Jan. 31, 2011

Re: user32/gdi32: Fix for a crash in create_alpha_bitmap()

by Vitaliy Margolen

On 01/30/2011 12:02 PM, Peter Schlaile wrote: > Nevertheless: I hope, I explained pretty well, how and why the current code > is wrong (aquiring a bitmap-pointer using CreateDIBSection, setting this > bitmap to readonly using the fast track code path within StretchDIBits, > writing to this bitmap again.). Sounds plausible to me, but I'm not expert in that area. > probably, but you will agree, that this is a rather trivial change... (and > pretty obvious, too). No, me and everyone else here won't agree. Please send a separate patch with explanation why the change is needed. Each patch have to contain one change and one change only. No exceptions. Please see patch submission guidelines here: http://wiki.winehq.org/SubmittingPatches Vitaliy.

Jan. 31, 2011