opacity on a group containing a blur filtered rect crashes
Created attachment 7748 [details] reduced testcase
This is probably caused by my bogus use of +[NSGraphicsContext setCurrentContext:]. Using opactiy will cause the parent to open a transparency layer. Using a filter will cause the child to try and swap out the current context with setCurrentContext while the transparency layer is still open. That's probably a bad thing :) And probably what is causing the crash.
Reproducible crash -> P1.
Date/Time: 2006-07-11 19:03:14.595 -0700 OS Version: 10.4.7 (Build 8J135) Report Version: 4 Command: Safari Path: /Build/symroots/Debug/Safari.app/Contents/MacOS/Safari Parent: tcsh [219] Version: 3.0 (521.15) PID: 8003 Thread: 0 Exception: EXC_BAD_ACCESS (0x0001) Codes: KERN_INVALID_ADDRESS (0x0001) at 0x7f80000f Thread 0 Crashed: 0 libSystem.B.dylib 0x9000197c pthread_mutex_lock + 36 1 <<00000000>> 0x00000000 0 + 0 2 com.apple.CoreGraphics 0x903bcf28 CGColorTransformGetColorSpace + 40 3 libRIP.A.dylib 0x9470d018 ripc_GetRenderingState + 88 4 libRIP.A.dylib 0x9470c1a4 ripc_DrawImage + 152 5 com.apple.CoreGraphics 0x903d18e8 CGContextDelegateDrawImage + 76 6 com.apple.CoreGraphics 0x904635a0 metalDelegate_FillRectWithImages + 360 7 com.apple.CoreGraphics 0x90463420 CGContextDelegateFillRectWithImages + 116 8 com.apple.CoreGraphics 0x90463204 Private_CGContextFillRectWithImagesPrivate + 124 9 com.apple.HIToolbox 0x934dadd8 HIThemeDrawMetalWithProvider(CGRect const*, CGContext*) + 224 10 com.apple.HIToolbox 0x9325a344 HIThemeDrawBackground + 220 11 com.apple.AppKit 0x937fff80 -[NSGrayFrame drawWindowBackgroundRegion:level:] + 340 12 com.apple.AppKit 0x937534a8 -[NSFrameView drawThemeContentFill:inView:] + 1028 13 com.apple.AppKit 0x937ffd3c -[NSGrayFrame drawRect:] + 48 14 com.apple.AppKit 0x9372f858 -[NSView _drawRect:clip:] + 2128 15 com.apple.AppKit 0x9372e5fc -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 736 16 com.apple.AppKit 0x9374f044 -[NSThemeFrame _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 192 17 com.apple.AppKit 0x93728054 -[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:] + 384 18 com.apple.AppKit 0x9371d348 -[NSView displayIfNeeded] + 248 19 com.apple.AppKit 0x9371d1b8 -[NSWindow displayIfNeeded] + 180 20 com.apple.Safari 0x000578d4 -[BrowserWindow displayIfNeeded] + 328 (BrowserWindow.m:301) 21 com.apple.AppKit 0x9371d064 _handleWindowNeedsDisplay + 200 22 com.apple.CoreFoundation 0x907db73c __CFRunLoopDoObservers + 352 23 com.apple.CoreFoundation 0x907db9dc __CFRunLoopRun + 420 24 com.apple.CoreFoundation 0x907db47c CFRunLoopRunSpecific + 268 25 com.apple.HIToolbox 0x931e6740 RunCurrentEventLoopInMode + 264 26 com.apple.HIToolbox 0x931e5dd4 ReceiveNextEventCommon + 380 27 com.apple.HIToolbox 0x931e5c40 BlockUntilNextEventMatchingListInMode + 96 28 com.apple.AppKit 0x936e9ae4 _DPSNextEvent + 384 29 com.apple.AppKit 0x936e97a8 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 116 30 com.apple.Safari 0x000322b4 -[BrowserApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 292 (BrowserApplication.m:166) 31 com.apple.AppKit 0x936e5cec -[NSApplication run] + 472 32 com.apple.AppKit 0x937d687c NSApplicationMain + 452 33 com.apple.Safari 0x00101de8 main + 420 (main.m:36) 34 com.apple.Safari 0x0000265c _start + 340 (crt.c:272) 35 com.apple.Safari 0x00002504 start + 60
I'm not sure if it helps, but you can avoid the crash by not releasing the m_filterCIContext from the Filter in kCanvasFilterQuartz::applyFilter (approximately line 154 in kCanvasFilterQuarts.mm). Interestingly when you create the filter, the CGContext you pass in gets it's retain count incremented, but when you release the filter, the CGContext it wraps doesn't appear to get it's retain count decremented. CGLayerRelease(m_filterCGLayer); m_filterCGLayer = 0; ->HardRelease(m_filterCIContext); //Remove this line and things seem fine... m_filterCIContext = 0; Also if you skip over the CGBegin/EndTransparencyGroup calls everything appears alright as well. Bugs with doing filters in transparency groups perhaps? calling and restoring the context in a transparencygroup doesn't seem to be the problem, I wrote up quite a bit of sample test code and couldn't repro.
Created attachment 9619 [details] Fix for 8425 and by extension 6947
Comment on attachment 9619 [details] Fix for 8425 and by extension 6947 Looks great! The only thing it needs is a changelog and your copyright. Without those however, this can't be landed.
(In reply to comment #7) > (From update of attachment 9619 [details] [edit]) > Looks great! The only thing it needs is a changelog and your copyright. > Without those however, this can't be landed. Create a ChangeLog entry using the WebKitTools/Scripts/prepare-ChangeLog script.
Created attachment 9630 [details] Fix for 8425 and by extension 6947 This is the updated patch with ChangeLog and copyright notice attached. Thanks!
Comment on attachment 9630 [details] Fix for 8425 and by extension 6947 But also, it's better to just do this with API instead of SPI. Just create and use an autorelease pool in the right place. NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init]; m_filterCIContext = HardRetain([CIContext contextWithCGContext:cgContext options:contextOptions]); [pool drain]; KCanvasFilterQuartz::prepareFilter shhould not use this SPI either -- same technique can be used there.
Created attachment 9641 [details] Fix for 8425 and by extension 6947 Darin...good catch. Fixed it up how you suggested.
Comment on attachment 9641 [details] Fix for 8425 and by extension 6947 r=me Comment is a little long, but the fix is good.
A few nitpicks: the test should ideally include pass criteria ("should not crash"), display something green to make success immediately obvious, and have a link to this Bugzilla issue. Also, onload on <svg> seems unnecessary, unless I'm missing something. Neither of these is important enough to prevent landing, but since we're "waiting for the floodgates to open" anyway... I'm also not quite sure about using -[NSAutoreleasePool release] here - if the goal is to manually control deallocation, isn't drain the right choice under GC?
Created attachment 9652 [details] Fix for 8425 and by extension 6947 New patch fixes up points raised by Alexey and Darin. Filed Radar describing CGEndTransparencyGroup badness <rdar://problem/4647735>
Comment on attachment 9652 [details] Fix for 8425 and by extension 6947 r=me
Committed revision 15603.