optimization: Build the G-code preview in C++ - #4481
Conversation
bd037e7 to
c0bf905
Compare
|
What are all those json files doing in there? It seems you want to do some "bake" test, but I did not see a Another point, you are "redecorating" the gcodemodule. Would it be an idea if we move this to pybind11 at the same time? Doing this work twice may be more work and may also be error prone. |
c0bf905 to
0b8fa5f
Compare
My fault, forgot to add test.sh - done now. JSON files are reference results for related .ngc files from another folder. No problem with headless - these tests are for C++ routines that do math only - no GUI, no graphics.
Any WIP on this or samples? Yes, I can work on pybind11 conversion, but any existing refrerences to such change in LinuxCNC will be helpful. If any |
|
I agree test files do not look good. I will rework it to easily see what each case is testing. In time being, I'd appreciate feedback on the rest of PR. |
OK, I have found halquery is using pybind now. I will try to rework gcodemodule to use it too in this branch. |
0b8fa5f to
7597f74
Compare
|
Reworked with pybind11 , it is a pleasure to work with. Resulting code is much easier to read and follow! Added some modern C++ which gcodemodule.cc deserve ) Tests reworked too. |
|
Last commit is breaking compatibility - returns tuples instead of arrays is not good idea, breaks code on Python side. I will revert this at Monday. |
It was false alarm, problem not caused by this code. Everything seems to be OK and ready for review and testing. Current live testing results: loading of 418Mb .ngc file (real work file sent by customer, with G2 arcs): 25 seconds to first render. |
🤤 |
alex-pres
changed the title
Together with PR #4491 : 12 seconds! P.S. As said before AXIS [specially AXIS] is using not optimised file editor code that now eats more memory and CPU than graphical preview. So for benchmarking I cut it off. Worth to be optimised too, may be later. |
|
Could you please fetch master, then merge or rebase and push, just to let the CI run again, and we shall get the shots of all the UIs to make sure nothing regressed. |
Adds C++ GCodeRenderer, which builds the whole preview - transform, arcs, taps, suppression, vertices, extents, lengths and event records - during gcode.parse and hands it over once, replacing the Python rendering engine. C++ handles Vec[9] -> X,Y,Z transform completely, and Python renders it into OpenGL using shaders. 2 protocols to interact with python: CallbackCanon (compat) and GCodeRenderer (new). Selected when "parse" method called based on canon properties.
2564163 to
075e6b8
Compare
Done. And squashed to one commit. |
|
And the screenshots seem to be similar to what we see elsewhere. |
| } | ||
| write_vertex(pt, line_number, sub == 0 ? KIND_NOOP : cat, nullptr); | ||
| } | ||
| data_->cur9 = p2; |
There was a problem hiding this comment.
After a failed reserve() this still claims the move completed, so the next one records no jump. Return after the first failed write_vertex instead?
There was a problem hiding this comment.
You're right that it sets cur9 either way. It's dead state though — write_vertex bumps interp_error, and append() (gcode_renderer.hh:394) and arc_feed both bail on that, so no further move reaches fill(). Nothing reads it again.
Does it worth adding bool return v and check for each vertex if it is handled anyway? If yes, I will do that.
| return int_array(l->mcodes, ACTIVE_M_CODES); | ||
|
|
||
| static void linecode_register(py::module_ &m) { | ||
| py::class_<LineCode> c(m, "linecode"); |
There was a problem hiding this comment.
gcode.linecode() was constructible before and is not now. Deliberate?
There was a problem hiding this comment.
yes, it was broken. restored and added a test
09730f1
This change belongs to the separate pr-perf-rtapi-strlcpy branch and was merged into this one by mistake. It is submitted as its own pull request, so drop it here to keep this branch limited to the C++ preview work.
Replaces use_gcode_renderer, a flag a canon with a catch-all __getattr__ could not opt out of.
The renderer transforms in C++ and forwards no offsets, so nothing read them.
set_plane, set_feed_rate, set_spindle_rate and select_plane, with the plane and feedrate they wrote to.
progress_ is a bound method, so it held the canon and its whole program until the next parse.
Drops lo/first_move/xo..wo from the canon contract; the current position reaches a preview as the caller's G53 G0 initcode.
gcode.linecode() constructs again, snapshotting the interpreter the way a delivery does rather than handing back zeros no later next_line could fill in. Outside a parse it raises. snapshot_line() is now the one place a LineCode is filled in, so the two paths cannot drift.
|
Finished with changes, hope all mentioned is fixed now |
|
You might want to look at CI the clang is reporting failures, we have it fail on warnings, gotta clean those up. |
3 participants
Follows PR##4293
Adds C++ GCodeRenderer, which builds the whole preview - transform, arcs, taps, suppression, vertices, extents, lengths and event records - during gcode.parse and hands it over once, replacing the Python rendering engine. C++ handles Vec[9] -> X,Y,Z transform completely, following all interpretator details, and Python renders it into OpenGL using shaders.
Parsing is ~2.9x faster on move-shaped programs and peaks at less memory; and specially for arcs, the last per-move Python call kept in code, go from 1.58 s to 0.075 s at 1.28M segments.
Even without speed benefit it is anyway better because resulting code is much more clean and easy to follow. Previous code was hard to read, understand and test. Right now we can test GCodeRenderer engine directly, it is independent of presentation.
Benchmark results, compare to current master
Headless,
fractal-1M.ngc(1,000,149 moves):Byte-identical geometry out, ~3.7x less time to produce it.
Results could be much better if ngc file use arcs (current master calculates arcs in Python which was bad idea, after moving to C++ it is 100x faster).