Render thick lines with correct thickness and beveled corners

[potocpav]

Polyline rendering is currently problematic. Antialiased and non-antialiased lines are handled separately, and each has its unique set of problems.

• Antialiased polylines are rendered with lower thickness around acute angles
• Non-antialiased polylines are rendered as disconnected segments which leads to gaps and overlaps

Antialiased line artifacts:

Non-antialiased line artifacts:

These problems are discussed in issue #2183, but without arriving at a single solution.

This PR contains code for non-antialiased rendering with correct line width and bevels around points. I will implement anti-aliased rendering as soon as there is consensus regarding the desired polyline appearance.

It results in this render:

I modified imgui_demo.cpp to better showcase the algorithm (see the images above). These changes should probably be reverted before merging.

The new algorithm does polyline triangulation like this:

This requires 3 verts and 3 faces for each segment. Previously, it was 4 verts and 2 faces. I did not do any rigorous benchmarks, but the code doesn't contain anything heavy.

The main question is: how to bevel the lines? With the algorithm as I implemented it, all angles are bevelled. This changes the appearance of rectangles, checkbox ticks, etc., as right angles were previously not bevelled. It may be better to bevel only acute angles, which complicates the algorithm a bit but should not be a big problem.

Should bevelling be performed only for acute angles, most segments may in practice consist of fewer segments and vertices (namely, 2 and 2). Is it possible to emit variable number of vertices to the draw buffers?

Edit: my use-case is graph plotting and currently, the graphs are a bit ugly. I tried rendering with miters (no bevel), but sometimes the miters are just too long.

[potocpav]

This is how bevel can be rendered to preserve sharp corners on obtuse and right angles

[potocpav]

I implemented the acute-angle-only bevels. This is what the render looks like now:

This seems like a close-to-perfect solution.

[meshula]

Wow, that's an elegant solution with respect to acute angle specialization, and maybe the smallest code I've seen to solve the problem in general. Very nice!

[thedmd]

@potocpav Drawing libraries usualy provide a way to set maximum length of the miter, atfer that it is just capped. Maybe this is what you're looking for?

[potocpav]

@thedmd That's another way to do it, for sure. I think the results would be similar, and the code would be a little bit more complicated. With my algorithm, bevel vertices are calculated trivially:

b1x = p1.x + (dx1 - dy1 * miter_sign) * half_thickness;
// similar for b1y, b2x, b2y

Limiting the length of the miter may require one extra line-line intersection and a vector normalization per vertex. Miter length would be customizable, but is is something somebody actually needs?

Admittedly, I didn't research SoTA and didn't know about this solution beforehand. Now I am curious to see how they handle very acute angles where "inner miter" is longer than adjacent line segments. My algorithm is quite hacky in that case.

Maybe we can even adapt some existing code? I will probably look into it.

[thedmd]

@potocpav Yeah, miter version is more costly.

I made a polyline rasterizer some time ago:
https://github.com/liballeg/allegro5/blob/master/addons/primitives/polyline.c
Maybe you can find something useful there.

It supports bevel, round and miter joints between segments and square, round, triangle and none end caps, closed polylines too. As far as I know it covers all styles SVG can do:
https://apike.ca/prog_svg_line_cap_join.html

Question is, does ImGui need full-featured version?
Do you have AA-version in plans to? I guess AA-fringe can be added like in case of AddConvexPolyFilled, by tracing generated polygon around edges.

[ocornut]

This looks potentially good but I would appreciate if the code followed on dear imgui coding style, didn't rely on C++11 and didn't make unnecessary/unrelated modifications. Thank you.

[potocpav]

@ocornut This PR is work-in-progress. I wanted to get some feedback before finalizing the algorithm and code. I will clean it up (as you suggested) before merging.

@thedmd Thanks for the pointers & link. I plan to implement AA soon, should be quite a simple addition if I sacrifice some sub-pixel accuracy around bevels. The thickness<=1px case may require a special (simpler) algorithm for performance & maintainability.

[ocornut]

@potocpav I'll give you access to the WIP imgui_tests repo in which you can add some performance tests if you want.

[potocpav]

@ocornut Would be great, thanks

[potocpav]

