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

electron simulator #531

Merged
merged 57 commits into from Apr 28, 2020
Merged

electron simulator #531

merged 57 commits into from Apr 28, 2020

Conversation

lexelby
Copy link
Member

@lexelby lexelby commented Aug 21, 2019

This is a brand new Simulate extension implemented in Electron!

It builds the simulation as an SVG with each stitch represented as an individual path. That way we can trigger the visibility on individual stitches as a way of "animating" the simulation. For needle penetration points, we use svg path markers. That would work for the density stuff too.

I've also laid the groundwork for an API that will be used by the electron front-end. The /stitch-plan API call just returns the StitchPlan object and lets it get converted to JSON so that the javascript can understand it. The InkStitchJSONEncoder class makes it easy to allow any object to be converted to JSON like this: just implement a __json__ method that returns a dict representation of the object. That's going to make translating from wxPython-based frontends to JavaScript much easier.

This simulator also adds some new features we haven't had before:

  • jump to next/previous command (color change, trim, etc)
  • show commands on the timeline bar (jump, color change, trim, stop)
  • color the timeline bar based on the thread color
  • realistic rendering with animation
  • cursor (crosshairs) can be hidden and is much easier to see

@lexelby lexelby requested a review from kaalleen Aug 21, 2019
@lexelby
Copy link
Member Author

lexelby commented Aug 21, 2019

I added pan/zoom support. With the SVG specified as a percentage of the page size, we get automatic resizing of the SVG. It's so easy! :D

I did have to fork our own version of svg.panzoom.js. The way they wrote it, once you zoom pretty big, zooming in more takes forever. It's a minor change, but having to fork is not ideal. Hopefully they'll take my pull request.

@kaalleen
Copy link
Collaborator

kaalleen commented Aug 22, 2019

I am only getting a electron-vue welcome screen ;) Guess I have to wait for your next update.

Although I am very excited to start the work at this branch, I have to say, that I am terribly busy these days. So I am not sure, when I can begin digging into the code. But it sounds so promising!

@lexelby
Copy link
Member Author

lexelby commented Aug 22, 2019

Oh, sorry about that! My last push was to back up my progress but I forgot it would replace the development release for this branch.

I've had a lot of time because I'm on vacation this week. I'll go back to having almost no time pretty soon ;)

@kaalleen
Copy link
Collaborator

kaalleen commented Aug 31, 2019

Finally I was able to see what you've done here. Great work! I see where we are going with this.
I got distracted by some clones though and had no time to look at the code yet.

I should ask someone to test it on macOS. Hopefully we have some improvement?!?

@lexelby
Copy link
Member Author

lexelby commented Aug 31, 2019

Haha, attack of the clones. :) I don't think anything I've done will fix Mac os. I keep thinking maybe I'll rent a "mac in the cloud" or try to set up a VM...

@lexelby
Copy link
Member Author

lexelby commented Aug 31, 2019

Did you try out the checkboxes to show trims and such on the timeline?

@kaalleen
Copy link
Collaborator

kaalleen commented Aug 31, 2019

Sorry, I misplaced my last comment in the wrong branch ...

Let's try it again:

Builds for every platform are failing with Error: Cannot find module 'cfonts'

@kaalleen
Copy link
Collaborator

kaalleen commented Aug 31, 2019

Yes, sure I did :) It's so great to have them. Somehow I was expecting <| |> (the one step buttons) to jump to the next marker. Maybe we should implement something like that. And name the one step buttons differently?

Also I was thinking it would be nice to display the colors of the color block withing the slider (or above it). What do you think?

@lexelby
Copy link
Member Author

lexelby commented Aug 31, 2019

Oh wow, those are both great ideas!! I was sad that I couldn't get it to snap the slider to the markers if you drag it, but I couldn't figure out how. Your idea is way better anyway! I also really love the idea of coloring the bar based on the thread color.

I'll try to work on those if I get a chance but if you want to work on them first, by all means go for it. I've been really busy lately (again).

@lexelby
Copy link
Member Author

lexelby commented Aug 31, 2019

No idea what's up with the cfonts thing :(

@wwderw
Copy link
Contributor

wwderw commented Sep 1, 2019

Also I was thinking it would be nice to display the colors of the color block withing the slider (or above it). What do you think?

Brother PE Designs has something very similar to that. Right above the slider they have colors of the pattern. As the slider progresses, you see which color block it's on.

@lexelby
Copy link
Member Author

