search

CauldronDevelopmentLLC / CAMotics

Open-Source Simulation & Computer Aided Machining - A 3-axis CNC GCode simulator
Other
622 stars 143 forks source link

Add support for syncing code to simulation #399

Closed 50J0uRNeR closed 1 year ago

50J0uRNeR commented 1 year ago

This pull request is to add a number of features related to syncing the code to the simulation view. It encompasses a number requests such as #82, #208, #239, #259, #291, #293 and #295. In order to achieve this I had to make a number of changes to the GUI layout, the most dramatic being the placing of the GL View and Tab Manager side by side so they are visible at once. I was careful however to use a Splitter so the layout has the same flexibility as the console and other elements of the existing GUI.

The layout changes are:

The following program logic was added:

Most of the code modifications are in the ToolPathView and how it assembles tool paths and in GLView and how it does object picking. The GL View object picking is a simple "Color Picking" method involving coloring each tool path with the color of their file line position, and rendering this separately to a unsampled framebuffer.

The only concerns I have is I was not able to get TPL working in my Linux build so I have not tested it. The behavior of the code is to ignore TPL code as it cannot track the line position with the simulation, but it has not been tested. Also I had to scale the QOpenGLWidget size to fit the grabFramebuffer size as they do not match. The method is simple and from what I can tell the nested widgets created in QT's design editor can cause widgets to not report the proper size. Lastly I have not tried building this in Windows so I don't know if there are potential problems with the color picking as I can imagine that different operating systems and video hardware may mangle the color values or the frambuffer?

Here are some screenshots of the various changes...

Separate each file in settings... Settings

Full layout running simulation (tool path stop at current line)... FullLayout-Simulation

Full layout during tool path picking (current tool path white with following paths dimmed)... FullLayout-PathPicking

Minimal layout with separate files in large project... MinimalLayout-SeparateFiles

Minimal layout with all files rendered in large project... MinimalLayout-AllFiles

Cheers!

jcoffland commented 1 year ago

First of all, this is fantastic work. I have just a few of things I'd like to resolve before merging.

One, can you explain the point of this?:

Made the first permanent file tab a project comment text that is saved with the project files.

I see from the code that you've added a comment field to the project JSON file but I'm not understanding the reason for it yet. Are you using it to remember some state?

Two, I'm concerned about the extra memory usage caused by adding std::string filename to GCode::Move. Some programs could have millions of moves. If the filename was long this could really increase the RAM requirement. Could we use the file index or possibly a reference to the filename rather than a copy of it?

Finally, you've done a really good job of following my coding style but there are a few minor style changes I will make after merging.

Thank you for your hard work. This is a big improvement.

jcoffland commented 1 year ago

I'm thinking now the "comment" is just what it says it is, a project comment block. This might be better off as a text input box in the settings dialog. I think it's taking up too much screen space.

I did a quick calculation on the extra memory usage caused by adding std::string filename to GCode::Move. The Aztec Calendar example is 223,857 lines long. On my system the path name is /home/jcoffland/projects/camotics/git/master/examples/aztec_calendar/aztec_calendar.nc. So the extra memory used is about 100MiB. Not huge given today's memory capacities but not small either. A file with 2M lines could require an extra 1GiB of memory.

Let's move ahead with merging what you have first but here are some wish list items:

  1. Picking a line jumps to the same GCode file and line number and highlights that line.
  2. Double clicking on a GCode line picks the line in the 3D viewer.
  3. Get the generated GCode from the TPL with references back to the TPL line number. Then show the generated GCode and/or TPL line with your picking code.

I'm guessing you've had trouble with libv8.

With the Aztec Calendar picking in the 3D window often misses by quite a large margin. This may need some more work.

50J0uRNeR commented 1 year ago

First of all, this is fantastic work. I have just a few of things I'd like to resolve before merging.

Thank you jcoffland!

I'm thinking now the "comment" is just what it says it is, a project comment block. This might be better off as a text input box in the settings dialog. I think it's taking up too much screen space.

The idea of adding a project comment to tab 0 was actually because I was having difficulty figuring out how to safely alter the code that makes sure tab 0 cannot be destroyed (because it contained the glview). Now that I'm more familiar with the code base, I can try again to remove that code so the tab manager can have no tabs. However I'm not sure it makes sense to hide project comments in program settings as comments are relevant to each project and usually should be easy to find. Maybe I could put it in the left hand panel under "Workpiece" instead? Or maybe it would be better to just remove project comments completely?

I did a quick calculation on the extra memory usage caused by adding std::string filename to GCode::Move. The Aztec Calendar example is 223,857 lines long. On my system the path name is /home/jcoffland/projects/camotics/git/master/examples/aztec_calendar/aztec_calendar.nc. So the extra memory used is about 100MiB. Not huge given today's memory capacities but not small either. A file with 2M lines could require an extra 1GiB of memory.

