ocornut / imgui

Dear ImGui: Bloat-free Graphical User interface for C++ with minimal dependencies
MIT License
59.99k stars 10.19k forks source link

Anti-aliased rendering #133

Closed phicore closed 9 years ago

phicore commented 9 years ago

It would really be nice if imgui would use nanovg as a backend for drawing widgets. As it seems now the backend already receive a list of triangle to render. I suppose then the integration with nanovg would need to be done at an upper layer.

What do you think ?

I already use nanovg for an user interface but I use pure C for the imgui and it is less rich than your library but it definitively give nice result.

ocornut commented 9 years ago

Can you clarify, to what end would you want to use NanoVG ?

If it is to render the triangle lists output by ImGui. You can do already do this since the rendering of those lists is your responsibility. However there would be no point rendering the triangles via NanoVG, they won't look any better. But for the sake of integration with an existing codebase it could be nice to provide stock function that uses NanoVG or bgfx (this function would be 30 lines of code).

If what you desire are anti-aliasing shapes such as the rounded rectangles, it would need to be done at a lower level.

I intend to implement more consistently anti-aliased primitives in ImGui but the implementation for that would need to stay very performant. Naively adding NanoVG in the middle of ImDrawList would affect performance, and although possible my intuition is that reimplementing anti-aliased primitives based on techniques used by NanoVG would be better. (In fact Mikko the author of NanoVG has done so in an ImGui branch).

So again I am curious to know exactly to what end you desire to include NanoVG in the loop.

phicore commented 9 years ago

The intent is to use imgui rich and clear API to render antialiased vectorized GUI. If there is already a fork using nanovg i will look at it. Thanks a lot.

phicore commented 9 years ago

Indeed mikko's fork gives already nice results https://github.com/memononen/imgui

image

On my side I did not observe a real performance hit (in opengl)

Do you intend adding mikko's change in your repository ?

ocornut commented 9 years ago

I won't merge them as is, but something similar and based on this will be integrated eventually. For rounded rectangles we'll use textures to reduce vertex count, which will also enable us to use rounded frames as the default.

There is a performance hit. It's not major but it exists and I need to measure it more carefully before taking on further step. It's just that it's not at the top of priority list right now. Mikko sent me an e-mail with infos because he knows about those stuff well but I haven't had time to dig on any of it yet.

However I see no harm in using this branch if you want the anti-aliased lines right away (preferably kept up to date, thats a rather old base it is using at the moment).

ocornut commented 9 years ago

I have merged and moved Mikko changes into a new branch until they are sorted out https://github.com/ocornut/imgui/tree/2015-03-antialiased-primitives

adamdmoss commented 9 years ago

IMHO this is pleasant, most noticeable in apps that use graphs and lines a lot (or probably almost any app on a low-DPI display).

One downside is that it seems pretty consistently about 5% slower than master imgui. That's not a big deal considering how fast imgui is anyway :) but it's a bit surprising that it's even measurable given how fast my* GPU is.

(* or any modern)

ocornut commented 9 years ago

We need to turn all the rendering into using indexed vertices, it'll save on both front.

ocornut commented 9 years ago

I have changed the rendering to output indexed primitives. Reduced vertices by about 35%. Unfortunately this causes a fair amount of breakage in the "user provided" render loop.

https://github.com/ocornut/imgui/tree/2015-04-indexed-rendering

So I'll only merge that into trunk along with bigger changes such as the AA rendering or new design, and we'll take the opportunity to change the signature of the render function and provide more information/tools to the user so that code can be smaller.

ImGui could flatten the vertices/indices into a contiguous buffer for you. Considering just doing that to make it easier for the user. The reason I'm not doing it is that some user may want to change the vertex format and then it becomes 2 cpu passes over the vertices instead of one.. so that becomes a bit of a waste.

ocornut commented 9 years ago

Did some more fixes to the AA branch and added a quick hack to easily toggle with and without AA for measurement (but all path building logic still runs).

Some quick measurement. Not super thorough but gives some useful info. Test consist in opening all demo windows + the graph pane + the app examples panes. Measurement with the DX11 runtime (screenshot shows DX9, but gets you an idea). all That's about 10-13 windows open including child windows. They aren't doing a lot of stuff tho. The time are for the entire application loop, including polling messages, clearing, rendering, swapping.

Release Build
all 0.57 ms - trunk
all 0.58 ms - indexed-rendering branch
all 0.61 ms - AA branch
all 0.57 ms - AA branch with AA disabled by runtime flag

For reference, with only 1 imgui window opened the time with trunk code is 0.42 ms, it seems to be roughly the maximum framerate I can get on my computer with DX11 at this window size (resizing the main window smaller/bigger).

Debug Build (DX11)
all 1.80 ms - trunk    (render func only ~0.25 ms), 11900 vtx
all 1.55 ms - indexed-rendering branch (render func only ~0.25 ms),     7822 vtx, 11900 idx
all 2.23 ms - AA branch   (render func only ~0.37)    16527 vtx
all 1.85 ms - AA branch with AA disabled by runtime flag,   11500 vtx

