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

Add an example to use ImGui with SDL / SDL_Renderer #3926

Closed
wants to merge 20 commits into from
Closed

Add an example to use ImGui with SDL / SDL_Renderer #3926

wants to merge 20 commits into from

Conversation

1bsyl
Copy link
Contributor

@1bsyl 1bsyl commented Mar 16, 2021

If this SDL2 pull request is merged
libsdl-org/SDL#4195
Then it will be possible to render triangles with SDL_Renderer.

This current PR is simply an example using ImGui + SDL/SDL_renderer
(could be opengl, gles2, d3d11, metal or software rendering back-end underneath)

Only tested on linux with Makefile.
Though I've updated all build files taken from the initial "example_sdl_opengl2" example.

NB:
I've never use ImGui before and this was just done in a couple of hours.
It was just done as a proof of concept and it may need to be implemented in a better way.

- send normalized texture coordinates
- some error/info log
- remove exe 'example_sdl_sdlrenderer'
@baidwwy
Copy link

baidwwy commented Apr 20, 2021

Is ImGui::EndFrame(); missing?
Sorry, I don’t speak English, this is from Google Translate

@ocornut
Copy link
Owner

ocornut commented Aug 11, 2021 via email

@ocornut
Copy link
Owner

ocornut commented Aug 24, 2021

Congrats on getting this change merged into SDL!

There have been a few small refactors here and there, so whenever you feel you are ready and assuming you still want this merged,

  • would be good to rebase
  • then diff imgui_impl_sdlrenderer.XXX with another renderer and try to apply some of the changes (mostly moved all globals into a struct carried in a void* in current imgui context) so things match more closely.
  • add comments in header files about required SDL version
  • add references in docs/BACKEND, docs/README

We'll apply a polish pass for coding style over that.

@1bsyl
Copy link
Contributor Author

1bsyl commented Aug 24, 2021

Hey @ocornut
I've updated the files with latest version !

sridenour and others added 9 commits September 12, 2021 12:32
… they want, and Dear ImGui can be drawn on top of their game.

Don't clear the SDL_Renderer or call SDL_RenderPresent() on it.
…s set via SDL by the user. Also allows us to honor ImDrawCallback_ResetRenderState draw command.
Integrate more nicely with SDL applications, support more Dear ImGui features
@ocornut
Copy link
Owner

ocornut commented Sep 15, 2021

Thanks for recent updates.
Do we have a timeframe for when this will be included in SDL?
Current release is 2.0.16 so I would assume 2.0.17 would include it, but your comments says 2.0.18.

  • I suggest adding a #ifdef version check for SDL version in the file and #error with a nice string if the version is not supported (instead of having user stumble on missing functions/symbols and be confused). Similarly the example main.cpp could use the same check for good measure.
  • SDL.h doesn't need to be included by imgui_impl_sdlrenderer.h, SDL_Renderer can be forward-declared.
  • In BACKENDS.md this should be listed under "Renderer backends". Other documentations/comments listing backend should list this (grep for e.g. "Metal" or any renderer backend name to find the other references).
  • Coding style is all over the place, I can fix those when merging.

@1bsyl
Copy link
Contributor Author

1bsyl commented Sep 15, 2021

@ocornut thanks ! here's a few modifications
last released SDL version is 2.0.16.
2.0.17 is for internal development, and 2.0.18 is (not before) https://libsdl.org/next.php

@ocornut
Copy link
Owner

ocornut commented Sep 21, 2021

Working on polishing and merging this now.
Note that the new SDL API SDL_RenderGeometryRaw doesn't support VtxOffset so ImGuiBackendFlags_RendererHasVtxOffset can't be supported. Perhaps it could be added before locking the API.

@1bsyl
Copy link
Contributor Author

1bsyl commented Sep 21, 2021

@ocornut what is this ImGuiBackendFlags_RendererHasVtxOffset / what benefit ? what API should look like to handle that ?
thanks

@ocornut
Copy link
Owner

ocornut commented Sep 21, 2021

It allow single window to use large meshes (more than 64k vertices) while keeping 16-bit indices (32-bit indices are opt-in compile time. See how other backends use the draw_cmd->IdxOffset/VtxOffset value.

Details at #2591

E.g. DX11 upload a single vtx and idx buffer and then draw with:

ctx->DrawIndexed(pcmd->ElemCount, pcmd->IdxOffset + global_idx_offset, pcmd->VtxOffset + global_vtx_offset);

DX9:

bd->pd3dDevice->DrawIndexedPrimitive(D3DPT_TRIANGLELIST, pcmd->VtxOffset + global_vtx_offset, 0, (UINT)cmd_list->VtxBuffer.Size, pcmd->IdxOffset + global_idx_offset, pcmd->ElemCount / 3);

OpenGL does:

#ifdef IMGUI_IMPL_OPENGL_MAY_HAVE_VTX_OFFSET
if (bd->GlVersion >= 320)
    glDrawElementsBaseVertex(GL_TRIANGLES, (GLsizei)pcmd->ElemCount, sizeof(ImDrawIdx) == 2 ? GL_UNSIGNED_SHORT : GL_UNSIGNED_INT, (void*)(intptr_t)(pcmd->IdxOffset * sizeof(ImDrawIdx)), (GLint)pcmd->VtxOffset);
else
#endif
    glDrawElements(GL_TRIANGLES, (GLsizei)pcmd->ElemCount, sizeof(ImDrawIdx) == 2 ? GL_UNSIGNED_SHORT : GL_UNSIGNED_INT, (void*)(intptr_t)(pcmd->IdxOffset * sizeof(ImDrawIdx)));

You can use dear imgui without but when doing large plots or very dense windows 16-bit indices without support for that VtxOffset can be limiting.

ocornut pushed a commit that referenced this pull request Sep 21, 2021
@ocornut
Copy link
Owner

ocornut commented Sep 21, 2021

Merged this with 62b17f9 (squash of your commits) + fba7561

Thank you very much !

@1bsyl
Copy link
Contributor Author

1bsyl commented Sep 21, 2021

@ocornut ok thanks !

It seems to me the SDL_RenderGeometryRaw API definition is compatible with VtxOffset,
as I understand, imgui will outputs 16 bits indices (instead of 32bits).

This can be done but using the last parameter 'size_indices' of SDL_RenderGeometryRaw.
Set this to '2' and it interprets indices as short.
( https://github.com/libsdl-org/SDL/blob/main/include/SDL_render.h#L1475 )

Though SDL backend aren't optimized for this, but this may changes without changing this API

}
idx_buffer += pcmd->ElemCount;
}
}

