vlachoudis / bCNC

GRBL CNC command sender, autoleveler and g-code editor
GNU General Public License v2.0
1.54k stars 528 forks source link

Resume Jobs. Faster Rendering. Coloring features for active spindle motions. Bug fixes. and more. #1841

Open rar8000 opened 11 months ago

rar8000 commented 11 months ago

An update with many awaited features and bug fixes.

Faster Rendering
Support for very large gcode files (millions of paths)... but drawing limited
    - Margins without drawing
    - File processing timeout separate from draw-timeout

Improved zoom behavior.
    - Zoom shortcuts zoom from the middle of the screen consistently.
    - Fit to screen improved. One click from any zoom location.
    - Error checking for scrollbars.
    - Error checking for extreme minimum and maximum zooms.

More canvas options that can be dynamically set, without restart. (some do require restart)
   Advanced coloring features with quick access to configuration
   Visualize active and inactive spindle motions (or don't)
      - Good for laser gcode
      - Visualize only where the router is on or below a surface. 
      - Dynamic margin that can filter inactive spindle motions Canvas colors can be dynamically set without restart.

Processing of file data is separated from drawing the data. Allows for:
   - Improved draw times (fast operations on canvas tags)
   - Completely hide disabled blocks from the canvas. Or show them in the disabled color. 
   - Allows for margin tracing without the performance and memory hits of drawing the entire file 
   - Trim margins to dynamically include paths where the spindle is active 
       * Active can be set to mean at or below a surface value (Z position) 
       * Active be set to mean above certain spindle value (S value) 
       * These options can be disabled and combined 
    - Resuming jobs (see below) 
    - All possible from the simple design choice of compiling motion data into immutable tuples.

Improved Editor and Selection behavior
   - The Split operation is now working to some degree.
   - Toggling expand or enabled does not affect the current selection 
   - Mouse click operation work similar common spreadsheet and presentation software. 
   - Shift click on selection will add to the current selection 
   - Ctrl click will toggle the clicked and deselect all others 
   - Shift-Ctrl click/select toggles the selection of clicked item without unselecting other items 
   - !NOTE! The Gantry shortcut of ctrl-shift click has been removed to avoid conflict! 
        * Use the existing G-click shortcut instead. 
        * Avoids accidently moving the gantry while selecting items using shift and ctrl.

New XY view with image coordinate orientation. Where the origin is at the top left.
Safe Z height setting is persistent on restart.
Resuming a job from any path while preserving Z position and Feedrate values.
    - Select the point and type "RESTART" or "RES" into the command. Type "RUN" or press run button.
    - To clear the resume point, Unselect All, then type "RESTART"/"RES" in the command. Or restart the program.
    - Setting a restart position will inject commands for feedrate and initial position; ONLY.
    - Clearing the restart position will not inject any commands.
    - If GCode does NOT have Z coordinates, then the gantry position at processing affects block position.
         * Restarting from the first block can be different than clearing restart position.
         * Restarting will restart from the last position calculated from the last processing step.
         * Clearing will send raw gcode, so gantry position at job start matters, when no Z specified.
     - Injection commands can be disabled with the command "RESTART_INJECT off". Enabled by default on restart.
     - Injection commands can be enabled with the command "RESTART_INJECT". Enabled by default on restart.
     - Custom restart commands can be injected from a button using the command "RESTART_BUTTON <button name>"
     - Clear the restart button using the command "RESTART_BUTTON". 
     - Restart button commands are not affected by the RESTART_INJECT option
     - Restart button commands are called before default injection commands when both enabled.
     !NOTE: All restart settings are ephemeral and reset when the program is closed.

Changed pathing behavior for G28, G30, G92 commands
    - Helps the visualization of 3D printed files
Fixed bug that could sometimes cause jobs to stop prematurely when queue was loading

Notable UI Changes:
    Separate buttons for canvas refresh; redrawing all canvas paths; and recompiling all gcode with redraw.
    New canvas button (gear icon) that opens advanced canvas settings.
        * A one-time Restart may be required to view new options.
    Two new buttons when advanced coloring options is enabled.
        * Trim margins to active spindle motions
        * Hide/Show inactive spindle motions
    Disabled blocks do not affect enabled blocks. Disabled blocks are processed after enabled blocks.
    Clicking on SELECT icon after it is already enabled will unselect all. Brings click access to a common operation.
    Holding shift during selection adds paths to the current selection set.
    Double clicking a disabled block will select it. Single click and area select will not select disabled blocks/paths.
    Bring canvas elements to the forefront by toggling them (margins, grid, etc). Quickly reset using refresh button.

Hi. I know there is a lot here, so I created a test branch with more test files. rar8000/bCNC->test. Lived with many of these changes for sometime now and feel comfortable about their stability and ready for code review. On my test branch there are more files in the tests/static folder, and a test plan document in the tests folder. Windows exe support is also on my test branch. Needs some work before submitting.

Cheers.

rar8000 commented 11 months ago

Some before and after examples for visualization.

Before: bCNC_org bCNC_org4

After: bCNC_new1 bCNC_new2 bCNC_new3 bCNC_new4

Harvie commented 11 months ago

Wow. This seems so cool. What exactly you did to speed up the rendering? (i haven't tested it yet) I will surely review this

rar8000 commented 11 months ago

Hi. You will need a large enough file that takes time to render for you currently. My test branch has several.

The current rendering compiles each line and draws it as it goes. I separated this into two steps. First, a processing step that compiles each line into a pathdata array that parallels the existing paths array. Then a 2nd step to draw the paths, it uses the precompiled pathdata to draw the canvas.

This allows switching views to render faster since it doesn't have to compile each line from text again. Also canvas tags can then be used to hide and show different lines and paths very quickly. You also don't have to draw anything at all to get Margins; set draw time to zero.

As a side effect, restarting jobs is very easy with the feedrate added to the pathdata.

Also checkout the changes to selection and the editor. This is no solution to Tkinter being a huge bottleneck, but I tried to utilize existing methods to speed some (but not all) things up.

Harvie commented 11 months ago

processing step that compiles each line into a pathdata array that parallels the existing paths array.

So this preprocessing has to be done every time something is changed in g-code using editor or CAM tools right?

rar8000 commented 11 months ago

processing step that compiles each line into a pathdata array that parallels the existing paths array.

So this preprocessing has to be done every time something is changed in g-code using editor or CAM tools right?

Yes. To ensure correctness. It has to be done there. However, this version doesn't block you from making more editor changes and restarting the processing over automatically.

MARIOBASZ commented 11 months ago

Hello rar8000 I have cloned from your repository, I ran into these two problems: 1- When I activate the Render Settings gear, Canvas - the sheet to modify the Canvas parameters is empty. 2- Trochoid 3d cannot generate the silhouette, or it does so with errors Greetings, and impressive work

rar8000 commented 11 months ago

Hi Mariobasz,

Thank you for testing!

You'll have to restart the program once to get the settings to show. This is an issue for many other settings and didn't want to add more complexity for this release. Also changing any canvas settings without a restart can be hit and miss. Sometimes needing a reprocessing step. The settings are ensured on restart but sometimes do not need a restart, and wanted to have that option.

For the Trochoid 3D, do you know if the problem also happens in the current official release of bCNC? When using Trochoid 3D with the official release I get an error about "cutradius = self["diam"]. Would you point me to a test file that break in this version but not the original? I would be happy to look into it. I tried not to change any data other systems would use.

MARIOBASZ commented 11 months ago

1- Restarted: now I can see the Canvas parameters, however I must read your explanation: Z threshold <= -6 however view ISO1 does not record changes. 2- Trochoid 3D seems to work, my mistake: it was loaded with the clothoid splices parameter (to reduce jerks) from my computer outside Github, which is the one I use with the machine and it did not recognize said parameter

rar8000 commented 11 months ago

The Z threshold currently works on a path level (EDITED). I'm not sure how useful the visualization is. If any points in a path are AT or BELOW the Z threshold, the path will be shown as active. Drawing of these paths is toggled by the "active spindle" button.

Below are 3 pictures of a 3D dog figurine about 45mm tall. The Z filter height is set to 25mm. The first two pictures show the greying of any paths above this threshold and the filtering of the paths from view using the "active spindle" button. The last picture shows the view with the Z filter off.

NOTE: You do have to press the big "Canvas" button for any changes to happen. There should be an existing call for a draw step (I can't eliminate currently), then it will reprocess all the data and redraw again, with changes enabled.

EDIT2: I do have the problem of having to hit the big Canvas button twice sometimes. Almost all other settings require a restart, so it's safest to restart to ensure the settings have taken. The larger fix for run-time settings changes is a much bigger can off worms.

Zfilter1 Zfilter2 Zfilter_off

TGPSKI commented 10 months ago

:+1: rendering speed is :100:

Harvie commented 8 months ago

What is canvas refresh button for? Do we now need to refresh manualy in some cases?

rar8000 commented 8 months ago

Hi @Harvie. Thank you for the question. The three buttons, "refresh", "redraw", "reprocess" (white, green, red), are as follows.

White refresh is basically instant and reorders all the layers to default settings in case of overlap. For example, if you wanted to see the grid (or margins) as the top layer. Toggling the grid will bring the grid (or margins) to the front. Hitting refresh will reset the grid to background almost instantly, even for big files.

The green redraw button will erase the canvas and redraw it from pre-compiled data. The CTRL-R shortcut should be tied to this operation. This is much like changing views; first erase the canvas and redraw. If all you want to do is re-order which layer is on top, this button is overkill. As it can take many seconds to redraw large projects.

The red reprocess button will reprocess all the data from the file. So each line has to be compiled and then redrawn. This takes the most time out of all the buttons. Used as a last resort.

Harvie commented 8 months ago

Isn't that gonna break workflow? eg. user processing/editing g-code, running some CAM commands or plugins, etc... we will have confused users asking why the operation did not changed the g-code all the time because newcomers will not know they are supposed to refresh something. i can even imagine myself crashing the machine, because i've forgotten to refresh view.

I yet have to test your fork, but this thing might be problematic.

rar8000 commented 8 months ago

A very understandable concern. Editing gcode, recoloring and other operations will automatically do the proper processing and/or redrawing. The bottom status bar was utilized to give the user feedback about the reprocessing and draw steps.

Ideally, a user should never have to use the Redraw-Green, or Reprocess-Red. And for small projects they may act identically to the user and cause confusion. But experience has shown that the Tkinter canvas library can bug out, especially with large files and/or on Windows. Only in this case are these buttons ever needed.

I did not like having to add two more buttons either. But included them so the user has the most options to get unstuck if Tkinter bugs out.

Harvie commented 8 months ago

In the long run it would make sense to use some native opengl accelerated visualisation library backend. #591 IIRC it was blocked by transition to python3, but now it might actualy be possible.

rar8000 commented 8 months ago

Absolutely, this is another reason why I separated the compile step from drawing. Currently they are intertwined. With the precompiled data in memory, it is much faster and easier to experiment using different canvas libraries. While still using Tkinter for all the buttons, to avoid a massive rewrite. However, the processing step of compiling the gcode from the text is also a big bottle neck, and perhaps could utilize the GPU, or a compiled C/C++ plugin. I was also going to try to compile with Cython to see how things work but haven't yet.

I wanted these speed ups in the mean time, for personal use. So no harm in sharing. Even if this PR isn't ever accepted.

Harvie commented 8 months ago

Absolutely, this is another reason why I separated the compile step from drawing. Currently they are intertwined.

While i was researching this topic few years ago, the result of my analysis was that it would be cool to write C/C++ python module which would work as accelerated g-code visualisation library, which would also understand individual blocks/lines in order to be able tell to bCNC which of them is currently selected (in case some button is pressed or drag'n'drop event was triggered).

Might be even more generic, does not neccesarily have to be "g-code visualiser", might as well be generic "3D geometry visualiser", but it has to be able to keep track of individual 3D objects so bCNC can update them individualy when block gets changed without updating whole scene. also it needs to be able to store some kind of metadata or provide handles for each 3D object (eg. reference to internal buffer of bCNC = knows which g-code block is represented by which 3D object). Also has to handle views (zooming/rotation/translation/selection/... using mouse) directly in native visualiser code without having to call python functions, so it can be fast and interactive.

  • The Split operation is now working to some degree.

What does this mean exactly?

Even if this PR isn't ever accepted.

Well. You probably have some neat ideas, but it's lot of work to review. It's always easier for opensource projects (especialy the non-profit ones like bCNC) to accept more smaller PRs in incremental steps rather than one combined with all the (possibly unrelated) features at once.

Honestly i am going through these during lunch break from my regular job, so if i cannot decide in 5 minutes whether i like it or not, i usualy have to postpone it for a long time. Also i try to judge if the PR (not just this one) creates unnecessary complexity that would make bCNC harder to maintain (because there are very few active maintainers). If it can break something on other platforms (eg. someone fixes bug on Windows, but breaks bCNC on Linux and Mac). I think i've even noticed that some guy fixed bCNC on his python version, broke it on other guys version which fixed it back, creating stupid loop that might take some time to be noticed. I try to asses how PR affects UX (obviously everyone has different workflows and i am personaly kinda biased towards having nice UX for CAM features). I only roughly check if the PR passes CI checks for code quality, since some features are so painfully missing that refactoring is not priority right now. But i am not very confident in merging conflicts in git, so i usualy don't try to fix PRs with conflicts unless the author fixes them (even when i might have originaly been the one causing the conflict - eg. by merging some other PR which edits same file). That is generaly speaking about PR merging that i do in bCNC, not just this one.

rar8000 commented 8 months ago

This refers to the gcode Editor tab. The Editor operation to Split a block into two blocks does not work for me on Linux, Mac or Windows; and various gcode blocks. The solution to fix this problem could potentially be done separately, but the code would not be the same depending if the merged was with or without other changes. This just complicates some bug fixes even more, and just needed something that worked.

I looked over the CI checks are basically CI failures in the current code base. And would require me changing all previous exception within the function too. There are also "Function too complex" complaints but really there no way around this or the previous version of the function also had this CI failure. The last type of CI here is on an extra white space I put there on purpose.

I did try to consider the smallest change possible for this task without introducing any new bugs, and stopped here. I did not change any workflow; only how the data is drawn. Utilizing existing tkinter features for faster renders. I'm just extending the current workflow in cosmetic way. I do not change the current data structures; they are still used in the same way.

Unfortunately you cannot make this kind optimization to the canvas without changing the Editor a little bit. There would be no way to do this in 5 minute code review increments without introducing (temporary) bugs along the way. These changes are too interconnected. I could take out the restart jobs code, but all the other changes need to be done all at once or things break in the Editor or Zoom. And if there are current bugs in the editor or zoom, may as well fix them while I'm in there.

Personally, I have to use my version now, especially when using the gcode Editor. Absolutely loved the idea behind this gcode editor. But it had a lot of bugs and would reset the selections in very time consuming, frustrating and potentially dangerous ways (ie: Ctrl-shift click vs Shift-click). Also for the file size I typically use, I also have to use my version for rendering and visualization.

For me, this was the point where the Editor and Canvas are a pleasure to work with, instead of frustrating. I would implore you to at least try it without connecting it to a machine, and just use the Editor and the Canvas on some gcode. Try hightlighting and splitting files, rearrange, etc. I'm confident you'll find it much easier to work with this version than the current version, for the same files (large and small).

Harvie commented 8 months ago

Try hightlighting and splitting files, rearrange, etc. I'm confident you'll find it much easier to work with this version than the current version, for the same files (large and small).

I don't object to that. But i just beleive that the canvas should correctly redraw anytime g-code is changed by any of the tools integrated in bCNC. without user having to manualy initiate redraw.

rar8000 commented 8 months ago

This version should not make the user need to refresh/redraw/reprocess manually. That would be pretty bad. Let me know if there is a case, as this is a bug. Things should work the same. No manual refresh is necessary on any operation. In many cases this version fixes a few edge case bugs.

For example, if there are 3 Blocks of gcode in the editor. The first two blocks have different Z values when they complete. The third block does not specify the Z value and only has X,Y in the gcode. In this case, the current version will render incorrect Z values if the 2nd block is enabled and then disabled. However the current version will correctly send all the enabled gcode, and only the render will be incorrect. A save and reload may be necessary to render like the gcode is (poorly) written.

Like the current version. Redraw and Reprocess are only needed if Tkinter bugs out. Refreshing (this version) is cosmetic and tells tkinter to instantly reset the default draw order on the various existing layers. Refresh is just a visual aid for the user, never a requirement.

Harvie commented 8 months ago

This version should not make the user need to refresh/redraw/reprocess manually.

When i tried last time, i needed to refresh manualy. Also what is the point of 3 different manual refresh buttons when user does not need to refresh manualy? It's still not clear to me what is the difference between these 3 buttons and i am bCNC developer. Therefore i have doubts that any newcomer will understand that or even find that intuitive and welcoming. I think the goal here is more like removing the refresh button altogether rather than making 3 instead of 1. TBH i don't remember why this button was even there in the first place... Perhaps workaround for something not correctly refreshing automaticaly?

(i am bit tired today, i have to check again :-)

MarkHerhold commented 3 months ago

If the refresh buttons are condensed into one button or removed completely, would this PR have a chance of being merged?

Harvie commented 3 months ago

would this PR have a chance of being merged?

I am not in essence opposed to any kind of progress. right away i do see many things in this PR that i would like to immediately merge. the fact they're all combined in single PR makes the process bit slower. it would be much easier for me to review 3 separate PRs. like one with fixes of stuff that is obviously wrong. other with tiny improvement and third with some larger improvement.

maybe i can try to start by cherry picking some of the more simple commits from your branch.

BTW i see you are calling garbage collector manualy. never thought of that. what kind of performance changes have you observed?

Harvie commented 3 months ago

Oh i've went quickly through the commits and they don't really seem to be aplicable separately by cherry picking, since they're mostly based on the changes in the first commit .

If the refresh buttons are condensed into one button or removed completely, would this PR have a chance of being merged?

That probably sounds reasonable. You can leave the 3 steps separate in code, so we can separate the buttons if it's gonna make sense in the future. But as i said... Right now, it seems just confusing.

or removed completely

I don't know. What is the point of the refresh button again? I don't rememeber having to use it lately...

rar8000 commented 3 months ago

I have some time, starting next month and can make rapid changes to try out less buttons.

Synopisis: 1) Reset layer priority when drawing (new feature). Could use a different icon. 2) Refresh everything, including reparsing all the gcode text 3) Refresh just the drawing data. Saves time by not reprocessing all the text.

rar8000 commented 3 months ago

"BTW i see you are calling garbage collector manualy. never thought of that. what kind of performance changes have you observed?"

I measured a memory difference for large files. I would push small VMs to OOM (large files) much faster without some GC calls. In particular, Tkinter data hangs around long after it was deleted unless GC is called.