lexelby commented Sep 1, 2019

Neat! First time I've seen findIndex, neat trick.

I wonder, do we want to limit the "next command" button to just the kinds of commands they have shown? Or add buttons for next/previous color change, next/previous trim, etc?

@lexelby
Copy link
Member Author

lexelby commented Sep 1, 2019

I'm a little worried that the control panel is too wide, especially if we want to have the simulator side-by-side with the Params window like we have currently. Maybe I made the icons too big? Maybe the controls should be multiple lines? I'm really at the edge of my CSS ability here -- you're so much better at that stuff.

BTW, if you want to switch to Sass or something like that, feel free. Apparently Vue supports those easily.

@kaalleen
Copy link
Collaborator

kaalleen commented Sep 2, 2019

Nice, I like the coloring of the trail. I would have had a completely different approach using the linear-gradient css property ;)

I am really struggling with time at the moment. But I moved the css part into it's own file, just to see if you agree to it. I find it easier to work with it that way. But maybe there was a reason why you had it in there.

I am not really familiar with sass, etc. I think it's not too hard to learn though. Maybe if the task needs more complicated styling, we can consider to use it.

I was thinking about the space issue too... still I like how you made it look. Maybe we could use this view if there is enough space and have some transitions if there is less space. But we will even need more controls, like fore example a switch for the different view modes (needle penetration points and the density views from @MatsIBengtsson ). He also said he would like to split windows and have the simulator several times at once in different modes. So we should consider this when adapting the style too. I am afraid though, that I will not have much time during this week.

@kaalleen
Copy link
Collaborator

kaalleen commented Sep 2, 2019

One question about the visible color changes on default. I was just wondering. Since the colored trail exists, this is the only command which we could actually see without marks. I find it somehow distracting. What do you think?

Also I was wondering, if we could make each command type have their marks in a different height level? They are sometimes (often) lying on top of each other. (... Just gathering ideas here, so I won't forget). The only problem about this would be that we would need to set CSS classes for each command type.

@lexelby
Copy link
Member Author

lexelby commented Sep 2, 2019

The only reason I had the CSS in the .vue file is that I didn't realize it could be in another file. ;) I like it separated out. My only question is if we can still use Vue's "scoped style". It's probably fine if we don't use that though.

I had forgotten about the needle penetration point options. I'm thinking that the simulator is really important, so it's probably worth letting it take up a lot of space on the screen.

Maybe we could make it so the controls can be "minimized"? Not sure how to do that. And I'm also low on time this week. No worries :)

I'm fine with not enabling the color change markers on startup. It was just an idea I was playing with, feel free to revert it.

Making the markers be at different heights would be awesome. I'm not sure if it's possible to set classes on the markers though?

@kaalleen
Copy link
Collaborator

kaalleen commented Sep 3, 2019

Making the markers be at different heights would be awesome. I'm not sure if it's possible to set classes on the markers though?

I couldn't figure it out. So I did set the style directly. Seems to be working. Color changes and stops still have the same height level.

Minimizing controls for small space side by side view with other Ink/Stitch panels will be possible. Haven't gotten to it though.

The display modes could also include the realistic view. That would be a nice feature too.

@lexelby
Copy link
Member Author

lexelby commented Sep 4, 2019

I love the styling you did on the slider bar!! It's so pretty :D

I figured out how to do what I wanted to do all along: use font-awesome icons for the labels. It renders the "label" slot separately for each label, and it passes in the SliderMark object. We can add extra properties and then access them inside the slot. That lets us set a class so we can avoid using inline styles. Yay!

I struggled a bit for the "jump" icon, but I settled on a frog. I hope it's universal enough :)

@kaalleen
Copy link
Collaborator

kaalleen commented Sep 4, 2019

The frog is explained in the panel above, so everyone should understand it's meaning. I think it's a funny choice ;)

@lexelby
Copy link
Member Author

lexelby commented Sep 5, 2019

Sorcery! I'm terrible at flexbox, thank you for making it work!

@lexelby
Copy link
Member Author

lexelby commented Sep 5, 2019

I was having a hard time wrapping my mind around whether the chevron down meant "expand the controls below" or "get rid of the controls". What do you think about plus/minus instead?

I did some digging in the Vue documentation and found v-show and v-if, makes this kind of thing so easy! I'm really starting to like Vue.

@lexelby
Copy link
Member Author

lexelby commented Sep 5, 2019