Choose a reason for hiding this comment

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

Should render clipping be disabled at the end of the ImGui_ImplSDLRenderer_RenderDrawData()?
i.e., SDL_RenderSetClipRect(bd->SDLRenderer, NULL);

Any additional render calls with that SDL_Renderer made by the user will only be able to be seen when the imgui windows are over the area in which the user has drawn.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is SDL_RenderSetClipRect(bd->SDLRenderer, NULL); moved in ImGui_ImplSDLRenderer_SetupRenderState();

I'd say, imgui should get all original user settings, save them, set its settings, and set user settings back ...
But I'm not sure how it's meant to be intergrated with user non-imgui code.

@ocornut
Copy link
Owner

ocornut commented Oct 6, 2021

Now pushed a fix to backup/restore the user's viewport and cliprect e443ea1

ocornut added a commit that referenced this pull request Oct 7, 2021
@ocornut
Copy link
Owner

ocornut commented Dec 1, 2021

@1bsyl Sorry for late answer, I forgot this message.

It seems to me the SDL_RenderGeometryRaw API definition is compatible with VtxOffset,
as I understand, imgui will outputs 16 bits indices (instead of 32bits).
This can be done but using the last parameter 'size_indices' of SDL_RenderGeometryRaw.
Set this to '2' and it interprets indices as short.
( https://github.com/libsdl-org/SDL/blob/main/include/SDL_render.h#L1475 )
Though SDL backend aren't optimized for this, but this may changes without changing this API

Those are two different things. SDL_Renderer supports 16 and 32 bits indices just fine, but not an offset into the vertex buffer. As a result using 16-bit indices it is impossible to have a large vertex buffer as the indices can never reach the whole buffer.
It is common to have a large vertex buffer (e.g all part of a large-ish mesh) and draw the different components, perhaps will different materials, but reading from that single buffer.

VtxOffset is a different parameter which is currently missing from the SDL_RenderGeometryRaw() API.
Check my comment #3926 (comment) for how this is used in DX/GL.

Now that SDL 2.0.18 is out I am afraid it would requires a new function name to add those.

@1bsyl
Copy link
Contributor Author

1bsyl commented Dec 1, 2021

@ocornut
Sorry I may still not understand ..

That would be using the API with taking into account the offset like this :

// ( maybe changing pointer cast )
xy += pcmd->VtxOffset;
color += pcmd->VtxOffset;
uv += pcmd->VtxOffset;

https://github.com/ocornut/imgui/blob/master/backends/imgui_impl_sdlrenderer.cpp#L173

@ocornut
Copy link
Owner

ocornut commented Dec 1, 2021

Oh my bad I think you are right. I was assuming we uploaded the whole buffer ahead of time.
So it should be possible to fix, I'll look soon.

But this API it suggest that every draw call will reupload the entire ImDrawList vtx buffer to GPU (or less when reaching VtxOffset > 0), which is going to be pretty wasteful (at least halving the upload performance since most windows have 2 draw calls).

@1bsyl
Copy link
Contributor Author

1bsyl commented Dec 2, 2021

ok!

Yes, you're correct: vertices butter is re-uploaded every time because SDL_Renderer queue internally all commands.
And it does some 'long' robustness check (make sure indices are not out-of-bounds), plus scale vertices eventually.

But this could be improved at some point in the future / or auto-detected / (adding eventually a SDL hint).
like: if it is a big indices vertices, without scale + so that the SDL_renderer flush its command, steal the pointer and render the vertices. So that it doesn't have to re-copy the buffer.
Or eventually adding one 1 function to set a global vertices array.

ocornut pushed a commit that referenced this pull request Dec 3, 2021
…) with 16-bit indices, enable 'ImGuiBackendFlags_RendererHasVtxOffset' in this backend. (#3926)
@ocornut
Copy link
Owner

ocornut commented Dec 3, 2021

Added support for VtxOffset with 5c388c3

@1bsyl
Copy link
Contributor Author

1bsyl commented Dec 3, 2021

@ocornut Great !
Maybe it would be nice to have in the demo an example of big plot (like in #2591 )
so that it can be used in the future for testing this

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.

None yet

6 participants