inkstitch / pyembroidery

libembroidery/EmbroideryFormats converted to python
MIT License
72 stars 28 forks source link

potential extra jump after trim #34

Closed lexelby closed 6 years ago

lexelby commented 6 years ago

Let's say I'm using low-level commands to create my design. I do this:

  1. STITCH
  2. STITCH
  3. TRIM
  4. JUMP
  5. STITCH
  6. ...

In other words, I'm aware that the next thing after a trim should be a jump, and I put one in myself.

EmbEncoder will still have state_trimmed set at step 5, so it will insert an extra JUMP.

Not the end of the world, but it'd be nice not to have an unnecessary JUMP inserted there. Maybe clear state_trimmed if you encounter a JUMP command?

tatarize commented 6 years ago

If you are using low-level commands and the encoder changes anything that was not invalid, it's a bug.

The bug is likely because it knows it's going to stitch and is state-trimmed it tosses in jump(s) to the next location. The problem is it goes 100% to the new location. It doesn't need to do that, if the stitch is within range it actually requires zero jumps. It will jump all the way to the new location of the stitch and then stitch. But, it should actually jump to within range of the stitch, if it is within that range the jump is unneeded.

It would crop up in some other places. Namely if you could bridge a gap with 4 jumps, to a new location. It might actually also be able to bridge that gap with 3 jumps and a stitch. And will do 4 jumps. Hit the new location. Then stitch with an offset of 0,0.

Clearing the state_trimmed is technically wrong. It is still trimmed during that jump. Correctly it needs to basically force an invoke of needle_to (or something akin to that). It needs to insert jumps to get within range and then stitch.

This is largely an oversight within constrained_step_to() namely the "striking distance" and the max allowed distance are taken to be the same thing. While this is correct if all commands are JUMP or all commands are STITCH, you might want to JUMP to within "striking distance" of a STITCH where your jumps are larger than your stitches. So to be technically correct requires properly calculating that constraint for what could be mixed length values.

This would, in theory only crop up for VP3 (of supported formats) which does not have stitches above 255, but could jump 32768 but these commands are actually identical in the format so it doesn't really matter, and that jump alone is exceptionally likely to make it in 1 given the lack of ten foot wide hoops (on single head machines, mind you, there are multihead machines that can get hoops that big but they work on different parts within a given range).

I'll modify the constrainted_step_to() to overtly define striking distance, and optimize them to exclude superfluous jumps. If your overt JUMP commands got within the range of the STITCH the encoder should change nothing.

tatarize commented 6 years ago

Well, I fixed that bug, which then oddly exposed another bug (my constrained stitch started at 0% causing a double stitch at that point). Which I then fixed. Which then exposed the fact that this method deviates between formats way more than I thought. It works for DST but they have the command STITCH mean MOVE(X,Y) NEEDLE_STRIKE, other formats have the command STITCH as NEEDLE_STRIKE, MOVE(X,Y)

If you don't jump all the way to the correct location, that needle strike is going to be in the wrong location, stitching the last jump.

tatarize commented 6 years ago

This does oddly mean that depending on the format you are saving to it will actually decide on a per format basis as to whether it needs to slip another jump in there or not. As I'm taking JUMP and stitch to consistently mean "movement then needle operation." Though you could just turn full_jump off and not have that happen.

lexelby commented 6 years ago

The latest code looks great. I love the control I get with full_jump. Thanks!