What does that mean:

And it will get better when we can merge indexed rendering into the AA branch (it will benefit greatly from it).

I still need to sort out some issues with lines alignment, some things are off by a pixel.

ocornut commented 9 years ago

I have merged the indexed rendering branch into the anti-aliased branch, and spent some more time doing optimisations, now rather satisfied with the result.

anti aliased

ocornut commented 9 years ago

Done some further optimisations. Measuring in Debug and testing with some fairly loaded screens, the AA branch with AA enabled is now slightly faster than Master at rendering. Of course it is biased because I haven't done as much of the finer optimisations on Master.

Opening all the windows from the demo, within the AA branch:

Note that those costs don't grow with clipped objects (outside of screen, outside of scrolled view), a list with thousands of items won't be affected. It only grows up to how much can fit on the screen.

Again those worse cases numbers are better than the current Master, so nothing to worry about.

ocornut commented 9 years ago

I have committed the render API breaking changes that should now be more future proof. (It was already broken in this branch, but now it is broken in a way that makes more sense for future updates)

If you are feeling like testing the anti-aliased branch now, please do it and give me feedback! I'd prefer to give you no further information and see if any of the changes are confusing!

I would like to know if the breakage makes sense, what's your experience with updating, etc. thanks!

ocornut commented 9 years ago

While I'm breaking this I may just rename all the fields in ImDrawList to use PascalCase to be consistent with the other types. It always bothered me that they used a lower_case_with_underscore because ImDrawList is a proper class with functions and it makes. It's now or never so I'd rather do it now.

emoon commented 9 years ago

A few questions:

  1. When do you think this will go into master?
  2. Is the AA stuff all or nothing? Meaning is it still possible to use non-AA in places where one doesn't want it?
Pagghiu commented 9 years ago

I see a couple of runtime style flags (AntiAliasedLines and AntiAliasedShapes) that I assume do what their name suggests.

emoon commented 9 years ago

Ah nice :)

ocornut commented 9 years ago

It's pretty much done, I may decide to do the UpperCase renaming of all fields in ImDrawData, ImDrawList but other than that not much, need some testing on people apps mainly.

  1. So probably within 1 or 2 weeks.
  2. See above it is part of the style (missing the push/pop enums yet). I'm doing a few things like axis-aligned filled rectangles don't do through AA path already, etc. shaving a bit of time for charged screen.
emoon commented 9 years ago

Sound good! :+1:

ocornut commented 9 years ago

OK i went and done the renaming I wanted to do. This is mildly annoying to the user but it's the final touch to the breaking changes of that branch. Most programmers should be able to fix everything in 5 minutes. HOPEFULLY. I can still revert that change if a riot start.

https://github.com/ocornut/imgui/commit/d03b046ef4c2c371002666449307928e13d28732

 - 2015/07/07 (1.42) - switched rendering data to use indexed rendering. this is saving a fair amount of CPU/GPU and enables us to get anti-aliasing for a marginal cost.
                       this necessary change will break your rendering function! the fix should be very easy. sorry for that :(
                     - if you are using a vanilla copy of one of the imgui_impl_XXXX.cpp provided in the example, you just need to update your copy and you can ignore the rest.
                     - the signature of the io.RenderDrawListsFn handler has changed! 
                            ImGui_XXXX_RenderDrawLists(ImDrawList** const cmd_lists, int cmd_lists_count)
                       became:
                            ImGui_XXXX_RenderDrawLists(ImDrawData* draw_data). 
                              argument   'cmd_lists'        -> 'draw_data->CmdLists'
                              argument   'cmd_lists_count'  -> 'draw_data->CmdListsCount'
                              ImDrawList 'commands'         -> 'CmdBuffer'
                              ImDrawList 'vtx_buffer'       -> 'VtxBuffer'
                              ImDrawList  n/a               -> 'IdxBuffer' (new)
                              ImDrawCmd  'vtx_count'        -> 'ElemCount'
                              ImDrawCmd  'clip_rect'        -> 'ClipRect'
                              ImDrawCmd  'user_callback'    -> 'UserCallback'
                              ImDrawCmd  'texture_id'       -> 'TextureId'
                     - each ImDrawList now contains both a vertex buffer and an index buffer. For each command, render ElemCount/3 triangles using indices from the index buffer.
                     - if you REALLY cannot render indexed primitives, you can call the draw_data->DeIndexAllBuffers() method to de-index your buffer. This is slow and a waste of CPU/GPU. Prefer using indexed rendering!
                     - refer to code in the examples/ folder or ask on the GitHub if you are unsure of how to upgrade. please upgrade!
                     - removed the 'thickness' parameter from ImDrawList::AddLine().

With that I'm pretty much ok to merge that in master after 2-3 people have tested it (I'll integrate it in a big codebase I have access to first).

Maybe provide a monospaced AA font as well ?