I see two options for the filename memory problem. The easiest solution is to remove the path from the filename to store much smaller strings in each move. The more difficult solution is to rewrite everything to use the file tab index in the tab manager instead of the filename, which should only require one unsigned integer per move. This will take more time to figure out but may be a better solution? Which would you prefer?

As for the wish lists:

  1. Picking a line jumps to the same GCode file and line number and highlights that line.
  2. Double clicking on a GCode line picks the line in the 3D viewer.

The "Sync Text Editor to Simulation" toggle, at the bottom of the text editor, is disabled by default. It must be toggled on so that line selections track with the simulation. Maybe you didn't turn this on when testing? Maybe I should change this to be on by default? The new setting "Simulate each file separately" is also not on by default, maybe it should be?

  1. Get the generated GCode from the TPL with references back to the TPL line number. Then show the generated GCode and/or TPL line with your picking code.

I agree following TPL code would be good, or maybe at least load the generated gcode in a tab and follow that? I don't have TPL working so it may be a while before I can work on that.

I'm guessing you've had trouble with libv8.

Yes, that was the problem. I got cbang to build with V8 but camotics throws Exception: Chakra or V8 support is required, please rebuild C! regardless of where I set V8_HOME. I believe it is because I am running a old version of Ubuntu 18.04 and that may be causing a library problem. I intend to update to 22.04 in the near future, but I need to finish several important projects before I'm comfortable doing that.

With the Aztec Calendar picking in the 3D window often misses by quite a large margin. This may need some more work.

I also noticed that picking gets erratic in complex situations. I will continue working on this and see if I can figure out why it's behaving this way.

jcoffland commented 1 year ago

Or maybe it would be better to just remove project comments completely?

I think this is the best solution. You can already add comments in both GCode and TPL. Sometimes less is more.

The easiest solution is to remove the path from the filename to store much smaller strings in each move.

I don't like this option because two files could have the same name but be in different directories. You could use the path relative to the project file but still it's wasting memory.

The more difficult solution is to rewrite everything to use the file tab index in the tab manager instead of the filename, which should only require one unsigned integer per move.

One issue here is that the moves then depend on the order of the files in the project. Although, I don't think they can change with out needing to regenerate the moves anyway.

Another option would be to store the cb::SmartPointer<File> which is just a pointer to the File stored in the Project. This uses a fixed amount of memory.

The "Sync Text Editor to Simulation" toggle, at the bottom of the text editor, is disabled by default.

You are right. I didn't have that checked. Should this even be optional?

Also, I'm confused about what "Simulate files separately" does. If you have multiple files, which file get's simulated? What does this mean for the user?

I got cbang to build with V8 but camotics throws Exception: Chakra or V8 support is required, please rebuild C!

Are you sure cbang actually built with v8? Run this:

grep V8 $CBANG_HOME/include/cbang/config.h

It should return #define HAVE_V8.

I also noticed that picking gets erratic in complex situations. I will continue working on this and see if I can figure out why it's behaving this way.

The flower_mold.nc example demonstrates the problem very well and is much less complex than the Aztec calendar.

Converting the picking color from an integer to a float and back is lossy. The picking array buffer should be a 32-bit unsigned integer instead of a float. When you call gl.glVertexAttribPointer() you don't have to use GL_FLOAT. Also, do you need to turn off lighting when picking?

Finally, it would be a nice feature and possibly helpful in debugging this issue to automatically pick and highlight a path line continuously with just a mouse hover. Then you could see in real-time what line would be selected before clicking.

50J0uRNeR commented 1 year ago

I think this is the best solution. You can already add comments in both GCode and TPL. Sometimes less is more.

Got it, I'll work on removing the comment tab and making the tab manager work without a forced tab 0.

Another option would be to store the cb::SmartPointer which is just a pointer to the File stored in the Project. This uses a fixed amount of memory.

This sounds like a fantastic option, I'll see if I can get that working.

You are right. I didn't have that checked. Should this even be optional?

I believe this should still be an option but I will change it to be enabled by default. I found it frustrating while editing code if the text cursor jumped if I accidentally clicked in the gl view. It is also nice to be able to disable syncing so I can watch the simulation play without losing my place in the code.

Also, I'm confused about what "Simulate files separately" does. If you have multiple files, which file get's simulated? What does this mean for the user?

Each file is simulated in order as usual, however It clears the tool paths and simulation between each file so the files do not show on top of each other. I produce gcode programs that are each a sheet of nested parts, with each project containing many sheets. I need to be able to load all the sheets of a project and view and simulate each sheet individually. The last two screen shots, of my initial post, is one of these projects with this feature in action.

grep V8 $CBANG_HOME/include/cbang/config.h

