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

Background image on working canvas #757

Merged
merged 2 commits into from
Aug 21, 2018

Conversation

iceblu3710
Copy link
Contributor

Working Kivy only implementation to load an image onto the maslow bed. #752

  • Background button added up on the top menu bar.
    • Open Background: Opens an image picker dialog and then loads the Pick Alignment screen
    • Re-pick Alignment: Uses the current file from the prefernces
    • Update Background: Re-loads the current file and correction points in case of screen artifacts
    • Remove Background: Removes the current file and points from the config file and clears the screen, GCode is unaffected.

Instructions:

  1. Take a picture of your sheet material on your Maslow
    nf
  2. Click the Background button then choose Open Background
  3. Select the image of your sheet material
  4. The Background PointPicker will popup
    groundcontrol_005
  5. The image ratio does not matter, all skewing is corrected by moving the Cyan circles to the corners of your sheet. The red lines should follow the edges of your sheet material.
    groundcontrol_006
  6. Press Resize Texture to get a preview of what you have selected. If you are off on a corner simpily press Reset Image and move that point closer. Once you are happy with the result press Accept
  7. GroundControl will automatically update with the prospective corrected image of your sheet material overlayed on the virtual workspace.
    groundcontrol_007
  8. You can now very easily tell if your gcode will fit on that sheet material. This way you can utilize as much material as possible.

Issue for testing.
@MaslowCommunityGardenRobot
Copy link
Collaborator

Congratulations on the pull request @iceblu3710

Now we need to decide as a community if we want to integrate these changes. Vote by giving this comment a thumbs up or a thumbs down. Votes are counted in 48 hours. Ties will not be merged.

I'm just a robot, but I love to see people contributing so I'm going vote thumbs up!

@BarbourSmith
Copy link
Member

This is looking great! I'll give it a test right now

from DataStructures.makesmithInitFuncs import MakesmithInitFuncs
from UIElements.backgroundPickDlg import BackgroundPickDlg
from kivy.core.image import Image as CoreImage
from PIL import Image as PILImage
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm running into an import error with this guy, I'm going to install it but I just wanted to make sure we need it 100%

@BarbourSmith
Copy link
Member

BarbourSmith commented Aug 20, 2018

It looks like Pillow might be the more up to date version of PIL

https://pypi.org/project/Pillow/2.2.1/

Edit: I'm having trouble getting PIL to install via pip, I think we want to use Pillow which is the same thing

@iceblu3710
Copy link
Contributor Author

iceblu3710 commented Aug 20, 2018

PIL is Pillow on my system. It seems when Pillow took over they used the same import to make it backwards comparable.

Docs says we can use this to make it load only Pillow:

import PIL as pillow
from PIL import Image as PILImage

Will check when I get home later.

@BarbourSmith
Copy link
Member

You are 100% correct @iceblu3710 with pillow pip installed this works flawlessly for me. It's a fantastic update to the software, great work!! 👍 👍 💯 🎉

I think we're almost ready. There seems to be a bug for me where if I choose the background then close and reopen the program the choose background popup is opened but not clickable in the background. Any thoughts on what could cause that? I'm available for testing and I'll see if I can figure out what is going on first thing tomorrow. Again, great contribution well executed, almost ready!

image

@iceblu3710
Copy link
Contributor Author

Good catch on the bug. It occurs when there was a file to load in the config but there were no registration points. It by default took the texture of the entire widget. how it was loaded onto the main window AS WELL as the proper raw texture is a bit of a special error inside Kivy.

I simply needed to move the config write to the same routine as when I update the registration marks. Should be golden now.

I borrowed a Win10 laptop from work but I get the OpenGL 1.1 issue on all 4 versions of Kivy I have tried so I can't test this from a windows perspective, only Linux.

@MaslowCommunityGardenRobot
Copy link
Collaborator

It looks like adding these changes right now isn't a good idea. Consider any feedback that the community has given about why not and feel free to open a new pull request with the changes

@iceblu3710
Copy link
Contributor Author

Bad robot! Apparently, it didn't like the verbiage I used.

@BarbourSmith BarbourSmith reopened this Aug 21, 2018
@BarbourSmith
Copy link
Member

BarbourSmith commented Aug 21, 2018

Hahah it's not the smartest robot. It just wait's 48 hours and then closes or merges the pull request.

I'm still seeing the bug where if I open a background it looks strange the next time I open the window. I'll dig into it on my end

image

@MaslowCommunityGardenRobot
Copy link
Collaborator

It looks like adding these changes right now isn't a good idea. Consider any feedback that the community has given about why not and feel free to open a new pull request with the changes

@iceblu3710
Copy link
Contributor Author

weird, I was able to reliably re-produce the issue and after fc7ac95 it was completely gone.

Might need to clear the background settings data out of the .ini but any successful point pick will write to both and any clear or unsuccessful should write defaults of blank to both.

@BarbourSmith
Copy link
Member

Hmmmm maybe I did something wrong in reproducing it today. I'll try deleting my .ini file and see what happens. I did try picking new points

@BarbourSmith
Copy link
Member

Yup, deleting the .ini file made the issue go away so I think we're good to go!

Again, great work @iceblu3710 this is a really stellar contribution that I think a lot of folks will be excited about 👍 👍

@BarbourSmith BarbourSmith reopened this Aug 21, 2018
@blurfl
Copy link
Collaborator

blurfl commented Aug 21, 2018

Wait! Doesn't 'deleting the .ini file made the issue go away' mean re-running the calibration or hand-editing the .ini file. Wouldn't it be better to fix the issue so that the .ini file doesn't need to be deleted?

@BarbourSmith
Copy link
Member

I think I just needed to delete the .ini file because my .ini file was generated with the earlier version of the feature. Now I can change the background multiple times or remove it and put a new on in without issue so nobody except me should ever need to delete their .ini file

@iceblu3710
Copy link
Contributor Author

fc7ac95 moved the only config updates to a routine where both are updated at the same time. This routine is called whenever a reset or failed file pick is called as well as on an accepted point pick.

I could put in a config sanity check but not sure what specific point to enter it. I didn't verify the values in the config write routine as I figured it unnecessary.

How are erroneous values in the config handled in the current code?

@BarbourSmith
Copy link
Member

We generally treat the values in the .ini file as true and try to catch erroneous values before they are entered in the config file.

I felt like I tried all the combinations to get bad values in the config file and I haven't been able to get bad values with the new version

@blurfl
Copy link
Collaborator

blurfl commented Aug 21, 2018

So if the user has never used this feature, the proper section is created in the .ini file, yes? That’s good then.

@BarbourSmith
Copy link
Member

Yes.

I think the case where we could get into trouble was if the user opened the feature with an earlier version of this PR to test and then exited out of the process mid way through. It was a small corner case that is now handled correctly

@iceblu3710
Copy link
Contributor Author

Good. combining all the config modifications in one place and only calling them twice in total, (fail or success condition) keeps it clean.

@MaslowCommunityGardenRobot
Copy link
Collaborator

Time is up and we're ready to merge this pull request. Great work!

@MaslowCommunityGardenRobot MaslowCommunityGardenRobot merged commit 486b6f0 into MaslowCNC:master Aug 21, 2018
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.

4 participants