I implemented AA. The code is extremely ugly, but everything seems to work.

• AA thin lines are special-cased with a primitive algorithm, analogous to the one originally used for all non-AA lines. Each segment is just a square, and there are sometimes artifacts around corners (visible in the screenshot, thin shapes nr. 3,4).
• I implemented AA thickness < 1px by decreasing line alpha. Previously, it was the same as thickness==1px.
• Caps are not AA.
• Bevel anti-aliasing is not perfect: it can be up to 1.4 px wide (as opposed to the perfect 1px).

There are probably some small-ish bugs.

[potocpav]

This is probably now more-or-less finished. I did some crude benchmarks with 1M pts and -O2, and it is very similar in performance to the old code. Only the number of generated vertices and faces seem to matter to performance, not the algorithm they are generated by. I may still merge the thin-line case with the rest of the code, because it would generate fewer vertices and produce nicer output.

@ocornut I removed non-related changes & tried to improve code style, but IDK if I followed all the ImGui conventions. Is there a document somewhere documenting code style? Should I squash commits and/or rebase on master instead of the docking branch?

[potocpav]

Here are some performance measurements.

I tested on gcc -O2 only, because I don't have access to a Windows PC. I will be able to do benchmarks on MSVC debug next week, probably.

Benchmarks consisted of repeatedly drawing a bunch of given geometry primitives using the imgui_test repo. Several notes:

• Thin AA strokes have a simple code path in the new algorithm. They don't render miters properly. That's why they are so quick.
• For the new algorithm, there is some extra work for acute angles. To benchmark this unfavourable case, I created the triangle_stroke* and long_jagged_stroke* benchmarks. These perform the worst, but old algorithm rendered them incorrectly.
• long_stroke* and rect_stroke* don't contain any acute angles
• long_* benchmarks consist of polylines of ~50k vertices. I wanted to show whether the new algorithm has an advantage there because it doesn't allocate any temporary buffers. It seems to matter but not too much.

AA	test	t_old [ms]	t_new [ms]	diff
Yes	rect_stroke	4.44	4.02	-9.45%
Yes	rect_stroke_thick	4.76	5.70	+19.74%
Yes	triangle_stroke	3.54	3.27	-7.67%
Yes	triangle_stroke_thick	3.64	6.39	+75.75%
Yes	long_stroke	5.59	4.84	-13.40%
Yes	long_stroke_thick	5.77	6.64	+14.98%
Yes	long_jagged_stroke	5.38	4.96	-7.72%
Yes	long_jagged_stroke_thick	5.33	8.65	+62.37%
No	rect_stroke_thick	3.61	4.51	+24.74%
No	triangle_stroke_thick	2.99	4.43	+48.23%
No	long_stroke_thick	4.36	5.26	+20.71%
No	long_jagged_stroke_thick	4.50	6.12	+36.17%

[ocornut]

Thanks for your work @potocpav! This is looking very good.

