pthom / hello_imgui

Hello, Dear ImGui: unleash your creativity in app development and prototyping
https://pthom.github.io/hello_imgui
MIT License
604 stars 91 forks source link

Metal backend + emscripten favicon and shell template #78

Closed wkjarosz closed 2 months ago

wkjarosz commented 6 months ago

I ran into a couple related issues regarding emscripten.

  1. I see that i can set a custom shell file, but, unlike the built-in shell file, customizing it with CMAKE variables is awkward. The cmake scripts currently check for shell.emscripten.html in the source directory. That means the shell file can't be customize via configure_file with cmake variables, or if I do generate shell.emscripten.html via configure_file, then i am forced to pollute my source tree with a generated file. Possible solutions:
    • check for shell.emscripten.html.in in the source directory first, if present, pass it through configure_file; elif shell.emscripten.html present in source tree, use that without configure_file.
    • check for shell.emscripten.html first in the build directory, and only if not present, also check for shell.emscripten.html in the source directory (this would allow the downstream project to configure the file before calling hello_imgui_add_app)
  2. what is the right way to add additional file dependencies to the shell file? For instance, if i'd like to include an external .js script file within the shell file it would need to be copied to the build directory. so far my workaround has been to just paste the entire contents of the .js file directly in my custom shell file, but that is suboptimal: it makes the shell file very long, and the generated html file has all comments (including copyright) of the embedded code stripped.

Thanks

pthom commented 6 months ago

Hello, The shell.emscripten.html in the source folder was not configured, which is an error. I modified this and added the possibilty to embed additional js files.

See https://github.com/pthom/hello_imgui#app-customization

As you see the app settings are now located inside assets/app_settings (and hello_imgui_add_app enable to specify a custom location for this folder).

wkjarosz commented 6 months ago

Thanks. Works like a charm.

One minor comment: all major browsers now support PNG files for favicons, so, since you already include the icon.png file for emscripten, its not strictly necessary to also convert it to a .ico.

wkjarosz commented 6 months ago