I really love how this is turning out! It's way better than the old one. I have a touchscreen, and I love that I can pinch and zoom on the simulator, it's so natural.

One thing we still need to figure out: showing where the current stitch is. I'm hoping that we can make it much easier to see than the old wxPython one. Maybe CSS blend modes would help?

@lexelby
Copy link
Member Author

lexelby commented Sep 5, 2019

For needle penetration points, I think it could be super-easy using SVG path markers. We can apply the marker-start property in CSS and just toggle a class on the <svg> element to turn them on and off. I need to go to bed now but I just wanted to get this idea out here so that you know what I'm thinking :)

@kaalleen
Copy link
Collaborator

kaalleen commented Sep 5, 2019

Sure, let's take plus and minus. I added an other symbol beside of it, which will help the user to see which shortcut keys they could use. This might especially be helpful when the controls are minimized. We should make an info box everywhere in the GUI where things are not obvious or need an explanation. We want to avoid that users actually have to read the website to understand Ink/Stitch.

No time for other things, although your suggestions are so interesting ... Curious what I will see when I wake up next morning ;)

@lexelby
Copy link
Member Author

lexelby commented Sep 7, 2019

OMG I absolutely love that info box. So beautiful! You're right, I want those everywhere.

That reminds me, I'm hoping to use this for translations: https://github.com/Polyconseil/vue-gettext

@lexelby
Copy link
Member Author

lexelby commented Sep 7, 2019

My needle points idea worked!!! Amazingly, it doesn't seem to hurt the rendering speed too much either.

I definitely need help on the styling though. On my screen, it seems to make it much more likely that the "Show" box ends up wrapping to the next line.

@lexelby
Copy link
Member Author

lexelby commented Sep 7, 2019

I can't believe we get such an awesome effect just by adding <collapse-transition> around those <div>s! Vue is amazing.

@kaalleen
Copy link
Collaborator

kaalleen commented Sep 7, 2019

My needle points idea worked!!! Amazingly, it doesn't seem to hurt the rendering speed too much either.

Nice! I just made them a tiny bit smaller. I was also wondering if they should be black or have the color of the thread? With SVG 2 it would be easy to achieve. With SVG 1.1 though, we would have to create a marker for each color block, which wouldn't be too bad neither. I am just undecided, if it's better in black or if they should have the thread color.

Vue is amazing.

Hm, maybe I should start reading the documentation :D
I haven't done it yet... if just the day had some more hours (or less working time) ;)

That reminds me, I'm hoping to use this for translations: https://github.com/Polyconseil/vue-gettext

Good, thanks for the hint. That answers a question I wanted to ask a while ago ;)

I really love how this is turning out! It's way better than the old one. I have a touchscreen, and I love that I can pinch and zoom on the simulator, it's so natural.

That actually is something I used to do with the old simulator ... but I am missing it now. It really doesn't work on my touchscreen for the electron simulator.

Also I am a little concerned about the loading time for the new simulator too. The old one loads up about 5 times faster than this one. But you said, there could be done something about it by keeping the electron part alive?

definitely need help on the styling though. On my screen, it seems to make it much more likely that the "Show" box ends up wrapping to the next line.

That was to be expected. I think it is important for these elements to be able to move to the next line, in case the display isn't wide enough to contain all of them beside of each other. We could make everything smaller, but if we add more options to the "show" box, we would face the same issue again. That's why we created the minimize button. What do you think?

@lexelby
Copy link
Member Author

lexelby commented Sep 7, 2019

Nice! I just made them a tiny bit smaller. I was also wondering if they should be black or have the color of the thread? With SVG 2 it would be easy to achieve. With SVG 1.1 though, we would have to create a marker for each color block, which wouldn't be too bad neither. I am just undecided, if it's better in black or if they should have the thread color.

Yeah, I was sad that context-stroke didn't work. I like that the black is really visible, but look what happens if you zoom in. That really doesn't look so good. I want to at least see if matching the color looks better.

That actually is something I used to do with the old simulator ... but I am missing it now. It really doesn't work on my touchscreen for the electron simulator.