Don't worry about squashing, we can do it when we merge but until them there's a possibility the history can come in useful. Coding style looks good as well (there's presently no document unfortunately).

You may push your test code branch into imgui_tests/ - in its own branch - so we'll have access to them, can perform further tests and eventually the measurement can also make it into that repository.

PS:
There's ongoing work by @ShironekoBen on a different change which will likely conflict with this. We are working on using textures to make >1.0f thick line same number of polygons are ==1.0f thick lines). You don't need to worry about it at all but by the look of thing I think we may merge Ben changes earlier (because by nature the other change is a more obvious "win-win" change whereas yours has more trade-off points and will need to examine all the impact on shapes). When we do we'll probably rebase yours on the new changes.

Will also need look into adding code to use the screen capturing api in imgui_tests to quickly compare all our render paths (espcially since Ben's changes above will add a new set of variations into the mix, being able to output XX exactly aligned .png to compare will be nice).

[ShironekoBen]

@potocpav, thanks for looking into this - the results look very promising!

So yeah, as @ocornut mentioned, there's some shared ground between this and the texture-based approach I've implemented, which basically just uses a set of prebuilt textures for known widths to give (approximately) one pixel of antialiasing without needing extra geometry.

Unfortunately I don't immediately see a way that we can apply that trick here - the fact that the bevels make the lines variable-width immediately causes a problem as the texturing relies on the width being constant. So it's possible that the most practical "solution" to merging them might be to have both drawing paths and come up with some metric to choose which to use based on the desired speed/quality tradeoff... does that sound like a reasonable analysis or have I missed some clever alternative approach?

Also, one small question about the code - might it be simpler to calculate unused_vertices and unused_indices at the end (based on recording _IdxWritePtr and _VtxWritePtr prior to filling the buffer)? It strikes me that might be less prone to errors if anyone alters the code, and could potentially be (very slightly) faster as it avoids updating those in the inner loop...

[potocpav]

@ocornut I pushed my changes to imgui_tests to the branch "line-perf". There are some new perf tests, new cmdline parameters, and standard deviation is calculated for each test. After using it for a bit, I am not convinced that SD is that useful of a value. Every change is in a separate commit and can be cherry-picked.

@ShironekoBen Would it be possible to use the textures for segments, and geometry for bevels? This would add 2 triangles per acute angle, but acute angles may not be very frequent in real-world UIs. Or just don't AA bevels at all, I think the artefacts caused by that would be quite small compared to status quo. Can we abuse texture wrapping? Is wrapping mode defined by imgui?

@ShironekoBen What do the textures & geometry look like? Is there a PR with the changes?

@ShironekoBen And yes, that's definitely a better way to compute unused_indices.

[ocornut]

What do the textures & geometry look like? Is there a PR with the changes?

(moving this to wip/private repo)

PR

Attached is a patch to fix most warnings of the PR (see CI servers above) along with a few stylistic issues, if you could apply it in your branch?

0001-Fix-various-warnings-tweak-comments.zip
EDIT Minor update below:
0001-Fix-various-warnings-tweak-comments.zip

There are still some "variable may be uninitialized" warnings that I haven't decided how to best fix yet.

Our CI actions (above) have "extra warnings" actions which I now realize are misleading because they are each for a given compiler, and sometimes digging in the detailed actions you can find other warning. Will tweaks the name of those actions now.

[potocpav]

Any progress on the optimized line rendering by @ShironekoBen, which this PR depends on?

[ShironekoBen]

Any progress on the optimized line rendering by @ShironekoBen, which this PR depends on?

Sorry about the slow response! So from my perspective the texture-based line rendering is pretty much complete - I've done various tests to check that rendering behaviour matches the old render path and they all seem fine.

On the subject of how we merge them, I personally feel like initially we should probably set it up so that a given line is either all-textured or all-geometry - I'm nervous that we'll have lots of (quite literal!) edge-cases trying to mix the two, so in terms of getting things in and working it seems reasonable to get everything co-existing first and then worry about if/how we can combine them. If we initially just had a drawlist flag to choose which to render with that would let us do comparison tests and figure out a real metric or mechanism for choosing between them.

Does that sound sensible?

[ocornut]

I'm going to work on resurrecting this PR, sorry to keep you waiting.
Also realized we could use the simpler path when segment count is 1, which is a very frequent thing to happen so seems to be worth it.

[ocornut]

@potocpav

Can we abuse texture wrapping? Is wrapping mode defined by imgui?

It's not defined, would not mind defining it if worthy FYI some web stack only support Clamping not repeating.

@ShironekoBen

Also, one small question about the code - might it be simpler to calculate unused_vertices and unused_indices at the end (based on recording _IdxWritePtr and _VtxWritePtr prior to filling the buffer)? It strikes me that might be less prone to errors if anyone alters the code, and could potentially be (very slightly) faster as it avoids updating those in the inner loop...

That's done in the amend mentioned just below.

Rebased branch

Caught it with this a little today:

https://github.com/ocornut/imgui/tree/features/potocpav-newer-lines-1
Has a copy of the branch from the time Pavel worked on this. That's because the original repo is now available and although github provides the patch it's nicer to have this as reference.
I made some amend: cacef5f (simplified unreserving vertices)

https://github.com/ocornut/imgui/tree/features/potocpav-newer-lines-2
Has the squashed result (1 commit), over latest master with a way to compare/toggle that code with the latest code (which use texture-based thick line rendering, #3245). As outlined by Ben above, it's not clear how those two can cohabit. (Maybe the texture-based approach was not a good path to pursue because of the edge cases involved, I'm not sure anymore? but it's quite optimal...).

In the function you can replace

bool USE_NEW_CODE = true;

by

bool USE_NEW_CODE = ImGui::GetIO().KeyShift;

To compare both.

Bonus idea, you can use ImDrawList itself to visually debug some of it, e.g...

ImDrawList* fg_draw_list = ImGui::GetForegroundDrawList();
if (this != fg_draw_list && bevel)
   fg_draw_list->AddCircleFilled(_VtxWritePtr[bi].pos, 4, IM_COL32(0, 255, 0, 255));

Some thoughts

1. Semi obvious but just in case: I am all for the improvements provided by this PR (stating this because some other PR are widely debated!).

2. A small hole is visible in the thin-line path with some shapes: (not investigated). If you bind the algorithm selection on a key modifier you can clearly see it happening. + (EDIT) lower-right of rectangle missing a pixel.
   

3. Compared to the new texture-based path, the new number of vertex/index is quite meaningful, we are at more than double.

4. As part of an experimental branch working on adding AABB to ImDrawCmd, we were looking at replacing bool closed of AddPolyline() with flags. If we have flags we could add one to distinguish convex/concave shapes, and since most of our high-level draw primitives (circle. rounded rectangle) are convex they could benefit from a faster path.

Unfortunately at this point I am unable to make any good conclusion or decision.

Recaps of code paths

I'm going to now make a recap of the code paths we have, in the hope that writing everything down can help up see things clearer. Not sure where this is going!

"N" here stands for EITHER "points_count" or "points_count - 1", ignoring the difference for simplification...

Path	Usage	Vertices	Indices	Notes
Master, Path 1	1.0 and >1.0 && < 63.0 (int only)	2 x N	6 x N	Texture-based
Master, Path 2	1.0	3 x N	12 x N
Master, Path 3	>1.0	4 x N	18 x N
Master, Path 4	1.0 and >1.0, no AA	4 x N	6 x N
New Branch, Path 1	1.0	6 x N	12 x N	Small bug to fix fixme to optimize?
New Branch, Path 2A	>1.0	4 x N + 2 x B	18 x N + 9 x B
New Branch, Path 2B	>1.0, no AA	2 x N + 1 x B	6 x N + 3 x B

So we have 7 paths to pick from right now. All the paths in master don't yield correct-looking thickness for acute angles (including for thickness = 1.0f, even if it is less noticeable).

• I don't really mind the additional cost for acute angles, more so the fact that we are paying extra overhead on everything.
• Some form of non-AA output is only desirable for low-end back-end which can't interpolate or blend.
• The texture based path was disabled for non-integer thickness because the difference is visible for small values, but potentially we could re-enable it for large thickness.

I meant to draw some conclusion from that recap but I'm too tired for it today but leaving this is, will resume later!

[ocornut]

Some news on that:

• This has been rebased on latest master, still at https://github.com/ocornut/imgui/tree/features/potocpav-newer-lines-2
• FYI although I made a fix in Triangle is only half as thick as other shapes with the same thickness value #4053 mitigating thickness issues without the miter (TL;DR; a bug appeared in January 2019 made things worst, partially but not entirely fixed in April 2019), this PR is still very much desired.
• Since the last post, we added ImDrawFlags flags parameter to certain drawing functions including AddPolyline(), so there is a reason for having opt-in features we can do it (of course better not to, but now we can).

Main reasons blocking this:

• TODO: There are missing small/missing pixels on some thin shapes.
  They appear or disappearing depending on exact tessellation, use Size=50 in that window to get same result.

• TODO: The 1.0f path has an "obvious" missing optimization, using too much vertices/indices (I may try to fix that for the sake of it, it's not the most valuable thing to do but probably not too hard).

For reference, here are the test cases from #4053 with this PR, it's good looking

[jarikomppa]

I would vote to merge this even with the small missing pixels, as the results are way better in the problematic cases.

[ocornut]

I would vote to merge this even with the small missing pixels

They appear in very common shapes. Literally rectangles are most visible thing on the screen when borders are enabled.

Rounded

Not rounded