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. |
| #: Must be the bool; the C side ignores any other value, and a canon that | ||
| #: sets it without a callable ``adopt_geometry`` is a TypeError rather | ||
| #: than a silent fall back to per-move callbacks. | ||
| use_gcode_renderer = True |
There was a problem hiding this comment.
Opting out gives RuntimeError: parse_file interp_error, because Translated.straight_feed calls a straight_feed_translated that no longer exists. Worth failing in make() with the flag named instead?
Also: an out-of-tree screen subclassing GLCanon to hook dwell or straight_feed now renders fine and is never called. Does that want a BREAKING: note in updating-linuxcnc.adoc, like the GL rewrite got?
There was a problem hiding this comment.
It was bad idea to opt-in opt-out with object property.
Reworked now to use class hierarchy which cannot be opt-out . Commit a023520
To use per-move classic protocol, just do not subclass from GlCanon. PrintCanon is good example that it still works.
There was a problem hiding this comment.
Also removed subclassing from Translated because there is no use for it now. Classic canon may still subclass from Translated to use it.
Commit d635bb9
There was a problem hiding this comment.
Opt-out half is gone, thanks. The updating-linuxcnc.adoc note is still open, and the rework widened what it would cover: lo, first_move, xo..wo, plane, feedrate, g5x_offset_*, g92_offset_* and g5x_index are off GLCanon now too. Nothing in tree reads them, so only out-of-tree screens would notice.
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. |
fixed with last commit. |
| py::object cls = py::reinterpret_borrow<py::object>((PyObject *)&PyType_Type)( | ||
| "RendererCanon", py::tuple(), ns); | ||
| m.attr("RendererCanon") = cls; | ||
| renderer_canon_type = (PyTypeObject *)cls.ptr(); // the module owns it |
There was a problem hiding this comment.
Only the module attribute holds this alive, so del gcode.RendererCanon leaves the pointer dangling into PyObject_TypeCheck. inc_ref() here?
There was a problem hiding this comment.
It's hard to imagine why one would want to run that. del gcode.RendererCanon , fixed anyway
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).