Washed-out colors / HDR issues?

[fiserj]

Hi, first thank you for all the outstanding work!

I've noticed that on my recent-ish Mac, the colors are somewhat washed out, compared to GLFW.

I hope the issue is going to be visible in the side-by-side screenshot (I'll take a photo of the monitor if not):

The repro is below, it's just setting up the window with Sokol / GLFW and clearing screen.

Sokol

#define SOKOL_IMPL
#define SOKOL_METAL

#include <sokol_app.h>
#include <sokol_gfx.h>
#include <sokol_glue.h>

void init() {
  sg_setup({.environment = sglue_environment()});
}

void frame() {
  sg_pass_action pass_action = {};
  pass_action.colors[0]      = {
         .load_action = SG_LOADACTION_CLEAR,
         .clear_value = {1.0f, 0.0f, 0.0f, 1.0f},
  };

  sg_begin_pass({.action = pass_action, .swapchain = sglue_swapchain()});
  sg_end_pass();
  sg_commit();
}

void cleanup() {
  sg_shutdown();
}

sapp_desc sokol_main(int, char**) {
  return {
    .init_cb      = init,
    .frame_cb     = frame,
    .cleanup_cb   = cleanup,
    .width        = 640,
    .height       = 480,
    .window_title = "Sokol Metal",
  };
}

GLFW + Metal (but works the same with OpenGL), code from here.

#define GLFW_INCLUDE_NONE
#define GLFW_EXPOSE_NATIVE_COCOA
#include <GLFW/glfw3.h>
#include <GLFW/glfw3native.h>

#import <Metal/Metal.h>
#import <QuartzCore/CAMetalLayer.h>

int main(void) {
  const id<MTLDevice>       gpu       = MTLCreateSystemDefaultDevice();
  const id<MTLCommandQueue> queue     = [gpu newCommandQueue];
  CAMetalLayer*             swapchain = [CAMetalLayer layer];
  swapchain.device                    = gpu;
  swapchain.opaque                    = YES;

  glfwInit();
  glfwWindowHint(GLFW_CLIENT_API, GLFW_NO_API);

  GLFWwindow* window              = glfwCreateWindow(640, 480, "GLFW Metal", NULL, NULL);
  NSWindow*   nswindow            = glfwGetCocoaWindow(window);
  nswindow.contentView.layer      = swapchain;
  nswindow.contentView.wantsLayer = YES;

  MTLClearColor color = MTLClearColorMake(1, 0, 0, 1);

  while (!glfwWindowShouldClose(window)) {
    glfwPollEvents();

    @autoreleasepool {
      id<CAMetalDrawable> surface = [swapchain nextDrawable];

      MTLRenderPassDescriptor* pass        = [MTLRenderPassDescriptor renderPassDescriptor];
      pass.colorAttachments[0].clearColor  = color;
      pass.colorAttachments[0].loadAction  = MTLLoadActionClear;
      pass.colorAttachments[0].storeAction = MTLStoreActionStore;
      pass.colorAttachments[0].texture     = surface.texture;

      id<MTLCommandBuffer>        buffer  = [queue commandBuffer];
      id<MTLRenderCommandEncoder> encoder = [buffer renderCommandEncoderWithDescriptor:pass];
      [encoder endEncoding];
      [buffer presentDrawable:surface];
      [buffer commit];
    }
  }

  glfwDestroyWindow(window);
  glfwTerminate();

  return 0;
}

For what it's worth, the Display settings on my Mac is like so:

When I select Internet & Web (sRGB) profile, the GLFW example also gets washed out, so it looks like GLFW is somehow respecting the current profile. I quickly skimmed through the code but didn't see anything special in the way the window was set up in GLFW.

Thank you!

[floooh]

It's surprising that Metal behaves differently here, I would have expected that GL is the issue (since sokol_gfx.h has a hack here so that the GL backend behaves the same as the Metal and D3D11 backend when it comes to SRGB handling:

sokol/sokol_gfx.h

Lines 9123 to 9145 in c1cc713

	// FIXME: Disabling SRGB conversion for the default framebuffer is
	// a crude hack to make behaviour for sRGB render target textures
	// identical with the Metal and D3D11 swapchains created by sokol-app.
	//
	// This will need a cleaner solution (e.g. allowing to configure
	// sokol_app.h with an sRGB or RGB framebuffer.
	if (atts) {
	// offscreen pass
	SOKOL_ASSERT(atts->gl.fb);
	#if defined(SOKOL_GLCORE)
	glEnable(GL_FRAMEBUFFER_SRGB);
	#endif
	glBindFramebuffer(GL_FRAMEBUFFER, atts->gl.fb);
	} else {
	// default pass
	#if defined(SOKOL_GLCORE)
	glDisable(GL_FRAMEBUFFER_SRGB);
	#endif
	// NOTE: on some platforms, the default framebuffer of a context
	// is null, so we can't actually assert here that the
	// framebuffer has been provided
	glBindFramebuffer(GL_FRAMEBUFFER, swapchain->gl.framebuffer);
	}

...in this commit:

...to find the related changelog entry search for 26-Jan-2023: https://github.com/floooh/sokol/blob/master/CHANGELOG.md

One potential difference might be the pixel format in CAMetalLayer vs sokol_app.h, but according to the documentation, both use BGRA8Unorm:

https://developer.apple.com/documentation/quartzcore/cametallayer/pixelformat?language=objc

vs:

sokol/sokol_app.h

Line 3993 in c1cc713

_sapp.macos.view.colorPixelFormat = MTLPixelFormatBGRA8Unorm;

...another difference might be the MTKView colorspace property which might kick off a colorspace conversion in the window system:

https://developer.apple.com/documentation/metalkit/mtkview/colorspace?language=objc

...but I'm not setting that anywhere in sokol_app.h (but MTKView's default of nil seems to be the same as CAMetalLayer: https://developer.apple.com/documentation/quartzcore/cametallayer/colorspace?language=objc, and I'm also not aware of anything else in the Metal backend that might explain the difference, but the reason is most likely lurking in sokol_app.h, not sokol_gfx.h (something in the way the window and swapchain is created vs GLFW and CAMetalLayer). Currently sokol_app.h doesn't use CAMetalLayer directly but the MTKView wrapper class.

There's also this discussion thread:

https://forums.developer.apple.com/forums/thread/745810

TL;DR: as far as I'm aware, there shouldn't be any difference between sokol_app.h's use of MTKView versus your code's CAMetalLayer, but apparently there is...

[floooh]

PS: also see this open issue: #713 and this: #758

(but I'm not sure if the issue you're seeing is about linear RGB vs sRGB surfaces, or about the general colorspace conversion stuff in the macOS composer)

E.g. Filament explicitly sets a color space on their macOS window:

https://github.com/google/filament/blob/29d2d819e345a124057d0af0c6727cc87fb56f6f/libs/filamentapp/src/NativeWindowHelperCocoa.mm#L42

...but GLFW doesn't seem to do anything like that (and sokol_app.h also doesn't).

