Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to export a png preview of the path #225

Closed
wants to merge 3 commits into from

Conversation

geekuillaume
Copy link
Contributor

This PR adds an option to export the canvas preview as a PNG file in addition to the path file.

Fixes #218

@jeffeb3
Copy link
Owner

jeffeb3 commented Apr 30, 2022

This is great. Looks good in my testing.

@bobnik
Copy link
Collaborator

bobnik commented May 3, 2022

This is a cool feature and it works perfectly. I think, however, that the way it's presented to the user is a bit clunky. It's not obvious that we will be outputting a second file.

Even though a PNG is not really a machine-printable output, I think that the toggle should be removed and the new format should be added to the existing drop list, e.g., "PNG preview". @geekuillaume, if you'd like help with this, let me know.

@bobnik
Copy link
Collaborator

bobnik commented May 3, 2022

One more minor nitpick: We omit semicolons at the end of each JS line as a style choice.

@geekuillaume
Copy link
Contributor Author

This is a cool feature and it works perfectly. I think, however, that the way it's presented to the user is a bit clunky. It's not obvious that we will be outputting a second file.

Even though a PNG is not really a machine-printable output, I think that the toggle should be removed and the new format should be added to the existing drop list, e.g., "PNG preview". @geekuillaume, if you'd like help with this, let me know.

The goal of this feature is to quickly get a preview of a file to easily show what the generated gcode will draw. It's already possible to export the path to SVG and use this for the preview but this requires more user actions to select the correct format and save it again.

We can change the label of the option to better explain this but I don't think adding PNG as an export format will best respond to the need for this feature.

For the code styling, I can change this but we should force this with a linter executed in github actions to make sure every PR matches the code style used here.

@geekuillaume
Copy link
Contributor Author

geekuillaume commented May 8, 2022

In fact, I just saw that we have an eslint configuration that ignore semicolons to the end of each line. Should we activate the rule to remove semicolons? Also, the configuration is broken because some packages are missing: @typescript-eslint/eslint-plugin, @typescript-eslint/parser and typescript.

@bobnik
Copy link
Collaborator

bobnik commented May 9, 2022

The goal of this feature is to quickly get a preview of a file to easily show what the generated gcode will draw. It's already possible to export the path to SVG and use this for the preview but this requires more user actions to select the correct format and save it again.

Ok. Can you help me understand how the PNG preview provides a better indication of what will be drawn than the preview window itself? I don't fully understand your use case. At one point we had the ability to deselect the current layer, which may be closer to the actual drawn pattern. We could add that back.

For the code styling, I can change this but we should force this with a linter executed in github actions to make sure every PR matches the code style used here.

Makes sense. I can add a PR to fix this and add the rule. Lint was broken when I ported it over to the Vite build.

@bobnik bobnik mentioned this pull request May 10, 2022
@jeffeb3
Copy link
Owner

jeffeb3 commented May 10, 2022

I can see both arguments here. I think the intent is to save a png image when you make your pattern, so you can preview the file on a hard disk later (compared to the preview in sandify). I would really like that feature. We just haven't done it before because we didn't have a good way to make it happen. It will be more important with a database feature to share patterns, because you will want the interface to have a preview available.

I don't see a solution that doesn't have negatives.

The png checkmark solution here downloads two files with one click, which I think breaks on chrome. I was downloading a bunch of files without changing the name. Firefox was automatically numbering them for me (sandify.gcode, sandify(1).gcode, etc.). When I added the png, the names got out of sync (sandify(1).gcode was downloaded at the same time as sandify.png). That's not a huge issue, but it is a bit surprising.

Making png a separate output type doesn't really fit the pattern either though. SVG is really for plotters, thr for sisyphus tables and gcode for marlin/grbl machines. PNG isn't for a machine, and all three types of machine could want to keep that preview with the pattern to be able to identify it later. SVG could maybe be stretched to fit that need. We can make the svg settings better, including changing it from black, if that helps.

We could zip up the pattern and the png. I have a personal bias against the zip format (and tar files aren't something most people use), but that would keep the names consistent, keep the files together, and only start one download to make chrome happy. But then you have to unzip the file.

Prusa slicer can embed a thumbnail in the gcode itself. But you can't open it in an image viewer. There is an octoprint plugin to read the thumbnail from uploaded gcode, which is pretty neat: https://plugins.octoprint.org/plugins/prusaslicerthumbnails/ . It's very limited on what can read those files though.

It has also been suggested that we should use the metadata in a png, jpeg, or svg to save sandify project information. If the entire state was saved in the image, then users could "save as..." their sandify project, which would look like an image to any image viewer, but could be imported back into sandify to restore all the sandify settings. The user would then save the project file as a PNG, and then also export the gcode/thr/svg. This is more work, and we have to decide on a save format. This would be a completely different export from the current ones we have. I have a bit of worry that people will publish their image to google photos or open it in an image editor, and it will mangle the metadata.

It seems like it should be easy, but it is hard to couple an image that can be opened on any computer with arbitrary sandify data. We have to make a compromise somewhere. But I would rather try something than do nothing. PRs don't require perfection, just improvement. Sandify isn't finished, so I am ok with moving this forward any way we agree to, and assuming we will have to revisit it in the future, or live with the compromise.

@bobnik
Copy link
Collaborator

bobnik commented May 10, 2022

@jeffeb3, I'll let @geekuillaume pitch in on this, but I didn't get the impression this was a "save once for later" action. It was a frequent action "to quickly get a preview of a file to easily show what the generated gcode will draw". To me, if it's an infrequent action, then the format (dropdown) approach makes more sense. If it's a frequent action that makes changing settings impractical, we should try to use the existing preview if possible to render the faithful representation.

Also not a fan of a zip file for the reasons you put forth.

@jeffeb3
Copy link
Owner

jeffeb3 commented May 10, 2022

we should try to use the existing preview if possible to render the faithful representation

I'm not sure what you mean here. Isn't the existing preview being used to generate the image for the PNG in this PR?

@bobnik
Copy link
Collaborator

bobnik commented May 10, 2022

we should try to use the existing preview if possible to render the faithful representation

I'm not sure what you mean here. Isn't the existing preview being used to generate the image for the PNG in this PR?

Earlier in the comments, I had asked @geekuillaume why the preview did not suffice as a PNG alternative for seeing what would be drawn.

@jeffeb3
Copy link
Owner

jeffeb3 commented May 10, 2022

The preview won't work if you already have the gcode/thr file on your hard drive and aren't actively working on it.

@bobnik
Copy link
Collaborator

bobnik commented Oct 14, 2023

This PR is no longer compatible with our code base.

@bobnik bobnik closed this Oct 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Saving images of patterns
3 participants