[10.10] Implement support for NSVisualEffectView for Yosemite UI effects

RESOLVED FIXED in mozilla34

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Nomis101, Assigned: mstange)

Tracking

unspecified
mozilla34
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Reporter

Description

5 years ago
It seems to get the translucent backgrounds and all other UI effects introduced in Yosemite, we need some support for NSVisualEffectView. It says [1]:
"The new class NSVisualEffectView enables you to create translucent backgrounds and support vibrancy."

There is also sample code for [2]:
"This sample shows how to use various options of NSVisualEffectView to do blurs, translucency, and vibrancy on OS X. It demonstrates in window blending, behind window blending, creating masks, using state, and custom vibrant controls."

[1]: https://developer.apple.com/library/prerelease/mac/releasenotes/MacOSX/WhatsNewInOSX/Articles/MacOSX10_10.html
[2]: https://developer.apple.com/library/prerelease/mac/samplecode/VisualEffectPlayground/Introduction/Intro.html
Assignee

Comment 1

5 years ago
Here's how to create an instance of NSVisualEffectView, and an instance of a class that inherits from NSVisualEffectView, when not building against the 10.10 SDK:

static BOOL
AllowsVibrancyYes(id self, SEL _cmd)
{
  return YES;
}

// [...]

  Class NSVisualEffectViewClass = NSClassFromString(@"NSVisualEffectView");
  if (NSVisualEffectViewClass) {
    NSView* effectView1 = [[NSVisualEffectViewClass alloc] initWithFrame:NSMakeRect(0, 100, 1000, 100)];
    [[window contentView] addSubview:effectView1];

    Class EffectViewClass = objc_allocateClassPair(NSVisualEffectViewClass, "EffectView", 0);
    class_addMethod(EffectViewClass, @selector(allowsVibrancy), (IMP)AllowsVibrancyYes, "I@:");
    NSView* effectView2 = [[EffectViewClass alloc] initWithFrame:NSMakeRect(0, 200, 1000, 100)];
    [[window contentView] addSubview:effectView2];
  }
Assignee: nobody → mstange

Updated

5 years ago

Updated

5 years ago
Depends on: 1052466
Assignee

Comment 2

5 years ago
I have this mostly working locally and will attach patches once I've extracted and cleaned them up a little.
Assignee

Updated

5 years ago
Depends on: 1055585
Assignee

Comment 4

5 years ago
I'm going to add a new class that needs access to an nsChildView solely for the coordinate conversion functions, so I'd like to pass a "const nsChildView&" to make the limited impact clear. I need the conversion functions to be const in order for that to work.
Attachment #8475235 - Flags: review?(smichaud)
Assignee

Comment 5

5 years ago
This patch depends on the patches in bug 1055585 in order to work.

It takes care of creating the NSVisualEffectViews that are required for letting the window server know the vibrant areas of a window, and it does that during nsChildView::UpdateThemeGeometries for the new vibrancy -moz-appearance values.

Depending on the development in bug 1055585, some small parts of this patch may be subject to changes, but this can be reviewed already anyway.

