Page MenuHome

Use `CAMetalLayer` in place of `NSOpenGLView` for presentation
Needs ReviewPublic

Authored by Tomoaki Kawada (yvt) on Mar 30 2019, 8:24 AM.

Details

Summary

This patch updates GHOST_ContextCGL and GHOST_WindowCocoa to present OpenGL contents via CAMetalLayer. This has been found to resolve the frame skipping issue described in T60043.

I think this patch may be rather incomplete. Specifically:

  • It assumes the system has a Metal-capable GPU. It will likely crash with "nil MTLDevice" on other systems. The old code should be preserved as a fall back if old systems need to be supported.
  • The virtual default framebuffer only has a non-multisample color buffer - no depth, stencil or MSAA. I haven't encountered an issue with this so far. Handling of them in GHOST was removed in eda7e84aac661f19478b2e5fe94d8899c9782cc8.
  • It always uses the system default GPU, so it'll be slightly inefficient for external displays connected to eGPUs, but this saves us handling eGPU notifications.

I'd like a feedback on which of these should be addressed in this patch.

Diff Detail

Repository
rB Blender
Branch
patch-metal-present (branched from master)
Build Status
Buildable 3506
Build 3506: arc lint + arc unit

Event Timeline

Tomoaki Kawada (yvt) added a comment.EditedApr 16 2019, 6:27 PM

I found two minor issues with this patch and I don't have a solution at this moment:

  • When working in a full-screen mode, the window contents sometimes revert to an old state. The problem seems to be triggered by macOS's notification banner and persists until the window is updated.
  • When working in a full-screen mode, the display brightness varies depending on whether the window is occluded by something (e.g., the system menubar). This does not occur when the screen is on the maximum brightness.
  • Screen tearing-like effect is observed while doing lasso select. Perhaps the window contents are presented before the lasso shape is fully rendered.

For the first two issues, I suspect that CAMetalLayer has some kind of exclusive full-screen mode which is automatically activated when the window is in a full-screen mode and not occluded by anything and I think there's a problem transitioning from and to this mode.

  • Merge remote-tracking branch 'origin/master' into patch-metal-present
  • Convert the indentation of Metal source code to spaces
  • Revert accidental mix-up with D4580

Overall the patch looks clean.

I cannot give any feedback on the Metal implementation and/nor test it because I don't own a mac.

So for my part it's mostly OK.

intern/ghost/GHOST_C-api.h
726

For API consistency (and be more future proof) I would prefer to have a GHOST_FramebufferHandle as type. Even if it is cast into uint after in the case of OpenGL.

intern/ghost/intern/GHOST_Window.h
255

Follow style and make a description.

source/blender/windowmanager/intern/wm_playanim.c
1290–1291

Code Style: Avoid nested function calls.

source/blender/windowmanager/intern/wm_window.c
731–732

Same here

  • Merge remote-tracking branch 'origin/master' into patch-metal-present
  • Merge remote-tracking branch 'origin/master' into patch-metal-present
  • Fix accidental breakage introduced in merge commits
  • Add documentation comments for getDefaultFramebuffer
  • Conform with the coding style: Avoid nested function calls
Tomoaki Kawada (yvt) marked 3 inline comments as done.Sat, May 4, 2:48 PM
Tomoaki Kawada (yvt) added inline comments.
intern/ghost/GHOST_C-api.h
726

Isn't GHOST_*Handle reserved for opaque pointers backed by actual C++ classes?

Tomoaki Kawada (yvt) edited the summary of this revision. (Show Details)Sat, May 4, 2:52 PM
Tomoaki Kawada (yvt) edited the summary of this revision. (Show Details)