imgui icon indicating copy to clipboard operation
imgui copied to clipboard

Fixed SDL(3)_SetRenderScale handling

Open bog-dan-ro opened this issue 6 months ago • 3 comments

Fixed mouse handling when using SDL_SetRenderScale for HiDPI.

Fixed #6065

bog-dan-ro avatar Dec 29 '23 10:12 bog-dan-ro

Hello Bogdan! thanks for the PR. I think it’d be important to see if this strategy can make sense in multi-viewport mode. Would you be able to test it in docking branch with multi-viewport enabled, and consider if varying render scale are possible / make sense? In particular, there’s normally a continuity of valid mouse coordinate values in the gap between os windows, and i am not sure how this could be affected by render scale.

ocornut avatar Dec 29 '23 10:12 ocornut

@ocornut Hi, thanks for your quick reply.

I don't see how it will work, because usually we call SDL_SetRenderScale once ... but I'll think about it :). Anyway, I have only my laptop with me and no way to test it with multiple view with different pixel densities, but when I'll get back home (after 8th Jan), I'll give it a try, but considering that I'm call SDL_SetRenderScale once I don't see how it will work. The only way I see it working is if SDL_SetRenderScale is issued from imgui every time when the window is moved, but I still see an issue if the window is (partially) visible on multiple screens with different scale factors.

Here is the test that I was using to test my patch: main.cpp:

#include <SDL3/SDL.h>
#include <backends/imgui_impl_sdl3.h>
#include <backends/imgui_impl_sdlrenderer3.h>

int main(int argc, char *argv[]) {
    if (SDL_Init(SDL_INIT_EVERYTHING) < 0) {
        SDL_Log("SDL_Init failed (%s)", SDL_GetError());
        return 1;
    }

    SDL_Window *window = NULL;
    SDL_Renderer *renderer = NULL;
    constexpr int wflags = SDL_WINDOW_HIGH_PIXEL_DENSITY | SDL_WINDOW_RESIZABLE;
    if (SDL_CreateWindowAndRenderer(640, 480, wflags, &window, &renderer) < 0) {
        SDL_Log("SDL_CreateWindowAndRenderer failed (%s)", SDL_GetError());
        SDL_Quit();
        return 1;
    }
    SDL_SetWindowTitle(window, "X-Mas");
    const auto scale = SDL_GetWindowDisplayScale(window);
    SDL_SetRenderScale(renderer, scale, scale);

    IMGUI_CHECKVERSION();
    ImGui::CreateContext();
    ImGuiIO& io{ImGui::GetIO()};
    io.ConfigFlags |= ImGuiConfigFlags_NavEnableKeyboard;
    io.ConfigFlags |= ImGuiConfigFlags_NavEnableGamepad;
    io.ConfigFlags |= ImGuiConfigFlags_NavEnableSetMousePos;

    ImGui_ImplSDL3_InitForSDLRenderer(window, renderer);
    ImGui_ImplSDLRenderer3_Init(renderer);

    while (1) {
        int finished = 0;
        SDL_Event event;
        while (SDL_PollEvent(&event)) {
            ImGui_ImplSDL3_ProcessEvent(&event);
            if (event.type == SDL_EVENT_QUIT) {
                finished = 1;
                break;
            }
        }
        if (finished) {
            break;
        }

        ImGui_ImplSDLRenderer3_NewFrame();
        ImGui_ImplSDL3_NewFrame();
        ImGui::NewFrame();

        ImGui::Begin("X-Mas", nullptr);
        ImGui::Text("Hello there !");
        ImGui::End();

        ImGui::Render();

        SDL_SetRenderDrawColor(renderer, 80, 80, 80, SDL_ALPHA_OPAQUE);
        SDL_RenderClear(renderer);

        ImGui_ImplSDLRenderer3_RenderDrawData(ImGui::GetDrawData());

        SDL_RenderPresent(renderer);
    }
    ImGui_ImplSDLRenderer3_Shutdown();
    ImGui_ImplSDL3_Shutdown();
    ImGui::DestroyContext();
    SDL_DestroyRenderer(renderer);
    SDL_DestroyWindow(window);

    SDL_Quit();
}

CMakeLists.txt:

cmake_minimum_required(VERSION 3.5)

project(SDL3_imgui LANGUAGES CXX)

set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

include(FetchContent)

find_package(SDL3 QUIET)

if(NOT SDL3_FOUND)
    set(SDL_SHARED TRUE CACHE BOOL "Build a SDL shared library (if available)")
    set(SDL_STATIC TRUE CACHE BOOL "Build a SDL static library (if available)")
    FetchContent_Declare(
        SDL
        GIT_REPOSITORY https://github.com/libsdl-org/SDL.git
        GIT_TAG main  # Replace this with a particular git tag or git hash
        GIT_SHALLOW TRUE
        GIT_PROGRESS TRUE
    )
    message(STATUS "Using SDL3 via FetchContent")
    FetchContent_MakeAvailable(SDL)
    set_property(DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/_deps/sdl-src" PROPERTY EXCLUDE_FROM_ALL TRUE)
endif()

FetchContent_Declare(
  imgui
  GIT_REPOSITORY "https://github.com/bog-dan-ro/imgui.git"
  GIT_TAG master
)

FetchContent_GetProperties(imgui)
if (NOT imgui_POPULATED)
  FetchContent_Populate(imgui)
endif ()

add_library(imgui
  ${imgui_SOURCE_DIR}/imgui.cpp
  ${imgui_SOURCE_DIR}/imgui.h
  ${imgui_SOURCE_DIR}/imconfig.h
  ${imgui_SOURCE_DIR}/imgui_demo.cpp
  ${imgui_SOURCE_DIR}/imgui_draw.cpp
  ${imgui_SOURCE_DIR}/imgui_internal.h
  ${imgui_SOURCE_DIR}/imgui_tables.cpp
  ${imgui_SOURCE_DIR}/imgui_widgets.cpp
  ${imgui_SOURCE_DIR}/imstb_rectpack.h
  ${imgui_SOURCE_DIR}/imstb_textedit.h
  ${imgui_SOURCE_DIR}/imstb_truetype.h
  ${imgui_SOURCE_DIR}/backends/imgui_impl_opengl3.cpp
  ${imgui_SOURCE_DIR}/backends/imgui_impl_opengl3.h
  ${imgui_SOURCE_DIR}/backends/imgui_impl_sdl3.h
  ${imgui_SOURCE_DIR}/backends/imgui_impl_sdl3.cpp
  ${imgui_SOURCE_DIR}/backends/imgui_impl_sdlrenderer3.h
  ${imgui_SOURCE_DIR}/backends/imgui_impl_sdlrenderer3.cpp)

target_include_directories(imgui PUBLIC ${imgui_SOURCE_DIR})
target_link_libraries(imgui PUBLIC SDL3::SDL3)

FetchContent_MakeAvailable(imgui)

add_executable(${PROJECT_NAME} main.cpp)
target_compile_definitions(${PROJECT_NAME}
    PRIVATE
        GL_GLEXT_PROTOTYPES
)
target_link_libraries(${PROJECT_NAME}
    PRIVATE
        SDL3::SDL3
        imgui
)

include(GNUInstallDirs)
install(TARGETS ${PROJECT_NAME}
    LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
    RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
)

bog-dan-ro avatar Dec 29 '23 10:12 bog-dan-ro

I think it’d be important to see if this strategy can make sense in multi-viewport mode.

This change only applies to SDL_Renderer as far as I can tell, and SDL_Renderer doesn't support multi-viewport, I think we can only theorize.

I do wonder if Dear ImGui should even support SDL_RenderSetScale at all though. If possible it might make more sense for us to always render at 100% scale and say that if people want to scale their GUIs along with their app they need to use ImGui::ScaleAllSizes etc--Otherwise we're just ending up in a situation where a single (famously problematic) render backend can be scaled using a method that's completely different from all the others.


As an aside, we already use SDL_RenderSetScale in example_sdl2_sdlrenderer2 for DPI scaling as of https://github.com/ocornut/imgui/commit/5a3f82e2f4f30e529f765d38aaa76866653b245a.

I can't imagine that's just always been broken, so someone should double-check that this PR didn't cause problems with it.

PathogenDavid avatar Dec 30 '23 19:12 PathogenDavid