I may need to re-add the thickness feature of AddLine(), or direct it to non-anti-aliased path for now. It's not hard to add in the AA path but quite a fair amount of extra code. Edit Done

unpacklo commented 9 years ago

I just tested it on my end and it was a very easy and trivial change! I am getting some warnings on my Linux machine, though:

clang++ -I../../ `pkg-config --cflags glfw3` -Wall -c -o main.o main.cpp
clang++ -I../../ `pkg-config --cflags glfw3` -Wall -c -o imgui_impl_glfw.o imgui_impl_glfw.cpp
clang++ -I../../ `pkg-config --cflags glfw3` -Wall -c -o ../../imgui.o ../../imgui.cpp
../../imgui.cpp:11149:33: warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast]
                ImGui::TreePush((void*)i);
                                ^
../../imgui.cpp:11190:37: warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast]
                if (ImGui::TreeNode((void*)i, "Child %d", i))
                                    ^
2 warnings generated.
clang++ -o imgui_example main.o imgui_impl_glfw.o ../../imgui.o -I../../ `pkg-config --cflags glfw3` -Wall `pkg-config --static --libs glfw3`
Build complete for Linux
g++ -I../../ `pkg-config --cflags glfw3` -Wall -c -o main.o main.cpp
g++ -I../../ `pkg-config --cflags glfw3` -Wall -c -o imgui_impl_glfw.o imgui_impl_glfw.cpp
g++ -I../../ `pkg-config --cflags glfw3` -Wall -c -o ../../imgui.o ../../imgui.cpp
../../imgui.cpp: In member function ‘void ImDrawList::AddPolyline(const ImVec2*, int, ImU32, bool, bool)’:
../../imgui.cpp:9211:9: warning: variable ‘start’ set but not used [-Wunused-but-set-variable]
     int start = 0, count = points_count;
         ^
../../imgui.cpp: In function ‘void ImGui::ShowTestWindow(bool*)’:
../../imgui.cpp:11149:40: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
                 ImGui::TreePush((void*)i);
                                        ^
../../imgui.cpp:11190:44: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
                 if (ImGui::TreeNode((void*)i, "Child %d", i))
                                            ^
g++ -o imgui_example main.o imgui_impl_glfw.o ../../imgui.o -I../../ `pkg-config --cflags glfw3` -Wall `pkg-config --static --libs glfw3`
Build complete for Linux
thevaber commented 9 years ago

I also tested this today - necessary changes were extremely straightforward and quick (except maybe for a function I use to draw linear gradient quads which uses internal ImDrawList stuff, but that's very clearly marked as internal, so that doesn't really count), everything looks great, didn't notice any issues at all and there seems to be zero performance impact even in debug builds - great work!

ocornut commented 9 years ago

Nice. Today I have merged in SDL and Allegro 5 sample to it was reassuring to see it all work. Allegro 5 doesn't support 16-bit indices or 32-bit colors but then it's a rather old and clunky library. Easy to convert the data.

Perhaps I should re-add the PrimVtx and PrimIdx functions for that sort of custom use?

thevaber commented 9 years ago

These could indeed be useful to avoid breaking changes in the future, maybe together with overloads (or something like "PrimVtxs" and "PrimIdxs") for arrays of ImDrawVerts and ImDrawIdxs; not sure how critical that is though (it's quite simple to update even when the internals change)

ocornut commented 9 years ago

I have re-added PrimVtx() plus lower-level helpers now. Don't want to surchage this so I won't add array versions but yeah it should be trivial.

ocornut commented 9 years ago

Now merged in and we have a more flexible font API: oversampling, horizontal sub-pixel positioning, ability to merge glyphs from multiple fonts into one font and various extra options.

I'll let it sit in master for a few days I assume there will be reactions/issues since it's quite a breaking change.

Pagghiu commented 9 years ago

In the next days I will branch my app applying this update on flexible fonts api and AA / Indexed rendering changes, reporting here any problems.

ApoorvaJ commented 9 years ago

FYI: Sean Barrett pushed stb_truetype.h v1.06 today, with significantly improved rasterization with AA. Might be pertinent here.

ocornut commented 9 years ago

Updated!

ocornut commented 9 years ago

Will release this as 1.43 tomorrow morning and finally close this topic!

Pagghiu commented 9 years ago

Updating my apps with latest master works with one exception. Stb_truetype 1.06 crashes on one assertion when adding font awesome. Manually Replacing 1.05 solves the crash.

ocornut commented 9 years ago

Do you have a repro? or - which setting are you using? I tried various combination and didn't get an assert firing.

On updating to indexed rendering - did you run into any issue or confusing thing that you think could have been easier? I need to keep in mind that many users are not as proficient in C/C++ as we may be, some are just learning programming so any confusion that anyone here has is mulitplied by 100 with a junior programmer and I'd like to smooth them out.

ocornut commented 9 years ago

Closing this!

ocornut commented 9 years ago

Re-opening this just so I can close it AGAIN.

emoon commented 9 years ago

Double close just to be safe! :)