Really? :( What OS are you running?

Also I am a little concerned about the loading time for the new simulator too. The old one loads up about 5 times faster than this one. But you said, there could be done something about it by keeping the electron part alive?

I'm a little concerned about that too. I think a big part of the slowness is that we're running in development mode. Webpack has to package everything up on the fly to serve to the browser. I was trying to test with a built version last night but the build was broken.

That was to be expected. I think it is important for these elements to be able to move to the next line, in case the display isn't wide enough to contain all of them beside of each other. We could make everything smaller, but if we add more options to the "show" box, we would face the same issue again. That's why we created the minimize button. What do you think?

I think what bugged me the most was having it wrap to another line just for the "show commands" box, which looked a little silly. :) It'll probably look better when we have more controls. I've also been thinking about maybe making it so that each control box can be minimized separately but that can wait until down the road.

@lexelby
Copy link
Member Author

lexelby commented Mar 5, 2020

Now I've updated panzoom and the electron branch.

Yay, thank you!

For electron I seperated the files for the component. I've learned that vue.js actually wants us to write every code for one component into a single file, but still I can't get used to it. It's a bit annoying when the file is getting so long. Hope that is ok for you.

I know! I feel similarly. This is definitely okay with me, seems like the right way to go.

I think the "proper" way to do this would be to break up the simulator into a bunch of little components that are put together into one simulator component, but that seems to me like it's more trouble than it's worth. I like it as it is, a single unit that we can use elsewhere (like in the Params extension).

The last commit includes the realistic preview. The way the realistic preview does load up is not really perfect. It will be build and inserted when it is requested for the first time. This will make the user wait quiet a bit. Maybe we can figure out, how it could load a bit earlier? Also it seems to me, that it is going to be slow for designs with many stitches.

I kind of like that it doesn't spend the time to build the realistic preview until it's requested. It is pretty slow though. :( I wonder if maybe we don't want to try to do realistic rendering with animation? Maybe realistic should be separate from the simulator? I do kind of like seeing it animated, though.

These are just the main points that could use some improvement. If you want to prepare the next release and don't want to have the realistic preview in there, I am totally fine if you removed the last commit for now.

Nah, let's leave it. What does it hurt?

@lexelby
Copy link
Member Author

lexelby commented Mar 5, 2020

notes to self (or whoever wants to fix this):

build failures are because node 12 isn't supported by electron-vue: SimulatedGREG/electron-vue#979

fix is to use node 11 by adding a setup-node action (https://github.com/actions/setup-node)

It seems that electron-vue isn't compatible with node 12 yet:

  SimulatedGREG/electron-vue#979
@lexelby lexelby force-pushed the lexelby/electron-simulator branch from ca54c7a to 3fc69fd Compare Mar 7, 2020
@lexelby lexelby force-pushed the lexelby/electron-simulator branch from 23b8473 to 6ac7700 Compare Mar 7, 2020
@kaalleen
Copy link
Collaborator

kaalleen commented Mar 7, 2020

Maybe realistic should be separate from the simulator? I do kind of like seeing it animated, though.

That's exactly what I thought about it. It's really slow, but it's somehow fascinating.

@kaalleen
Copy link
Collaborator

kaalleen commented Mar 7, 2020

I believe we could return to the original panzoom by now?
What do you think? I believe, they've accepted your changes.

@lexelby
Copy link
Member Author

lexelby commented Mar 7, 2020

Yes, I think we could.

@lexelby
Copy link
Member Author

lexelby commented Mar 7, 2020

I took a look at your realistic rendering code to see if I could figure out why it's much slower than the print PDF realistic view... and then I remembered that I render that one to a PNG. ;) I guess we'd have to do something like that, but I'm not sure how with animation...

Anyway, I noticed that some of the numbers in your filters are different and you do the path a different way. I was curious about the thought process behind your changes. I can't remember why I did things the way I did, and frankly, I've forgotten how the filter works since then...

@lexelby
Copy link
Member Author

lexelby commented Mar 8, 2020

I tried really hard to switch back to the upstream svg.panzoom.js, but I failed. Their recent work uses the ?? operator introduced in EcmaScript 2020. We can't use that unless we upgrade to babel 7.8.0+ (I think?). Apparently babel-loader isn't compatible with babel 7+ or something? I have no idea what I'm doing but it was just one error after another. I'm not particularly good at nodejs.

@lexelby
Copy link
Member Author

lexelby commented Mar 8, 2020

I managed to get babel-loader to be happy with @babel/core 7.8.3, and everything seemingly works, except I still get the syntax error with the ?? in svg.panzoom.js. I don't get what's going wrong, but it seems like babel 7 isn't actually transpiling that module? Anyway, I still have no idea what I'm doing, so I'm gonna give up. If you have any idea how to fix this, please go for it :)

@kaalleen
Copy link
Collaborator

kaalleen commented Mar 8, 2020

Uh, I guess we ended up in dependency hell, which is very likely for javascript frameworks.
Let's stay with our copy for now.

Anyway, I noticed that some of the numbers in your filters are different and you do the path a different way.

It's quiet a while ago. I guess I wanted to avoid to create too many nodes within the path and the svg filter plugin did not allow all filters present in your version. Maybe I also wanted to avoid using to many filters at once, because filters really are a speed issue. But, it's quiet a while ago, I do not remember a lot of it. If you find a better way of doing it, I'd really like to see your version - especially if it is faster ;)

@lexelby
Copy link
Member Author

lexelby commented Mar 17, 2020

Definitely not faster. :) I remember how I made the print preview one less terrible: I had it render the SVG in an off-screen canvas and then add it to the DOM as a static image. We could do something like that for this but it'll be more complicated.

I see what you're getting at, although I think a <rect> with a radius is probably just as bad as the SVG path in lib/svg/rendering.py. I think I did the path just to do a little trick with the filter rendering or something.

I'll tinker with it a little. Your way works, but it doesn't look quite right on my monitor...

@lexelby
Copy link
Member Author

lexelby commented Mar 28, 2020

This gets us closer to how Print PDF does it. There's one big remaining problem though. Print PDF rotates the actual path data using simplepath.rotatePath(). This code uses an SVG transform to do the rotation. The thing is, if you use an SVG transformation, the rotation happens after the lighting is applied. Definitely not what we want! :)

I think the solution is to use this to rotate the path data: https://github.com/fontello/svgpath

I'll try to add that when I get a chance.

@kaalleen
Copy link
Collaborator

kaalleen commented Mar 28, 2020

Oh nice, I see where it's going. It's looking good!

@lexelby
Copy link
Member Author

lexelby commented Apr 26, 2020

Now that's more like it! What I like best is that this matches what the Print/Realistic Preview produces. That's one step closer to moving all of the UI into electron. Thank you!

It's been a long road, but I vote we mark this ready to merge. I think the only thing left before merging is to remove the old Simulate extension (!!!) and replace it with this one. No need to bother the user with the fact that this is Electron.

@lexelby lexelby marked this pull request as ready for review Apr 26, 2020
@kaalleen
Copy link
Collaborator

kaalleen commented Apr 27, 2020

That's how I feel about it too.
I've started to remove the old simulator. For now it's simply removed from the menu.
The old simulator will still be called beside of the params and the lettering module.

@lexelby
Copy link
Member Author

lexelby commented Apr 27, 2020

Yup. But now we have a Simulator vue component so in theory it should be much easier to move Params into electron :D

@kaalleen
Copy link
Collaborator

kaalleen commented Apr 27, 2020

Uh :) ... shall we save this topic for an other pr?

