macOS Browser Panel Support

[tbodt]

Description

Mac support for browser panels.

Depends on obsproject/obs-studio#2386

Motivation and Context

How Has This Been Tested?

Build and run on 10.14.6, click around, not that many things crash

Types of changes

• New feature (non-breaking change which adds functionality)

Checklist:

• My code has been run through clang-format.
• I have read the contributing document.
• My code is not on the master branch.
• The code has been tested.
• All commit messages are properly formatted and commits squashed where appropriate.

[tbodt]

TODO:

• copy/paste with keyboard shortcuts and menu bar
• automatically symlink CEF into rundir/Frameworks
• (stretch goal) script to download and build the right version of CEF

[tbodt]

The packaging is incredibly janky, so I'm just going to say it's Somebody Else's Problem

[jp9000]

Packaging?

[tbodt]

Getting Chromium Embedded Framework.framework into the right place relative to the executable. Though it's possible this has already been solved by the CI scripts and it's just broken for me because I don't build the same way as CI.

[jp9000]

Yea I'm confused, we already include it for the browser source. Were you meaning something different?

[tbodt]

No, you understood right. I guess that's not actually a problem.

Is there some way to package the app in the same way that CI does but locally?

[jp9000]

Short answer is not very easily at the moment. Long answer is we used to be able to do that with the bundle_app.py script but some things changed and now the CI scripts seem to have some added (and probably unnecessary) complexity. I think they should be something that can be simplified in to a single script but it's multiple scripts at the moment on CI. I was going to take a look at it after we release 25 originally.

[tbodt]

It's very hard to resize panels with this. Seems like you need to get the mouse onto exactly the pixel between the panels, or the panel will grab the mouse event.

[PatTheMav]

@tbodt Checked this out today, resizing is indeed hard - even if the mouse cursor changed to the vertical/horizontal resize variant, you end up creating a selection box in the canvas.

Also when closed in detached state and then reopened, the reopened window can't be resized, but dragged and re-attached.

Panels stay in their attached and detached places between OBS sessions, so that part seems to work fine.

[PatTheMav]

Did some further checks today, most functionality seems fine (i.e. renaming the title, opening links in a new tab when modifier is pressed), the size is restored correctly for attached panels, but detached panels are reset, maybe the Resize() method needs to be implemented for macOS as well.

Reloading a panel is not possible as the default keystroke CMD+R is already taken by OBS to reset transforms, might be better to move those transform-keystrokes to CMD+Option+R, etc.

As for packaging, CEF expects to be placed in a proper macOS app bundle which CMake doesn't generate, so the framework references need to be patched, which Cmake would do automatically when -DBROWSER_DEPLOY=OFF is used, but that part needs to be updated and even with it in place OBS crashes right now.

I've added a PR to the main project with a cleaned-up build script which just needs to be run with -s -b to create an app bundle that will run with all necessary libraries.

[dodgepong]

Reloading a panel is not possible as the default keystroke CMD+R is already taken by OBS to reset transforms, might be better to move those transform-keystrokes to CMD+Option+R, etc.

Maybe this is as good a time as any to implement a refresh button in the title bar of the dock itself.

[WizardCM]

It's very hard to resize panels with this. Seems like you need to get the mouse onto exactly the pixel between the panels, or the panel will grab the mouse event.

Would adding a couple pixels of margin around the CEF wrapper in the dock do the trick? Likely best to add it on all sides.

[PatTheMav]

From what I could tell this is an issue with all panels on macOS not just with the browser panel.

Aside from the compilation fix above I'd say that the non-resizable window that you end up when

1. Detaching
2. Closing
3. Re-opening

the panel needs to be fixed as well and then this might be good to go. Haven't had time to figure out to test the built-in Twitch panels with oAuth-flow though.

[WizardCM]

From what I could tell this is an issue with all panels on macOS not just with the browser panel.

Weird, because that's supposed to be controlled by macOS. I guess time to experiment with these? According to a quick Google, modifying the styling of the widget itself won't help.

[WizardCM]

