From 29dc40aaa5e3741918c54a9050db07d130171660 Mon Sep 17 00:00:00 2001 From: shill-lucasfilm <40578207+shill-lucasfilm@users.noreply.github.com> Date: Tue, 3 Sep 2024 21:25:40 -0700 Subject: [PATCH] Fix banding artifacts in macOS viewer (#1977) This addresses subtle banding on macOS with 10 bit / EDR displays. I'm not a Metal expert and I suspect that the conversion code I added for screenshot saving could be improved (see commit comment), or ideally avoided (by saving to an HDR format such as EXR), but it gets the job done. --- source/MaterialXRenderMsl/CMakeLists.txt | 3 +- source/MaterialXRenderMsl/MetalFramebuffer.mm | 2 +- source/MaterialXRenderMsl/MetalState.h | 2 +- source/MaterialXRenderMsl/MetalState.mm | 6 ++-- source/MaterialXView/RenderPipelineMetal.mm | 30 ++++++++++++++----- source/MaterialXView/Viewer.cpp | 7 ++++- 6 files changed, 36 insertions(+), 14 deletions(-) diff --git a/source/MaterialXRenderMsl/CMakeLists.txt b/source/MaterialXRenderMsl/CMakeLists.txt index 296c454e2b..014c99eaae 100644 --- a/source/MaterialXRenderMsl/CMakeLists.txt +++ b/source/MaterialXRenderMsl/CMakeLists.txt @@ -59,7 +59,8 @@ elseif(APPLE) target_link_libraries(${TARGET_NAME} PUBLIC "-framework Foundation" - "-framework Metal") + "-framework Metal" + "-framework MetalPerformanceShaders") elseif(UNIX) target_link_libraries(${TARGET_NAME} PUBLIC diff --git a/source/MaterialXRenderMsl/MetalFramebuffer.mm b/source/MaterialXRenderMsl/MetalFramebuffer.mm index 17ee9de9a1..7309da6bfe 100644 --- a/source/MaterialXRenderMsl/MetalFramebuffer.mm +++ b/source/MaterialXRenderMsl/MetalFramebuffer.mm @@ -81,7 +81,7 @@ height:height mipmapped:NO]; [texDescriptor setStorageMode:MTLStorageModePrivate]; - [texDescriptor setUsage:MTLTextureUsageRenderTarget | MTLTextureUsageShaderRead]; + [texDescriptor setUsage:MTLTextureUsageRenderTarget | MTLTextureUsageShaderRead | MTLTextureUsageShaderWrite]; if (extColorTexture == nil) { diff --git a/source/MaterialXRenderMsl/MetalState.h b/source/MaterialXRenderMsl/MetalState.h index b22470de69..ff742b857c 100644 --- a/source/MaterialXRenderMsl/MetalState.h +++ b/source/MaterialXRenderMsl/MetalState.h @@ -43,7 +43,7 @@ struct MX_RENDERMSL_API MetalState void endEncoder(); void endCommandBuffer(); - void waitForComplition(); + void waitForCompletion(); MaterialX::MetalFramebufferPtr currentFramebuffer(); diff --git a/source/MaterialXRenderMsl/MetalState.mm b/source/MaterialXRenderMsl/MetalState.mm index 01903dbd89..4ce170a6ba 100644 --- a/source/MaterialXRenderMsl/MetalState.mm +++ b/source/MaterialXRenderMsl/MetalState.mm @@ -102,7 +102,7 @@ MTLTileRenderPipelineDescriptor* renderPipelineDescriptor = [MTLTileRenderPipelineDescriptor new]; [renderPipelineDescriptor setRasterSampleCount:1]; - [[renderPipelineDescriptor colorAttachments][0] setPixelFormat:MTLPixelFormatBGRA8Unorm]; + [[renderPipelineDescriptor colorAttachments][0] setPixelFormat:MTLPixelFormatRGBA16Float]; [renderPipelineDescriptor setTileFunction:function]; linearToSRGB_pso = [device newRenderPipelineStateWithTileDescriptor:renderPipelineDescriptor options:0 reflection:nil error:&error]; } @@ -171,7 +171,7 @@ MTLRenderPipelineDescriptor* renderPipelineDesc = [MTLRenderPipelineDescriptor new]; [renderPipelineDesc setVertexFunction:vertexfunction]; [renderPipelineDesc setFragmentFunction:Fragmentfunction]; - [[renderPipelineDesc colorAttachments][0] setPixelFormat:MTLPixelFormatBGRA8Unorm]; + [[renderPipelineDesc colorAttachments][0] setPixelFormat:MTLPixelFormatRGBA16Float]; [renderPipelineDesc setDepthAttachmentPixelFormat:MTLPixelFormatDepth32Float]; linearToSRGB_pso = [device newRenderPipelineStateWithDescriptor:renderPipelineDesc error:&error]; } @@ -225,7 +225,7 @@ [cmdBuffer waitUntilCompleted]; } -void MetalState::waitForComplition() +void MetalState::waitForCompletion() { std::unique_lock lock(inFlightMutex); while (inFlightCommandBuffers != 0) diff --git a/source/MaterialXView/RenderPipelineMetal.mm b/source/MaterialXView/RenderPipelineMetal.mm index 872ec8e10d..59da87a847 100644 --- a/source/MaterialXView/RenderPipelineMetal.mm +++ b/source/MaterialXView/RenderPipelineMetal.mm @@ -15,6 +15,8 @@ #include +#include + namespace { @@ -59,10 +61,10 @@ MTL(device), width * _viewer->m_pixel_ratio, height * _viewer->m_pixel_ratio, - 4, mx::Image::BaseType::UINT8, + 4, mx::Image::BaseType::HALF, MTL(supportsTiledPipeline) ? (id)color_texture : nil, - false, MTLPixelFormatBGRA8Unorm)); + false, MTLPixelFormatRGBA16Float)); } void MetalRenderPipeline::resizeFramebuffer(int width, int height, @@ -697,16 +699,30 @@ unsigned int width = MTL(currentFramebuffer())->getWidth(); unsigned int height = MTL(currentFramebuffer())->getHeight(); - MTL(waitForComplition()); + MTL(waitForCompletion()); + + id srcTexture = MTL(supportsTiledPipeline) ? + (id)_viewer->_colorTexture : + MTL(currentFramebuffer())->getColorTexture(); + mx::MetalFramebufferPtr framebuffer = mx::MetalFramebuffer::create( MTL(device), width, height, 4, mx::Image::BaseType::UINT8, - MTL(supportsTiledPipeline) ? - (id)_viewer->_colorTexture : - MTL(currentFramebuffer())->getColorTexture(), + nil, false, MTLPixelFormatBGRA8Unorm); - mx::ImagePtr frame = framebuffer->getColorImage(MTL(cmdQueue)); + id dstTexture = framebuffer->getColorTexture(); + + id cmdQueue = MTL(cmdQueue); + + // Copy with format conversion + MPSImageConversion* conversion = [[MPSImageConversion alloc] initWithDevice:MTL(device)]; + id cmdBuffer = [cmdQueue commandBuffer]; + [conversion encodeToCommandBuffer:cmdBuffer sourceTexture:srcTexture destinationTexture:dstTexture]; + [cmdBuffer commit]; + [cmdBuffer waitUntilCompleted]; + + mx::ImagePtr frame = framebuffer->getColorImage(cmdQueue); // Flips the captured image std::vector tmp(frame->getRowStride()); diff --git a/source/MaterialXView/Viewer.cpp b/source/MaterialXView/Viewer.cpp index c3910c4f1e..71cccf36c8 100644 --- a/source/MaterialXView/Viewer.cpp +++ b/source/MaterialXView/Viewer.cpp @@ -52,6 +52,11 @@ const float DEFAULT_CAMERA_ZOOM = 1.0f; namespace { +#ifdef MATERIALXVIEW_METAL_BACKEND +const bool USE_FLOAT_BUFFER = true; +#else +const bool USE_FLOAT_BUFFER = false; +#endif const int MIN_ENV_SAMPLE_COUNT = 4; const int MAX_ENV_SAMPLE_COUNT = 1024; @@ -152,7 +157,7 @@ Viewer::Viewer(const std::string& materialFilename, int screenHeight, const mx::Color3& screenColor) : ng::Screen(ng::Vector2i(screenWidth, screenHeight), "MaterialXView", - true, false, true, true, false, 4, 0), + true, false, true, true, USE_FLOAT_BUFFER, 4, 0), _window(nullptr), _materialFilename(materialFilename), _meshFilename(meshFilename),