@lexelby
Copy link
Member Author

lexelby commented Apr 27, 2020

Oh yes! I was just dreaming of the glorious future. This is ready to merge :)

@kaalleen
Copy link
Collaborator

kaalleen commented Apr 28, 2020

Ok, then :) Let's do it!

@kaalleen kaalleen merged commit cb2b4e3 into master Apr 28, 2020
3 of 4 checks passed
@kaalleen kaalleen deleted the lexelby/electron-simulator branch Apr 28, 2020
kaalleen added a commit that referenced this issue May 16, 2020
## New Features

- New Simulator (#531)
- Import Threadlist (#666)
- Export Threadlist in ZIP file (#664)
- Option to include SVG in ZIP file (#648)
- Break Apart and Retain Holes (#653)
- G-Code: option to alternate z-value (#659)
- New Stitch Plan Extension (#640)
- Optionally enable/disable ties (lock down stitches) (#619)
- Multiple Fill Underlays
- Additional Fonts (#683):
    * Geneva
    * DejaVu

## Improvements

- Better Algorithm for Satin Columns (#607)
- Better Algorithm for Fill Stitches (#606)
- Convert to Satin with Loops (#608)
- Adding '@ #' to all fonts (#680)

## Bug Fixes

- Fix Issue with Troubleshoot Pointer Position (#696)
- Inherit Styles (#673)
- Fix Color Palette Issues (#660)
- Preserve Aspect Ratio (#646)
- and more

## Under the Hood

- Namespaced Attributes (#657)
- Remove stub.py (#629)
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.

None yet

3 participants