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

Fix highdpi screens on SDL examples with viewports #2306

Closed
wants to merge 1 commit into from

Conversation

rasky
Copy link

@rasky rasky commented Jan 23, 2019

(Dup of #2284, done over the docking branch now, as requested by @ocornut)

imgui_impl_opengl3.cpp and imgui_impl_opengl2.cpp currently have a bug in the docking branch when used with a HighDPI configuration. I discovered this bug while testing ImGui in an application that turns on HighDPI (not to improve rendering of imgui itself, but rather the application itself over which ImGui is drawn).

To reproduce this bug, you can use any OpenGL-based example. I personally use SDL so it's possible to trigger this by modifying example_sdl_opengl3/main.cpp to add SDL_WINDOW_ALLOWHIGHDPI when calling SDL_CreateWindow(). After having added this flag, running the example immediately produces a broken output like this:

screenshot 2019-01-15 19 18 41

The bug is in ImGui_ImplOpenGL3_RenderDrawData: the draw_data->DisplayPos variable is not scaled by the current framebuffer scale.

This PR fixes this bug for both OpenGL2 and OpenGL3 implementation. Moreover, it activates HighDPI rendering on SDL examples. This is not strictly required to fix any bug, but I think it's a good idea to keep it active because it increases the amount of code being tested by running the examples, and is thus easier to uncover hidden bugs with highdpi scaling.

@ocornut ocornut added the dpi label Jan 23, 2019
ocornut added a commit that referenced this pull request Feb 11, 2019
…ue from io.DisplayFramebufferScale).

This is to allow render functions being written without pulling any data from ImGuiIO, allowing incoming multi-viewport feature to behave on Retina display and with multiple displays. If you are not using a custom binding, please update your render function code ahead of time, and use draw_data->FramebufferScale instead of io.DisplayFramebufferScale. (#2306, #1676)
Examples: Metal, OpenGL2, OpenGL3: Fixed offsetting of clipping rectangle with ImDrawData::DisplayPos != (0,0) when the display frame-buffer scale scale is not (1,1). While this doesn't make a difference when using master branch, this is effectively fixing support for multi-viewport with Mac Retina Displays on those examples. (#2306) Also using ImDrawData::FramebufferScale instead of io.DisplayFramebufferScale.
Examples: Clarified the use the ImDrawData::DisplayPos to offset clipping rectangles.
@ocornut
Copy link
Owner

ocornut commented Feb 12, 2019

Hello @rasky,

Both of your change have been integrated through various commits in made yesterday.

For io.DisplayFramebufferScale I have researched it a little more and concluded this should be an information provided in ImDrawData. This also makes the render function independent of ImGuiIO fields. I made the clip rectangle offset/scale code explicit and not relying on ScaleClipRects(). This function being destructive made it impossible to call render multiple times (e.g. for VR) so it may be sane to just obsolete it, but I also need old/copied bindings to still work. We could have core imgui itself transform the ClipRect but I am not yet confident this would satisfy every use case. For now I'm doing the transform in the bindings that are OSX compatible.

See a79785c

For SDL_WINDOW_ALLOWHIGHDPI: I added it in the main.cpp files also in master branch, and in viewport/docking I am inheriting/copying the flag from the main viewport.

Thanks for the help!

@ocornut ocornut closed this Feb 12, 2019
@rasky
Copy link
Author

rasky commented Feb 15, 2019

Thanks I'll test them.

ocornut added a commit that referenced this pull request Feb 16, 2019
ocornut pushed a commit that referenced this pull request Oct 24, 2024
…WINDOW_HIGH_PIXEL_DENSITY from the main window. (#8098, #2306)

Amend a526ff8 (#6146)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants