932656 - [10.9] Missing background blur in menu with CGSNewCIFilterByName error in system log.

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Alex]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20131029030201

Steps to reproduce:

I noticed rather frequent errors regarding CGSNewCIFilterByName in the system log related to Firefox, and I eventually tracked it down to opening a menu in Firefox (like the bookmark dropdown menu on the toolbar button, system menus are unaffected). I noticed the menus were lacking the normal background blur (Which I remembered was implemented by a CoreImage filter), I assume it's been changed by Apple in Mavericks.

These are the errors I'm seeing via Console whenever I open a menu.

30/10/2013 3:03:35.241 pm firefox[16446]: CGSNewCIFilterByName
30/10/2013 3:03:35.241 pm WindowServer[141]: CGXSetCIFilterValues: Invalid filter 0
30/10/2013 3:03:35.242 pm WindowServer[141]: CGXSetWindowFilter: Invalid filter 0

Comment 1

[Steven Michaud [:smichaud] (Retired)]

I see the console errors you report, but I don't know what you mean by "the normal background blur".

I see these errors only on OS X 10.9, with both Firefox 25 and today's mozilla-central nightly.

Any "non-native" menu triggers these errors -- even a context menu (raised by doing right-click or ctrl-click).

Comment 2

[Steven Michaud [:smichaud] (Retired)]

The only call to CGSNewCIFilterByName() anywhere in the tree is here:

https://hg.mozilla.org/mozilla-central/annotate/829d7bef8b0a/widget/cocoa/nsCocoaWindow.mm#l975

So this must be where the problem is happening.

Comment 3

[Markus Stange [:mstange]]

Steven, the background of context menus is slightly transparent. What you see through it of the window under it should be slightly blurred, and apparently that's no longer the case on 10.9.

Thanks for the report, Alex!

Comment 4

[Steven Michaud [:smichaud] (Retired)]

The non-native menus I see are still slightly transparent on OS X 10.9 (to the same degree as on other versions of OS X, as best I can tell), even with the errors.  My eyes aren't good enough to tell to what degree the overlaid content is blurred, though :-)

Comment 5

[Markus Stange [:mstange]]

Attachment: use CGSSetWindowBackgroundBlurRadius instead

I haven't tested this on 10.9 yet, but I expect it to work.

Comment 6

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8350007 [details] [diff] [review]
use CGSSetWindowBackgroundBlurRadius instead

Markus, what OSes did you test your patch on?  Also, how did you find out about CGSSetWindowBackgroundBlurRadius()?

I doubt my eyes are good enough to test your patch directly.  One thing I *can* do, though, is make sure CGSSetWindowBackgroundBlurRadius() is present (and works in the same way, with the same parameters) on all versions of OS X that we support.

Comment 7

[Markus Stange [:mstange]]

I tested it on 10.8.5, both on a HiDPI display and on an external non-HiDPI monitor.
I found out about CGSSetWindowBackgroundBlurRadius with the Hopper application: I loaded /System/Library/Frameworks/CoreGraphics.framework/CoreGraphics into Hopper and searched for "blur". Then I googled for CGSSetWindowBackgroundBlurRadius and found its signature in this file: http://code.google.com/p/chromium/issues/attachmentText?id=56154&aid=561540033000&name=controller.m (which is an attachment on http://code.google.com/p/chromium/issues/detail?id=56154 which is interesting in its own right).

Comment 8

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8350007 [details] [diff] [review]
use CGSSetWindowBackgroundBlurRadius instead

Thanks for the info.

This looks fine to me.

I used Hopper to check SetWindowBackgroundBlur()'s definition on all our supported versions of OS X, and its behavior on OS X 10.6.8 and 10.9.1.

> CGError CGSSetWindowBackgroundBlurRadius(
>     CGSConnectionID cid, CGSWindowID wid, NSUInteger blur);

This seems basically correct.  I suspect blur is actually a uint32_t.  But we're not going to be using high enough values for that to matter -- especially given the comment about '32' (and presumably higher) causing crashes on OS X 10.6.  By the way, you should probably add that comment to our code.

I used gdb (on OS X 10.6.8) and lldb (on OS X 10.9.1) to check Safari's usage of CGSSetWindowBackgroundBlurRadius().  I didn't see it used for anything other than sheets and menus (including context menus).  In all cases 'blur' was set to '4' (as your patch does).

> http://code.google.com/p/chromium/issues/detail?id=56154

Interesting indeed.  I've always wanted to reverse engineer the WindowServer ... but I've never had the time.  The one long comment would be an interesting starting point.  But you'd still have to rely heavily on tools like Hopper, gdb/lldb and interpose libraries.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a21ca8f06806