Skip to content

Commit 29dc40a

Browse files
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.
1 parent 9ad29d5 commit 29dc40a

File tree

6 files changed

+36
-14
lines changed

6 files changed

+36
-14
lines changed

source/MaterialXRenderMsl/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ elseif(APPLE)
5959
target_link_libraries(${TARGET_NAME}
6060
PUBLIC
6161
"-framework Foundation"
62-
"-framework Metal")
62+
"-framework Metal"
63+
"-framework MetalPerformanceShaders")
6364
elseif(UNIX)
6465
target_link_libraries(${TARGET_NAME}
6566
PUBLIC

source/MaterialXRenderMsl/MetalFramebuffer.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@
8181
height:height
8282
mipmapped:NO];
8383
[texDescriptor setStorageMode:MTLStorageModePrivate];
84-
[texDescriptor setUsage:MTLTextureUsageRenderTarget | MTLTextureUsageShaderRead];
84+
[texDescriptor setUsage:MTLTextureUsageRenderTarget | MTLTextureUsageShaderRead | MTLTextureUsageShaderWrite];
8585

8686
if (extColorTexture == nil)
8787
{

source/MaterialXRenderMsl/MetalState.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ struct MX_RENDERMSL_API MetalState
4343
void endEncoder();
4444
void endCommandBuffer();
4545

46-
void waitForComplition();
46+
void waitForCompletion();
4747

4848
MaterialX::MetalFramebufferPtr currentFramebuffer();
4949

source/MaterialXRenderMsl/MetalState.mm

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@
102102

103103
MTLTileRenderPipelineDescriptor* renderPipelineDescriptor = [MTLTileRenderPipelineDescriptor new];
104104
[renderPipelineDescriptor setRasterSampleCount:1];
105-
[[renderPipelineDescriptor colorAttachments][0] setPixelFormat:MTLPixelFormatBGRA8Unorm];
105+
[[renderPipelineDescriptor colorAttachments][0] setPixelFormat:MTLPixelFormatRGBA16Float];
106106
[renderPipelineDescriptor setTileFunction:function];
107107
linearToSRGB_pso = [device newRenderPipelineStateWithTileDescriptor:renderPipelineDescriptor options:0 reflection:nil error:&error];
108108
}
@@ -171,7 +171,7 @@
171171
MTLRenderPipelineDescriptor* renderPipelineDesc = [MTLRenderPipelineDescriptor new];
172172
[renderPipelineDesc setVertexFunction:vertexfunction];
173173
[renderPipelineDesc setFragmentFunction:Fragmentfunction];
174-
[[renderPipelineDesc colorAttachments][0] setPixelFormat:MTLPixelFormatBGRA8Unorm];
174+
[[renderPipelineDesc colorAttachments][0] setPixelFormat:MTLPixelFormatRGBA16Float];
175175
[renderPipelineDesc setDepthAttachmentPixelFormat:MTLPixelFormatDepth32Float];
176176
linearToSRGB_pso = [device newRenderPipelineStateWithDescriptor:renderPipelineDesc error:&error];
177177
}
@@ -225,7 +225,7 @@
225225
[cmdBuffer waitUntilCompleted];
226226
}
227227

228-
void MetalState::waitForComplition()
228+
void MetalState::waitForCompletion()
229229
{
230230
std::unique_lock<std::mutex> lock(inFlightMutex);
231231
while (inFlightCommandBuffers != 0)

source/MaterialXView/RenderPipelineMetal.mm

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
#include <nanogui/messagedialog.h>
1717

18+
#include <MetalPerformanceShaders/MetalPerformanceShaders.h>
19+
1820
namespace
1921
{
2022

@@ -59,10 +61,10 @@
5961
MTL(device),
6062
width * _viewer->m_pixel_ratio,
6163
height * _viewer->m_pixel_ratio,
62-
4, mx::Image::BaseType::UINT8,
64+
4, mx::Image::BaseType::HALF,
6365
MTL(supportsTiledPipeline) ?
6466
(id<MTLTexture>)color_texture : nil,
65-
false, MTLPixelFormatBGRA8Unorm));
67+
false, MTLPixelFormatRGBA16Float));
6668
}
6769

6870
void MetalRenderPipeline::resizeFramebuffer(int width, int height,
@@ -697,16 +699,30 @@
697699
unsigned int width = MTL(currentFramebuffer())->getWidth();
698700
unsigned int height = MTL(currentFramebuffer())->getHeight();
699701

700-
MTL(waitForComplition());
702+
MTL(waitForCompletion());
703+
704+
id<MTLTexture> srcTexture = MTL(supportsTiledPipeline) ?
705+
(id<MTLTexture>)_viewer->_colorTexture :
706+
MTL(currentFramebuffer())->getColorTexture();
707+
701708
mx::MetalFramebufferPtr framebuffer = mx::MetalFramebuffer::create(
702709
MTL(device),
703710
width, height, 4,
704711
mx::Image::BaseType::UINT8,
705-
MTL(supportsTiledPipeline) ?
706-
(id<MTLTexture>)_viewer->_colorTexture :
707-
MTL(currentFramebuffer())->getColorTexture(),
712+
nil,
708713
false, MTLPixelFormatBGRA8Unorm);
709-
mx::ImagePtr frame = framebuffer->getColorImage(MTL(cmdQueue));
714+
id<MTLTexture> dstTexture = framebuffer->getColorTexture();
715+
716+
id<MTLCommandQueue> cmdQueue = MTL(cmdQueue);
717+
718+
// Copy with format conversion
719+
MPSImageConversion* conversion = [[MPSImageConversion alloc] initWithDevice:MTL(device)];
720+
id<MTLCommandBuffer> cmdBuffer = [cmdQueue commandBuffer];
721+
[conversion encodeToCommandBuffer:cmdBuffer sourceTexture:srcTexture destinationTexture:dstTexture];
722+
[cmdBuffer commit];
723+
[cmdBuffer waitUntilCompleted];
724+
725+
mx::ImagePtr frame = framebuffer->getColorImage(cmdQueue);
710726

711727
// Flips the captured image
712728
std::vector<unsigned char> tmp(frame->getRowStride());

source/MaterialXView/Viewer.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ const float DEFAULT_CAMERA_ZOOM = 1.0f;
5252

5353
namespace
5454
{
55+
#ifdef MATERIALXVIEW_METAL_BACKEND
56+
const bool USE_FLOAT_BUFFER = true;
57+
#else
58+
const bool USE_FLOAT_BUFFER = false;
59+
#endif
5560

5661
const int MIN_ENV_SAMPLE_COUNT = 4;
5762
const int MAX_ENV_SAMPLE_COUNT = 1024;
@@ -152,7 +157,7 @@ Viewer::Viewer(const std::string& materialFilename,
152157
int screenHeight,
153158
const mx::Color3& screenColor) :
154159
ng::Screen(ng::Vector2i(screenWidth, screenHeight), "MaterialXView",
155-
true, false, true, true, false, 4, 0),
160+
true, false, true, true, USE_FLOAT_BUFFER, 4, 0),
156161
_window(nullptr),
157162
_materialFilename(materialFilename),
158163
_meshFilename(meshFilename),

0 commit comments

Comments
 (0)