Actually, it turns out that to get the web app to appear with a proper icon on the home screen on iOS and iPadOS, you need to also specify <link rel="apple-touch-icon" ...>, and it seems this file needs to be a PNG (.ico won't work), so perhaps for emscripten its better to just copy the icon over as a PNG so it can be used as both favicon and apple-touch-icon?

pthom commented 6 months ago

Thanks!

We now use png as favicons: see 57487d2bc9b8007127c79b438588390ee2b58234

(where I also added doc for apple Web Clips)

pthom commented 6 months ago

PS: would you mind sharing a screenshot of what you are developing? I'm always interested to see what others develop with my libraries

wkjarosz commented 6 months ago

Sure. I've been porting Samplin' Safari from nanogui to dear/hello imgui. GitHub project, live emscripten app.

pthom commented 6 months ago

Nice, nice, nice! The emscripten app works like a charm. I like the look and feel with square buttons.

How was the experience when comparing HelloImGui to NanoGui?

PS: I guess you also heard about Dear ImGui Bundle which is a library that I developed on top of HelloImGui. It provides accurate python bindings, and lots of additional widgets. However, it is heavier for emscripten distribution, and provide an equivalent cmake imgui_bundle_add_app.

wkjarosz commented 6 months ago

Yes, I'm aware of the bundle and was considering using it, but decided to keep it simple for now. Good to know it's more heavyweight for emscripten. Will keep that in mind.

Regarding NanoGUI vs HelloImGui: HIM made it easier for me to get an emscripten version working. While NG also supports it, it isn't really documented so I never tried. I primarily switched to IM because NG has a pretty limited set of widgets. I wanted docking windows and menu bars. NG does however have better functionality for general OpenGL/Metal drawing (providing an abstraction for shaders, somewhat similar to the abstraction you wrote up for the custom background demo). Critically for my other project, is that NG supports Metal and Extended dynamic range displays. I would seriously consider porting HDRView to HIM if EDR/HDR/Metal was supported, but I don't know enough about these different technologies to make the changes myself. My understanding is that GLFW supports EDR/Metal, but am not sure if SDL does.

pthom commented 6 months ago

Hello Wojciech,

Critically for my other project, is that NG supports Metal

I just added support for Metal (with SDL2 and Glfw3) in the branch refactor_backends. It seems quite stable and usable.

You now have those backends:

# Use Glfw3 + OpenGl3
option(HELLOIMGUI_USE_GLFW_OPENGL3 "Build HelloImGui for Glfw3+OpenGL3" OFF)
# Use SDL2 + OpenGL3
option(HELLOIMGUI_USE_SDL_OPENGL3 "Build HelloImGui for SDL2+OpenGL3" OFF)
# Use Glfw3 + Metal (Apple only)
option(HELLOIMGUI_USE_GLFW_METAL "Build HelloImGui for Glfw3+Metal" OFF)
# Use SDL2 + Metal (Apple only)
option(HELLOIMGUI_USE_SDL_METAL "Build HelloImGui for SDL2+Metal" OFF)
# Note: Metal and OpenGl3 are mutually exclusive!

If you want to access the internals of the Metal backend, you can include rendering_metal.h, and use GetMetalGlobals(). I'd be interested to know if you need this and if I shall publish it in a more visible way.

I would seriously consider porting HDRView to HIM if EDR/HDR/Metal was supported, but I don't know enough about these different technologies to make the changes myself.

Please, if you have time, do run a simple test with a sample dummy app (or helloimgui_demodocking) and keep me posted!

My understanding is that GLFW supports EDR/Metal, but am not sure if SDL does.

You probably know a lot more than me about this. A quick google search confirm that Glfw appears to support it


~There is one known issue with the Metal + Glfw backend, when using viewports: windows dragged out of the main window may appear black (now fixed)~

wkjarosz commented 6 months ago

Thanks. I'll give that branch a try with a dummy app when I have the chance.

wkjarosz commented 6 months ago

Nothing concrete yet, but I did a bit more digging into this and figured I would provide some comments and ask questions:

As far as I can tell, upstream GLFW and SDL don't yet support EDR/HDR.

NanoGUI has circumvented this by using a forked and modified version of GLFW.

There is an issue with patch for GLFW to enable this (which, as far as I can tell, is nearly identical to the changes in @wjakob's GLFW fork), but it has remained unapplied for a couple years.

SDL has a related issue but it remains open, though with a comment that they'd like to add this to SDL 3.

As it stands now, I don't believe linking with vanilla GLFW or SDL will allow creating floating point framebuffers for EDR/HDR support (at least with OpenGL. it's possible that the Metal backends to SDL or GLFW would support HDR framebuffers out of the box, but I don't know).

One short-term solution would be to link against @wjakob's forked GLFW. It seems the easiest approach would be to add that GLFW version as a subproject/fetch/CPM.cmake dependency to my own app, but then I'd have to convince hello_imgui to use this GLFW version instead of the system-installed one or hello_imgui's own fetched version. Is there an existing way to point hello_imgui at a custom GLFW implementation?

pthom commented 6 months ago

Ok, you have several possibilities:

  1. by setting -DHELLOIMGUI_DOWNLOAD_GLFW_IF_NEEDED=OFF
option(HELLOIMGUI_DOWNLOAD_GLFW_IF_NEEDED "Download and build GLFW if needed" ${autodownload_default})

In this case, you would have to set CMAKE_PREFIX_PATH so that find_package looks inside a folder where you installed your own version of glfw.

  1. By hacking _him_fetch_glfw_if_needed inside src/hello_imgui/CMakeLists.txt
function(_him_fetch_glfw_if_needed)
    set(shall_fetch_glfw OFF)
    if (HELLOIMGUI_DOWNLOAD_GLFW_IF_NEEDED AND NOT TARGET glfw)
        find_package(glfw3 QUIET)
        if (NOT glfw3_FOUND)
            set(shall_fetch_glfw ON)
        endif()
    endif()

    if (shall_fetch_glfw)
        message(STATUS "HelloImGui: downloading and building glfw")
        include(FetchContent)
        Set(FETCHCONTENT_QUIET FALSE)
        FetchContent_Declare(glfw
            GIT_REPOSITORY    https://github.com/glfw/glfw.git
            GIT_TAG           3.3.8
            GIT_PROGRESS TRUE
        )

        set(GLFW_BUILD_EXAMPLES OFF)
        set(GLFW_BUILD_TESTS OFF)
        set(GLFW_BUILD_DOCS OFF)
        set(GLFW_INSTALL OFF)
        FetchContent_MakeAvailable(glfw)
    endif()

endfunction()
  1. By adding glfw to your build. If glfw exist as a target, it will be used:
function(him_use_glfw_backend target)
    _him_fetch_glfw_if_needed()
    if (NOT TARGET glfw) # if glfw is not built as part of the whole build, find it
        find_package(glfw3 CONFIG REQUIRED)
    endif()
    target_link_libraries(${helloimgui_target} PUBLIC glfw)

    target_sources(${target} PRIVATE
        ${imgui_backends_dir}/imgui_impl_glfw.h
        ${imgui_backends_dir}/imgui_impl_glfw.cpp
    )
    target_compile_definitions(${helloimgui_target} PUBLIC HELLOIMGUI_USE_GLFW)
    target_compile_definitions(${helloimgui_target} PUBLIC HELLOIMGUI_USE_GLFW3)
endfunction()
wkjarosz commented 6 months ago

Thanks.

I used the third option. I've tried setting 16 bit color float buffer for glfw under opengl using the forked glfw, but it doesn't seem to enable EDR mode. My current guess is that this is only supported on macOS via metal, but haven't been able to do a full test. I do know that nanogui only supports this on macOS via Metal.

For Metal, from what I can tell, having access to the GetMetalGlobals won't be enough for a downstream application to enable it. There will need to be changes within hello_imgui. Looking at how nanogui does it, i believe PrepareGlfwForMetal would need to be updated to set at least:

gMetalGlobals.caMetalLayer.wantsExtendedDynamicRangeContent = YES;
gMetalGlobals.caMetalLayer.pixelFormat = MTLPixelFormatRGBA16Float;
gMetalGlobals.caMetalLayer.colorspace = CGColorSpaceCreateWithName(kCGColorSpaceExtendedSRGB);

if requesting EDR/HDR mode.

pthom commented 6 months ago

There will need to be changes within hello_imgui

If your modifications work, we could later add a flag for them inside RunnerParams or BackendPointers (it would be inside a #ifdef HELLOIMGUI_USE_GLFW_METAL)

pthom commented 6 months ago

Note: the branch refactor_backends was now merged to master.

It adds supports for Metal, Vulkan and DirectX11.

wkjarosz commented 6 months ago

FYI: i've been able to confirm that replacing:

gMetalGlobals.caMetalLayer.pixelFormat = MTLPixelFormatBGRA8Unorm;

with

gMetalGlobals.caMetalLayer.pixelFormat = MTLPixelFormatRGBA16Float;
gMetalGlobals.caMetalLayer.wantsExtendedDynamicRangeContent = YES;
gMetalGlobals.caMetalLayer.colorspace = CGColorSpaceCreateWithName(kCGColorSpaceExtendedSRGB);

in both rendering_metal_glfw.mm and rendering_metal_sdl.mm allows me to set the clear color to an HDR value like (1.0, 1.5, 1.5) and indeed see an unclipped cyan color brighter than the white of the standard UI elements.

I'm new to Metal, so beyond just setting HIM's clear color, I wasn't able to get a full-fledged shader working yet in Metal, but will continue trying modulo availability.

Instead of incorporating this with a compile-time #define, it would be nicer if the application can choose between LDR or HDR mode programmatically (e.g. by first checking the availability of an EDR capable display). Perhaps something along the lines of BackendOptions would be the way to go? Suggestions of how to fit this in welcome.

wkjarosz commented 6 months ago

Some updates:

I managed to get a demo work of EDR support in Metal via both SDL and GLFW.

The demo is in this repo, which uses this fork of hello_imgui.

The demo draws a color gradient from -2.0 to +2.0 for each of the RGB channels. The background is also set to an HDR color value of (1.0, 1.5, 1.5). If you have a mac, you can try it out yourself. You can change which backend you use in the CMakeLists.txt file. If you choose a Metal-based backend it will render with HDR support. With GL, the background clips to white, and the gradients clip to [0,1].

I only made a few small changes in the hello_imgui fork.

Firstly, I added the 16bit pixel form, EDR hint, and colorspace hint to PrepareSdlForMetal and PrepareGlfwForMetal. Before merging, it would be good to have this happen optionally based on some backend option the user sets programmatically. Suggestions welcome.

Secondly, I needed to move around the command buffer, encoder, and clear color logic in rendering_metal.mm. As it was set up previously, the encoder and buffer were already activated in Impl_NewFrame_3D, which makes it cumbersome for CustomBackground to set up its own render pass. I've hence moved these to Impl_RenderDrawData_To_3D where they are actually needed. Additionally, mtlRenderPassDescriptor was previously set to clear the screen even if the user sets up their own CustomBackground. This was inconsistent with the GL backend, and also overwrites anything the user may have defined in their CustomBackground callback. Instead, I initially set loadAction = MTLLoadActionLoad, and only set loadAction = MTLLoadActionClear in Impl_Frame_3D_ClearColor.

I'm very new to objective-c and metal, so its possible that even with these small changes I've introduced some errors, so a second set of eyes would be appreciated.

Please let me know if you'd like to iterate further here, or if I should create a pull request and we iterate on that.

Happy new year!

pthom commented 5 months ago

Many many thanks for this, and for these details! I found your input very interesting.

Sorry for answering a bit late. I was busy working on adding freetype support (+ colored fonts) to HelloImGui. As always, adding dependencies, esp. in a multiplatform settings is always more time-consuming than expected.

I managed to get a demo work of EDR support in Metal via both SDL and GLFW. The demo is in this repo, which uses this fork of hello_imgui.

It works nicely! The CMakeLists.txt is very clean and easy to read, and I was able to switch between the different backends without any issue. It works nicely on a Mac, with Metal + SDL and OpenGL.

The demo draws a color gradient from -2.0 to +2.0 for each of the RGB channels. The background is also set to an HDR color value of (1.0, 1.5, 1.5). If you have a mac, you can try it out yourself. You can change which backend you use in the CMakeLists.txt file. If you choose a Metal-based backend it will render with HDR support. With GL, the background clips to white, and the gradients clip to [0,1].

Work perfectly. I discovered something I did not know, thanks! You are right, this Cyan is bright

Firstly, I added the 16bit pixel form, EDR hint, and colorspace hint to PrepareSdlForMetal and PrepareGlfwForMetal. Before merging, it would be good to have this happen optionally based on some backend option the user sets programmatically. Suggestions welcome.

OK, let's brainstorm:

  1. BackendPointers could be a host for those options:

I do not like this solution, for two reasons: it is named "Pointers" (hence its intent is only to store pointers), and it makes no difference between the platform backend (Sdl, Glfw), and the rendering backend (Metal, Vulkan, OpenGL, ...).

  1. Add a new structure, RendererBackendOptions, to HelloImGui::RunnerParams:

I think this would feel much more natural. Here is a possible draft:

renderer_backend_pointer.h


struct RendererBackendOptions
{
#ifdef HELLOIMGUI_HAS_METAL
    // Metal specific options
    /*
     Add options that account for the two usages we know of:

     1. One that will use standard RGBA
    gMetalGlobals.caMetalLayer.pixelFormat = MTLPixelFormatBGRA8Unorm;

     2. One or two options that will allow the use of Extended Dynamic Range (EDR) and ExtendedSRGB
            gMetalGlobals.caMetalLayer.pixelFormat = MTLPixelFormatRGBA16Float;
            gMetalGlobals.caMetalLayer.wantsExtendedDynamicRangeContent = YES;
            gMetalGlobals.caMetalLayer.colorspace = CGColorSpaceCreateWithName(kCGColorSpaceExtendedSRGB);
     */

#endif
};

As far as how to implement those options, I'd like to discuss this with you.

The easiest solution would be to add strategies, where each strategy would set a list of options inside rendering_metal.mm:

enum class MetalRenderingStrategy
{
    StandardRGBA,
    ExtendedDynamicRangeWithFloat16AndExtendedSRGB, // long name to account for the different changes you had to make

    // possible other strategies implemented later
    ...
};

struct RendererBackendOptions
{
#ifdef HELLOIMGUI_HAS_METAL
    MetalRenderingStrategy metalRenderingStrategy = MetalRenderingStrategy::StandardRGBA;
#endif
}

But may be it would be more adequate to separate the behavior into different options. It is more versatile, but does not give any insurance that the options are consistent with each other and that any combination is valid.

enum class MetalPixelFormat
{
    RGBA8Unorm,
    RGBA16Float,
    ...
};

enum class MetalColorSpace
{
    StandardSRGB,
    ExtendedSRGB,
    ...
};

struct RendererBackendOptions
{
#ifdef HELLOIMGUI_HAS_METAL
    MetalPixelFormat metalPixelFormat = MetalPixelFormat::RGBA8Unorm;
    MetalColorSpace  metalColorSpace  = MetalColorSpace::StandardSRGB;
    bool wantsExtendedDynamicRangeContent = false;
#endif
}

What do you think?

Secondly, I needed to move around the command buffer, encoder, and clear color logic in rendering_metal.mm. As it was set up previously, the encoder and buffer were already activated in Impl_NewFrame_3D, which makes it cumbersome for CustomBackground to set up its own render pass. I've hence moved these to Impl_RenderDrawData_To_3D where they are actually needed. Additionally, mtlRenderPassDescriptor was previously set to clear the screen even if the user sets up their own CustomBackground. This was inconsistent with the GL backend, and also overwrites anything the user may have defined in their CustomBackground callback. Instead, I initially set loadAction = MTLLoadActionLoad, and only set loadAction = MTLLoadActionClear in Impl_Frame_3D_ClearColor.

Great job!!! Your commit f598d1861e8e9134d140257261a73f5689585826 "EDR/HDR support; moving command buffer+encoder later" on rendering_metal.mm only brings up improvements. It solves an issue I was not happy about: how to separate the "clear_color" step from the rendering.

I'm very new to objective-c and metal, so it's possible that even with these small changes I've introduced some errors, so a second set of eyes would be appreciated.

I did study your changes in rendering_metal.mm, with attention, and they seem perfectly valid to me!

I'm very new to ObjectiveC also. As a matter of fact, I added support for Metal by adapting code from ImGui examples, but I am in no way a specialist in this domain. I did work with Objective-C a bit although (on macOS and iOS).


I think I will iterate on your work to add similar patches for the Vulkan backend, where I have the same issue (clear_color is mixed with rendering). If you happen to know about Vulkan, I would appreciate advices!

Extract from rendering_vulkan.cpp:

        callbacks->Impl_NewFrame_3D = [callbacks]
        {
            auto & gVkGlobals = HelloImGui::GetVulkanGlobals();
            auto window = HelloImGui::GetRunnerParams()->backendPointers.glfwWindow;

            // Resize swap chain?
            if (gVkGlobals.SwapChainRebuild)
            {
                ScreenSize screenSize  = callbacks->Impl_GetFrameBufferSize();
                if (screenSize[0] > 0 && screenSize[0] > 0)
                {
                    ImGui_ImplVulkan_SetMinImageCount(gVkGlobals.MinImageCount);
                    ImGui_ImplVulkanH_CreateOrResizeWindow(gVkGlobals.Instance, gVkGlobals.PhysicalDevice,
                                                           gVkGlobals.Device, &gVkGlobals.ImGuiMainWindowData,
                                                           gVkGlobals.QueueFamily, gVkGlobals.Allocator,
                                                           screenSize[0], screenSize[1],
                                                           gVkGlobals.MinImageCount);
                    gVkGlobals.ImGuiMainWindowData.FrameIndex = 0;
                    gVkGlobals.SwapChainRebuild = false;
                }
            }

            // Start the Dear ImGui frame
            ImGui_ImplVulkan_NewFrame();
            // ImGui_ImplGlfw_NewFrame();
        };

        callbacks->Impl_Frame_3D_ClearColor  = [] (ImVec4) {};

        callbacks->Impl_RenderDrawData_To_3D = []
        {
            auto & gVkGlobals = HelloImGui::GetVulkanGlobals();
            ImGui_ImplVulkanH_Window* wd = &gVkGlobals.ImGuiMainWindowData;
            ImVec4 clear_color = HelloImGui::GetRunnerParams()->imGuiWindowParams.backgroundColor;

            ImDrawData* main_draw_data = ImGui::GetDrawData();
            const bool main_is_minimized = (main_draw_data->DisplaySize.x <= 0.0f || main_draw_data->DisplaySize.y <= 0.0f);
            wd->ClearValue.color.float32[0] = clear_color.x * clear_color.w;
            wd->ClearValue.color.float32[1] = clear_color.y * clear_color.w;
            wd->ClearValue.color.float32[2] = clear_color.z * clear_color.w;
            wd->ClearValue.color.float32[3] = clear_color.w;
            if (!main_is_minimized)
                HelloImGui::VulkanSetup::FrameRender(wd, main_draw_data);
        };
pthom commented 5 months ago

Please let me know if you'd like to iterate further here, or if I should create a pull request and we iterate on that.

A pull request would be worthwhile. But there is perhaps a better way by setting a branch where we can collaborate on this. Let me explore the possibilities and come back to you

pthom commented 5 months ago

Ok, I incorporated your changes in the edr branch.

I added a commit on top where I add scaffolding for renderer_options.h (create the file, pass it were needed, scaffold the documentation generator).

The options are voluntarily very "dumb", so that you can iterate on them:

#ifdef HELLOIMGUI_HAS_METAL
struct MetalOptions
{
    // to be completed
    bool fixmeDummyOptionName = false;  // For the moment you need to set this to true to enable your changes
};
#endif

struct RendererBackendOptions
{
#ifdef HELLOIMGUI_HAS_METAL
    MetalOptions metalOptions;
#endif
};

May be you can take it from here and do a PR onto this branch?

pthom commented 5 months ago

Interestingly, the docking demo works great in EDR mode:

image

Colored fonts are rendered correctly, and texture loading also works, via ImageFromAsset, which uses image_metal.mm internally

wkjarosz commented 5 months ago

Sorry for my delay.

I have never used vulkan before, so can't be of much help there.

I'll try to look into the MetalOptions stuff in the edr branch soon and will respond here when there is something to discuss.