Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARC / Autorelease fixes with metal-cpp and extensions #5403

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions backends/imgui_impl_metal.mm
Original file line number Diff line number Diff line change
Expand Up @@ -92,32 +92,32 @@ - (MetalBuffer*)dequeueReusableBufferOfLength:(NSUInteger)length device:(id<MTLD

bool ImGui_ImplMetal_Init(MTL::Device* device)
{
return ImGui_ImplMetal_Init((id<MTLDevice>)(device));
return ImGui_ImplMetal_Init((__bridge id<MTLDevice>)(device));
}

void ImGui_ImplMetal_NewFrame(MTL::RenderPassDescriptor* renderPassDescriptor)
{
ImGui_ImplMetal_NewFrame((MTLRenderPassDescriptor*)(renderPassDescriptor));
ImGui_ImplMetal_NewFrame((__bridge MTLRenderPassDescriptor*)(renderPassDescriptor));
}

void ImGui_ImplMetal_RenderDrawData(ImDrawData* draw_data,
MTL::CommandBuffer* commandBuffer,
MTL::RenderCommandEncoder* commandEncoder)
{
ImGui_ImplMetal_RenderDrawData(draw_data,
(id<MTLCommandBuffer>)(commandBuffer),
(id<MTLRenderCommandEncoder>)(commandEncoder));
(__bridge id<MTLCommandBuffer>)(commandBuffer),
(__bridge id<MTLRenderCommandEncoder>)(commandEncoder));

}

bool ImGui_ImplMetal_CreateFontsTexture(MTL::Device* device)
{
return ImGui_ImplMetal_CreateFontsTexture((id<MTLDevice>)(device));
return ImGui_ImplMetal_CreateFontsTexture((__bridge id<MTLDevice>)(device));
}

bool ImGui_ImplMetal_CreateDeviceObjects(MTL::Device* device)
{
return ImGui_ImplMetal_CreateDeviceObjects((id<MTLDevice>)(device));
return ImGui_ImplMetal_CreateDeviceObjects((__bridge id<MTLDevice>)(device));
}

#endif // #ifdef IMGUI_IMPL_METAL_CPP
Expand Down Expand Up @@ -413,7 +413,7 @@ static void ImGui_ImplMetal_CreateWindow(ImGuiViewport* viewport)
CAMetalLayer* layer = [CAMetalLayer layer];
layer.device = device;
layer.framebufferOnly = YES;
layer.pixelFormat = MTLPixelFormatBGRA8Unorm;
layer.pixelFormat = bd->SharedMetalContext.framebufferDescriptor.colorPixelFormat;
#if TARGET_OS_OSX
NSWindow* window = (__bridge NSWindow*)handle;
NSView* view = window.contentView;
Expand Down Expand Up @@ -589,8 +589,8 @@ - (instancetype)init
{
if ((self = [super init]))
{
_renderPipelineStateCache = [NSMutableDictionary dictionary];
_bufferCache = [NSMutableArray array];
self.renderPipelineStateCache = [NSMutableDictionary dictionary];
self.bufferCache = [NSMutableArray array];
_lastBufferCachePurge = GetMachAbsoluteTimeInSeconds();
}
return self;
Expand Down
18 changes: 18 additions & 0 deletions backends/imgui_impl_osx.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,27 @@

#include "imgui.h" // IMGUI_IMPL_API

#ifdef __OBJC__

@class NSEvent;
@class NSView;

IMGUI_IMPL_API bool ImGui_ImplOSX_Init(NSView* _Nonnull view);
IMGUI_IMPL_API void ImGui_ImplOSX_Shutdown();
IMGUI_IMPL_API void ImGui_ImplOSX_NewFrame(NSView* _Nullable view);

#endif

#ifdef IMGUI_IMPL_METAL_CPP_EXTENSIONS

// #include <AppKit/AppKit.hpp>

#ifndef __OBJC__

IMGUI_IMPL_API bool ImGui_ImplOSX_Init(void* _Nonnull view);
IMGUI_IMPL_API void ImGui_ImplOSX_Shutdown();
IMGUI_IMPL_API void ImGui_ImplOSX_NewFrame(void* _Nullable view);

#endif

#endif
12 changes: 12 additions & 0 deletions backends/imgui_impl_osx.mm
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,18 @@ static ImGuiKey ImGui_ImplOSX_KeyCodeToImGuiKey(int key_code)
}
}

#ifdef IMGUI_IMPL_METAL_CPP
Copy link
Contributor

@PathogenDavid PathogenDavid Jun 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be IMGUI_IMPL_METAL_CPP_EXTENSIONS and/or should the header file use IMGUI_IMPL_METAL_CPP instead?

It's not clear if you meant to use the same macro as the Metal backend's C++ support. If it is meant to be the same: Is this actually OK for developers targeting older versions of macOS? (Older versions which maybe have Metal C++ but not full application C++.)

Also should the term "Metal" be used here at all? The imgui_impl_osx backend can be used with OpenGL too. (Edit: I'm seeing now that it seems Apple is using the term "Metal C++ Extensions" for the addition of these not-literally-Metal system APIs to metal-cpp. This seems somewhat confusing to me, but I'll if this is what name macOS developers are going to know for this it's maybe probably fine.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be IMGUI_IMPL_METAL_CPP_EXTENSIONS.

I originally kept it as IMGUI_IMPL_METAL_CPP, but thought for backward compatibility it might be best to not use the same preprocessor for Metal-related and AppKit-related stuff.

I kept it "Metal" because that's how it shows up in Apple's examples, as metal-cpp-extensions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, thanks for clarifying!


IMGUI_IMPL_API bool ImGui_ImplOSX_Init(void* _Nonnull view) {
return ImGui_ImplOSX_Init((__bridge NSView*)(view));
}

IMGUI_IMPL_API void ImGui_ImplOSX_NewFrame(void* _Nullable view) {
return ImGui_ImplOSX_NewFrame((__bridge NSView*)(view));
}

#endif


bool ImGui_ImplOSX_Init(NSView* view)
{
Expand Down