Note: This changes the NSView hierarchy. I know you won't like that part, Steven, but adding NSVisualEffectViews is really the only documented way to achieve the desired effect. And from what I've seen of the internal workings, all of the communication of vibrant regions with the window server happens inside methods of NSVisualEffectView, so we really do have to create them.
(We've had a similar conversation before around 944836 comment 15, and my opinion is still that we should just try it and see whether your concerns there were warranted.)
Attachment #8475242 - Flags: review?(smichaud)
Assignee

Comment 6

5 years ago
(In reply to Markus Stange [:mstange] from comment #5)
> (We've had a similar conversation before around 944836 comment 15

That's bug 944836 comment 15.
Assignee

Updated

5 years ago
Blocks: 1055614
Assignee

Updated

5 years ago
No longer blocks: theme-yosemite
> Note: This changes the NSView hierarchy. I know you won't like that
> part, Steven, but adding NSVisualEffectViews is really the only
> documented way to achieve the desired effect.

I'm not dead set against changing the NSView hierarchy.  I'm just
reluctant to do so if there's any reasonable alternative.

Before reviewing your patches here, I want to spend a few hours
learning what I can about how Apple apps use NSVisualEffectView on
Yosemite.  I should have time for that tomorrow.  So I should be able
to review your patches tomorrow or the day after.
Assignee

Comment 8

5 years ago
(In reply to Steven Michaud from comment #7)
> Before reviewing your patches here, I want to spend a few hours
> learning what I can about how Apple apps use NSVisualEffectView on
> Yosemite.

Thanks. Concerning documentation from Apple, the most detailed stuff about vibrancy is probably on https://developer.apple.com/videos/wwdc/2014/ , in "Adapting Your App to the New UI of OS X Yosemite" starting on page 165, and in "Adopting Advanced Features of the New UI of OS X Yosemite".

Here are a few notes from my experiments about things that weren't clear to me from the presentations:
 - There's a difference between the vibrancy you get when you override allowsVibrancy and have it return YES, vs when you return NO. Returning NO does not mean that there's no vibrancy at all! It only means that the blurred vibrancy effect is only visible in those pixels in your window that you leave transparent. I'd call this "background vibrancy". If you return YES from allowsVibrancy, the vibrancy blending also applies to opaque content in your window; I'd call that "foreground vibrancy". We probably don't need foreground vibrancy in Firefox. It looks like it's mostly used for vibrant text labels.
 - Whether you get light or dark vibrancy depends on the appearance you set on the NSVisualEffectView.
 - The appearance value on subviews of the NSVisualEffectView doesn't let you toggle between dark and light, but instead between foreground and background vibrancy, IIRC. Setting the appearance to aqua gives the view only background vibrancy, regardless of the return value of allowsVibrancy. Setting the appearance to light (or dark?) and also returning YES from allowsVibrancy means that you get foreground vibrancy.
Attachment #8475235 - Flags: review?(smichaud) → review+
Comment on attachment 8475242 [details] [diff] [review]
part 3: Hook everything together

Thanks for the info, Markus.  What I was most wanted to do was to check out what you say here:

+  // The super implementation would clear the background.
+  // That's fine for views that are placed below their content, but our
+  // setup is different: Our drawn content is drawn to mContainerView, which
+  // sits below this EffectView. So we must not clear the background here,
+  // because we'd erase that drawn content.
+  // Of course the regular content drawing still needs to clear the background
+  // behind vibrant areas. This is taken care of by having nsNativeThemeCocoa
+  // return true from NeedToClearBackgroundBehindWidget for vibrant widgets.

Playing around a bit with an interpose library on Yosemite DP6 (the current DP), I confirmed what you say.  If Safari is any guide, Apple normally (always?) uses NSVisualEffectViews (or subclasses thereof) to make "vibrant" the content of that view (or its subviews).  This patch uses them to make vibrant the content the content of their superview(s) -- or rather of that part of their superviews which they overlap.

I haven't tested this.  But I'll take your word for it that it works.

Could you post one or more testcases here?

Your patch only changes the NSView structure for pages where we want to use vibrancy effects.  I don't imagine there will be too many cases of that.  And especially, I don't imagine we'll ever want to make a plugin "vibrant".  It's plugins that are most likely to have problems with changes to the NSView structure.  So I think it's likely we'll be able to get away with this change without causing trouble.
Attachment #8475242 - Flags: review?(smichaud)
Reporter

Comment 10

5 years ago
I don't know why, but part 3 doesn't compile for me. I've tried now several times, but I everytime get:

In file included from /Volumes/Developer/mozilla-central/widget/cocoa/VibrancyManager.mm:7:
/Volumes/Developer/mozilla-central/widget/cocoa/VibrancyManager.h:85:17: error: 
      field has incomplete type 'nsIntRegion'
    nsIntRegion region;
                ^
../../dist/include/nsTArray.h:32:7: note: forward declaration of 'nsIntRegion'
class nsIntRegion;
      ^
SkQuarticRoot.o
In file included from /Volumes/Developer/mozilla-central/widget/cocoa/VibrancyManager.mm:7:
/Volumes/Developer/mozilla-central/widget/cocoa/VibrancyManager.h:91:48: error: 
      unknown type name 'NSRect'
  NSView* CreateEffectView(VibrancyType aType, NSRect aRect);

Is there a patch from another bug needed to make this working?
Assignee

Comment 11

5 years ago
Fixed the build errors by adding the missing #includes, and added a check for VibrancyManager::SystemSupportsVibrancy() to nsChildView::ClearVibrantAreas() so that we don't crash on pre-10.10.
Attachment #8475242 - Attachment is obsolete: true
Assignee

Comment 12

5 years ago
(In reply to Steven Michaud from comment #9)
> Could you post one or more testcases here?

I've attached a patch to bug 1059278 which you can use to test this. You can open the sidebar using View -> Sidebar -> Bookmarks (or Cmd+B).

> Your patch only changes the NSView structure for pages where we want to use
> vibrancy effects.  I don't imagine there will be too many cases of that.

Actually, web content can't use vibrancy at all. It can only be used by the browser chrome.
(This is ensured by this line: http://hg.mozilla.org/mozilla-central/annotate/0753f7b93ab7/layout/base/nsDisplayList.cpp#l1766 which causes us not to call nsChildView::UpdateThemeGeometries for "-moz-appearance"-styled web content elements.)
You need to log in before you can comment on or make changes to this bug.