[floooh]

Long story short: sokol_app.h needs more explicit control over RGB vs sRGB and color profiles (but at least the latter is probably not portable between platforms).

[fiserj]

Thank you for all the pointers, I'll see if I can find something.

Interestingly, CAMetalLayer docs, as you pointed out, say that the default pixel format is MTLPixelFormatBGRA8Unorm but when I explicitly set it as such:

// Added to the above GLFW + Metal code.
swapchain.pixelFormat = MTLPixelFormatBGRA8Unorm;

I get the same washed-out colors (!). But when I query it before setting, it already has that value, so I'm puzzled as to what is going on.

[floooh]

Ok, that's really weird :D Apple docs being out of date wouldn't be surprising, but the property returning MTLPixelFormatBGRA8Unorm even though it behaves like another format (most likely MTLPixelFormatBGRA8Unorm_sRGB is weird).

ObjC properties are setter/getter methods under the hood, maybe there's another object below CAMetalLayer which now has a different default, but CAMetalLayer maintains its own 'cached' pixel format value which is then out-of-sync until explicitly set. Just speculation though.

[fiserj]

I did try adding implementation of (CALayer*)makeBackingLayer into _sapp_macos_view, returning a new CAMetalLayer but with no luck. It gets called during the view initialization _sapp.macos.view = [[_sapp_macos_view alloc] init]; as expected but immediately afterwards, if I take a look at _sapp.macos.view.layer, the view's address differ, so I suspect it gets overriden again.

So I made the following addition to _sapp_macos_view interface implementation:

- (CALayer*)makeBackingLayer {
    CAMetalLayer* layer = [[CAMetalLayer alloc] init];
    NSLog(@"makeBackingLayer created layer: %p", layer);
    return layer;
}
- (instancetype)initWithFrame:(CGRect)frame device:(id<MTLDevice>)device {
    self = [super initWithFrame:frame device:device];
    if (self) {
        NSLog(@"after init, layer is: %p", self.layer);
    }
    return self;
}
- (void)setLayer:(CALayer *)layer {
    NSLog(@"setLayer called with: %p", layer);
    [super setLayer:layer];
}

And indeed, it seems like setLayer is being called twice:

makeBackingLayer created layer: 0x102f93b90
setLayer called with: 0x102f93b90
setLayer called with: 0x102f94b50
after init, layer is: 0x102f94b50

I'll have to take a look at the call stack of setLayer to see what's going in both cases it's being invoked. I'm by no means an expert in Apple's frameworks, and also doesn't know if injecting a fresh Metal layer helps, but will keep going down this rabbit hole a bit longer, I guess 🙃.

[fiserj]

OK, so I did "solve" it by essentially patching the view in sapp's init callback like in the GLFW example above, and then patching sglue_swapchain to manually set the layer's drawableSize and get nextDrawable.

Note that I'm patching neither the depth-stencil (nor the msaa color texture), so that might be eventually needed as well, but I currently don't need depth buffer in my toy project, so it works as is.

Leaving my "solution" here in case anyone else faces the same issue. I'm afraid it would require more poking, to understand what's really going on and make a proper workaround.

static CAMetalLayer* g_layer = nil;

void patch_sokol_app_macos() {
#if defined(SOKOL_METAL) && defined(_SAPP_MACOS)
  assert(g_layer == nil);

  g_layer        = [CAMetalLayer layer];
  g_layer.device = _sapp.macos.mtl_device;
  g_layer.opaque = YES;

  _sapp.macos.view.layer      = g_layer;
  _sapp.macos.view.wantsLayer = YES;
#endif
}

sg_swapchain patched_sglue_swapchain() {
#if !(defined(SOKOL_METAL) && defined(_SAPP_MACOS))
  return sglue_swapchain();
#else

  g_layer.drawableSize         = CGSizeMake(sapp_width(), sapp_height());
  id<CAMetalDrawable> drawable = [g_layer nextDrawable];

  sg_swapchain swapchain                = {};
  swapchain.width                       = sapp_width();
  swapchain.height                      = sapp_height();
  swapchain.sample_count                = sapp_sample_count();
  swapchain.color_format                = (sg_pixel_format)sapp_color_format();
  swapchain.depth_format                = (sg_pixel_format)sapp_depth_format();
  swapchain.metal.current_drawable      = (__bridge const void*)drawable;
  swapchain.metal.depth_stencil_texture = sapp_metal_get_depth_stencil_texture(); // Might need to be patched, too.
  swapchain.metal.msaa_color_texture    = sapp_metal_get_msaa_color_texture();    // Might need to be patched, too.
  swapchain.d3d11.render_view           = sapp_d3d11_get_render_view();
  swapchain.d3d11.resolve_view          = sapp_d3d11_get_resolve_view();
  swapchain.d3d11.depth_stencil_view    = sapp_d3d11_get_depth_stencil_view();
  swapchain.wgpu.render_view            = sapp_wgpu_get_render_view();
  swapchain.wgpu.resolve_view           = sapp_wgpu_get_resolve_view();
  swapchain.wgpu.depth_stencil_view     = sapp_wgpu_get_depth_stencil_view();
  swapchain.gl.framebuffer              = sapp_gl_get_framebuffer();

  return swapchain;
#endif
}

[fiserj]

I spoke too soon 😂. Acquiring the depth-stencil texture in sapp_metal_get_depth_stencil_texture aborts the program when I try to resize the window, but I assume the fix should be straightforward now.

[floooh]

Thanks for the investigation! It's on my list to kick out MTKView and go one level lower to CAMetalLayer and CVDisplayLink (or whatever that is called nowadays), I guess it makes sense to wait with the proper fix until that is done (but tbh I don't know yet when I'll do that, I want to tackle compute shader support in sokol-gfx first to get that out of the way).

[fiserj]

That's understandable. I have this workaround for now. Compute shaders sound awesome. I can't imagine managing a project like sokol 🙂!

Not sure if you want to close the issue or keep it alive, but either is fine by me.

[floooh]

I'll keep the issue open until I get around to working on a fix.