Thanks for this info. I assumed cbang found V8 because during the build it shows that it found V8 config, however what you showed me says it did not build with it. I just found a ppa that allowed me to install a backport of libnode-dev and I was able to get CAMotics to build with TPL support!

Converting the picking color from an integer to a float and back is lossy. The picking array buffer should be a 32-bit unsigned integer instead of a float. When you call gl.glVertexAttribPointer() you don't have to use GL_FLOAT.

I was under the impression that would not work, but I will look it more closely. It is definitely preferable to converting the type multiple times.

Also, do you need to turn off lighting when picking?

It is necessary to do a number of things. Clear buffer, Color(0, 0, 0, 0) in background, no sampling or filtering, no lighting and lines are rendered as solid color by shader (no shading). This is all so when I collect color pixels they are not potentially altered. Do you see a problem with disabling lighting during the picking render pass?

Finally, it would be a nice feature and possibly helpful in debugging this issue to automatically pick and highlight a path line continuously with just a mouse hover. Then you could see in real-time what line would be selected before clicking.

I'll look into how QT handles the mouse hover event. It might be nice for it to lightly highlight the line under the mouse before clicking. It also provides a hint to the user that the gl view is interactive. The down side of this is it means the view will probably re-render much more often.

I'm very busy at the moment so it will be a while before I can get to all this. I'll update as soon as I can, thanks.

jcoffland commented 1 year ago

I'm not totally convinced that lighting is turned off during picking but maybe I'm missing something. In GLComposit.cpp:

    program.set("doPicking", gl.getPicking());
    program.set("light.enabled", o.getLight());

light.enabled depends only on the GLObject's state. Then in phong.frag:

void main() {
  if (light.enabled) {
    gl_FragColor = phong_color();
  } else {
    if (doPicking) {
      gl_FragColor = fPicking;
    } else {
      gl_FragColor = fColor;
    }
  }
}

I think it would be better written as:

void main() {
  if (doPicking) gl_FragColor = fPicking;
  else if (light.enabled) gl_FragColor = phong_color();
  else gl_FragColor = fColor;
}
jcoffland commented 1 year ago

I tested this change and it really improves the picking accuracy.

jcoffland commented 1 year ago

Ignore my earlier statement about using an unsigned int instead of a float for the picking array. Since gl_FragColor is a vec4 you have to convert to floats. Also, you're mapping an 8-bit integer on to a 32-bit float so there should be no precision loss.

However, if there were ever more than 16M (2^24) path segments there would be a problem. You could probably map a 10-bit integer to each color channel to support up to 1B (2^30) path segments. A possible improvement for later.

jcoffland commented 1 year ago

I've merged your changes in to the master branch and made some of my own. Here's what I did:

Note, because of the way the picking is done it starts with the first line in the top left corner of the picking window. It would be better if it picked the closest line to the pick point first.

A further enhancement would be to add a 2nd picking phase where you draw only the picked line but with a color gradient. Then you get the picked color again and use the gradient color to find the position on the picked line. This would be useful for long lines.

Thanks so much for getting this going. It's a big improvement!

50J0uRNeR commented 1 year ago

Awesome, thanks jcoffland!

I have a few preliminary observations...

For my build the cursor picking coordinates are misaligned with the framebuffer. I could not find a clear answer on the internet but on some operating systems the reported QT widget width and height is not the same size as the actual widget framebuffer. I was using the size of the framebuffer (which is always pixel perfect) to adjust the mouse position. I was assuming that if the widget size is correct the ratio will simply be 1.0 however if it is bigger or smaller it self adjusts.

As proof here is an image of the framebuffer and the white dot is where picking selected, however I double clicked in the corner... framebuffer

When adding this code back to GLView.cpp I can get the mouse and picking coordinates to line back up:

// TODO Adjust mouse position with ratio of buffer dims by widget dims?
xPicking *= ((float)image.width() / (float)width());
yPicking *= ((float)image.height() / (float)height());

framebuffer

Next, on my build when double clicking the glview it seems to select the previous toolpath, not the one under the cursor? However in the text editor it selected the proper line. If I then double click that same line in the text editor, then the toolpath in the glview jumps to where it should've selected with mouse cursor. Hopefully that makes sense and you can recreate it?

Here is a screenshot of the mouse cursor and the line that double clicking selected... picking_last_line

And here is a screenshot of double clicking the text line that was selected and proper path being highlighted after... double_clicking_text

Another picking related issue seems to be that the same text line number is selected across multiple files when double clicking in the text editor or glview. I suspect that ToolPathView is not recognizing changes between files? Here is a screenshot of line 18 being selected across multiple files... mutliple_selctions

Also it appears that the "Program Name" in the Machine Status dock is blank? Did you forget to remove this from the dock?

jcoffland commented 1 year ago

I believe I've fixed the above mentioned problems in my latest commit. Thanks again!