Follow-up from my previous comment - after experimenting with docks on Windows, I can (unfortunately) confirm that docks are locked to the Qt::Tool window type, which means we have very little more we can do (as in, we can't use "bigger" native decorations).

Technically, Qt supports custom window decorations, but in my testing on Windows the grips around the window turn from 5px to 1px in custom mode, and I expect the behaviour would be similar on other platforms. It's a terrible experience and I wouldn't recommend the switch.

[eric]

I'm running into the objc_msgSend issue when I try to build this

[PatTheMav]

@eric if I'm not mistaken this issue will occur when a more recent version of platform libraries or clang is used, so it's a must fix in my book.

[eric]

Is someone else going to take up the work on this pull request?

[tbodt]

For the curious, I'm pretty sure they changed the objc_msgSend prototype because they knew the varargs ABI wouldn't be compatible with normal parameters on the upcoming ARM macs, and decided to change the prototype a year early to get everyone ready

[tbodt]

Went ahead and rebased this as well

[zavitax]

Oh, I'm talking about docked panels. Didn't realize you were talking about something different, since they both have some kind of resize grip.

There you have it. I was not aware docked state even has an issue.

[zavitax]

Some more info from Qt's docs:
https://doc.qt.io/qt-5/stylesheet-reference.html

QDockWidget
Supports styling of the title bar and the title bar buttons when docked.
The dock widget border can be styled using the border property. The ::title subcontrol can be used to customize the title bar. The close and float buttons are positioned with respect to the ::title subcontrol using the ::close-button and ::float-button respectively.

When the title bar is vertical, the :vertical pseudo class is set. In addition, depending on QDockWidget::DockWidgetFeature, the :closable, :floatable and :movable pseudo states are set.

Note: Use QMainWindow::separator to style the resize handle.

Warning: The style sheet has no effect when the QDockWidget is undocked as Qt uses native top level windows when undocked.

[tbodt]

So far I've figured out that despite the splitter being 5px wide (in OBS default dark theme) its QWidgetWindow has a mask that ignores any input outside a 1px wide region. I'm not sure where this mask comes from. The QSplitter widget sets a mask on itself, but my understanding is that widget masks affect only drawing, not input. On top of this it looks like there might be a bug in Qt handling of mouse events on masked windows, where the mouse down event passes through but not the mouse drag or mouse up event.

[WizardCM]

Unrelated to the current discussion, I just realised a discrepency of featureset - on Windows, we listen for a Ctrl+R keypress to allow users to reload the panel's contents. A similar condition should be added for macOS (I assume Cmd+R would be best?).

obs-browser/panel/browser-panel-client.cpp

Lines 200 to 209 in 35b26e9

	#ifdef _WIN32
	if (event.type != KEYEVENT_RAWKEYDOWN)
	return false;
	if (event.windows_key_code == 'R' &&
	(event.modifiers & EVENTFLAG_CONTROL_DOWN) != 0) {
	browser->ReloadIgnoreCache();
	return true;
	}
	#endif

[PatTheMav]

As mentioned in #197 (comment):

Reloading a panel is not possible as the default keystroke CMD+R is already taken by OBS to reset transforms, might be better to move those transform-keystrokes to CMD+Option+R, etc.

Even better would be non-global keyboard shortcuts (that is shortcuts that change based on window focus) but that's not how Qt works iirc.

[WizardCM]

At least on Windows, when CEF is focused, Qt doesn't have focus so default shortcuts don't work. I assume this behaviour is different on macOS due to the command bar? I assume you've verified the behaviour yourself by adding in the relevant check?

[PatTheMav]

@WizardCM I wouldn't know where to implement those checks, but I can confirm that OBS intercepts all keystrokes, so even copy&paste functionality is intercepted (so no pasting passwords into the oauth panels). Right-clicking in the CEF window and choosing "copy/paste" works however.

[PatTheMav]

This needs to be changed at least for macOS:

obs-browser/panel/browser-panel.cpp

Line 83 in 1c34fd7

BPtr<char> path = os_get_abs_path_ptr(rpath.Get());

obs_module_config_path already returns an absolute path on macOS (possibly all POSIX-based systems?) and for some reason os_get_abs_path_ptr will fail (as realpath will return an error, probably because the path didn't exist on first run).

Possible fix:

#ifdef _WIN32
		BPtr<char> rpath = obs_module_config_path(storage_path.c_str());
		BPtr<char> path = os_get_abs_path_ptr(rpath.Get());
#else
        BPtr<char> path = obs_module_config_path(storage_path.c_str());
#endif

With this Twitch oauth and panels worked fine, as did chat addons.

[WizardCM]

@PatTheMav I can confirm on Ubuntu 20.04 that that line requires changing too. Nice catch!

[PatTheMav]

I've seen @tbodt pushed some requested changes - great! Would you mind applying this change as well? macOS and Linux require it: #197 (comment)

[tbodt]

@PatTheMav I can't reproduce the crash. Is there something special I need to do?

[PatTheMav]

@PatTheMav I can't reproduce the crash. Is there something special I need to do?

You need to delete the obs_profile_cookies directory under Application Support/obs-studio/plugin_config/obs-browser if it exists. When you launch OBS without that fix it will crash.

---

I've tested this PR with and without Twitch OAUTH support and also with CEF 3770 and CEF 4183, works fine with both versions. Our patched Qt 5.15.2 fixes the resize bug with detached panels and otherwise I didn't run into unexpected behaviour.

I will try testing this with a debug build tomorrow to catch any low-level CEF issues, but looks fine so far.

[PatTheMav]

I tested this today as well, deleted the obs-browser plugin config directory before and did not get any crash so far. Also tested our Twitch authentication with it, all worked as expected (including the quirks known from the Windows side of things).

Good to merge from my POV.

[WizardCM]

Confirmed everything works very well with latest OBS master on 10.14. I was unable to reproduce the quirks others have mentioned, so I have to assume they are introduced in 10.15 and above.

[tbodt]

🚀