EmbroidePy / pyembroidery

pyembroidery library for reading and writing a variety of embroidery formats.
MIT License
181 stars 33 forks source link

Janome JEF Stop may indicate corruption on some machines. #115

Closed tatarize closed 3 years ago

tatarize commented 3 years ago

inkstitch/inkstitch#905 @MK-12-20MM

Apparently the backend may write JEF files that cannot be loaded, this is a prima-facie bug in pyembroidery.

Almost all rejected files have a trim. IT could be that the given machine does not permit trims. Stops do not exist and would need to be interpreted as color changes to be used.

This may require trims set to false for the given filetype with that machine.

ghost commented 3 years ago

I have never had any issues with applying trims to objects, trims work fine. Trims on multiple layers - never had issues. Every combination of trim I can think of, never had an issue.

Looking back at this - It's been awhile, it seems that on the back end if I apply a stop, then on the back end it gets converted into a trim that where the trouble starts. Granted that stops do not exist.

tatarize commented 3 years ago

It might be trying to do the color change to a non-existent or double-same color but I'm not seeing that either. There's not much of a change with the inkscape or local version. So it seems strange. I do note that your list of test value didn't sanity check a 2-layer jef with no stops. Like one that would trim and another that would just stop which would look like a trim with one failing and the other passing.

Since in that case it might be easier to track down the error.

tatarize commented 3 years ago

You say 6 passed and 7 failed. Analysis shows the stitches are spot on the same but there's some hex differences in the writing routine in the header. I could also test this against 5 that also failed. I'll see what the different values.

tatarize commented 3 years ago

5 is in a different order and thus a problem, since it's apples and oranges.

6 claims there is 1 color. There are 0 color changes. 7 claims there are 2 colors. There are 0 color changes.

Janome is very likely rejecting it as corrupt because the declared number of color changes does not match the real number of color changes.

tatarize commented 3 years ago

7s.zip

Does this file work correct as having a stop?

ghost commented 3 years ago

Give me a bit.. I will try it..

ghost commented 3 years ago

Machine detects this as a single color. So no color stop..

tatarize commented 3 years ago

But, not as corrupt? Does it do a stop at the color change that is clearly within it? The machine might pick up the double color change to the same color and directly call that a stop or something.

tatarize commented 3 years ago

The file clearly has a colorchange event within it, but only 1 color. But everything else is duplicated color-wise. So it might actually just say that that is an proper stop as it's a color change to the same color.

tatarize commented 3 years ago

janome

Janome registers it this way. So it likely has colors rather than color changes. Since there's only black that's 1 color not 2. But, divided with that change from black to black means that's a correct stop command. Assuming the actual sewing machine agrees. In either case there is a bug writing corrupted files. Namely if there's 2 threads but 0 color changes it should say that that is 1 color in total. Which was the first element of failure. Outside of that interpolating stop with a color change appears to work, though having an embroidery machine agree is the gold standard there.

Fixing the corruption bug is a solid first step, but if that gets properly treated as a stop I think they have some similar code there.

ghost commented 3 years ago

File is not corrupt. Reads it fine.. Stitching out for real does not stop or no stop between the objects.

tatarize commented 3 years ago

https://github.com/inkstitch/pyembroidery/blob/c0b817d0626140221e707e88be571728c393a3ca/pyembroidery/JefWriter.py#L27

Should read:

color_count = pattern.count_color_changes() + 1

I think @Kaalleen has some similar code. Which may or may not do stops depending on whether that file gets that treatment from the embroidery machine.

https://github.com/EmbroidePy/pyembroidery/blob/61122b9c74e9e4222363e63501e2e111c5ec8416/pyembroidery/EmbPattern.py#L680

tatarize commented 3 years ago

Okay, so likely it either can't stop or would need a true color change to a different color to qualify. So you could color change to something fake and get a stop but it has no real stop. Okay.

Any option in the machine itself for how to deal with that. Sometimes machines have options to respect all color changes etc and can be coaxed into dealing with that as a stop. It varies from machine to machine but there's clearly a color change in that file. And I would guess some machines could be made to respect that as a stop.

Any options in the machine that might be relevant?

ghost commented 3 years ago

All color changes the machine will stop. Been doing fake color changes for other reasons since owning the machine( 2 years) So for the 400 options are surrounding this

Color change trim. Option to cut on color change. Still stops on color change regardless. This is on or default Cutting command - cuts or trims if cutting command is in imported files.. This is on as well.

I think maybe I need to rerun use cases to narrow things down better (just to ensure the results are the same) since that issue is old aged a couple of months. If you have additional use cases I could run those as well..

ghost commented 3 years ago

new_test.zip

File Corrupt Note 1s.jef NO 1 object satin square -1 color 2s.jef NO 1 object satin square -1 color - 1 trim 3s.jef NO 1 object satin square -1 color - 1 trim -add stop 4s.jef NO Removed embroidery objects 5s.jef NO add 1 object - 2 satin square -1 color (2 objects -same layer) Drawn not copied 6s.jef NO Apply trim to 1st object 7s.jef NO Remove trim 1st object - add trim to 2nd object 7as.jef NO Remove trim from 2nd object - add trim to both object 8s.jef YES Remove all trim. Add stop to 1st object 8sEMB.jef YES Same as 8s.jef but used menu item Embroidery instead of save copy as. 9s.jef NO Remove stop from 1st object. Add stop to 2nd obbject 9sEMB.jef NO Same as 9s.jef but used menu item Embroidery instead of save copy as. 10s.jef YES Add stop to both objects

Corresponding svg are in zip file

ghost commented 3 years ago

Files 5-10 just changed the second object color only

As above 8 and 10 indicated corrupt 5-10c.zip

Interesting enough the 8c jef file that seems corrupt has only 1 color block, but 2 colors in the SVG.. I did not look at 10..

ghost commented 3 years ago

The 8C file or corrupt file - removing the stop in the svg now has 2 color blocks in the jef file.

Machine no longer thinks the file is corrupt. Color stops are detected.. 8cNoT.zip

tatarize commented 3 years ago

The 7s I sent you has a color change in it. So if all color changes are respected it would respect that. Janome's software called it 2 layers, both black.

The difference between 7s and 7 is the color change and a rounding error at the very start. So the color change decorrupted that one but was not respected as a color change.

tatarize commented 3 years ago

New:

Statistics say it has no color changes. 16 jumps, 2518 stitches, 1 trim.

Hex says it has 2 colors. It does not have 2 colors. 8shex

8emb: Identical file (with the exception of an internal timestamp. 20210303111518 vs 20210303112632. Says you saved it 3 minutes later.

10s has 0 color changes. Byte 0x18 is again 2 meaning 2 layers. Which will be found to be corrupt since the number of color changes does not equal the number of stated colors.

tatarize commented 3 years ago

Byte 0x18 in 8c.jef is 02 meaning 2 colors. Looking at the actual stitches in EmbroidePy give s 0 color changes. The file is corrupt.

tatarize commented 3 years ago

8cNoT: 1 COLOR_CHANGE. 2nd color appears red rather than black.

8cnot-view

The hex says 2 colors.

"#","[THREAD_NUMBER]","[HEX_COLOR]","[DESCRIPTION]","[BRAND]","[CATALOG_NUMBER]","[DETAILS]","[WEIGHT]"
"$","0","#000000","Black","Jef","002","None","None"
"$","1","#ff0000","Red","Jef","225","None","None"

CSV details has Black Jef, and Red Jef colors.

This stop/color is just a color change to a different color. You said 7s did not respect the color change there to the same color. Just sewed on passed it?


1) Did you check that 7s does not get respected as a color change? That in the physical sew out it just fails to stop there?

Assuming 1. This means that JEF does not respect color changes to the same color. In order to successfully perform a stop command, it currently builds a nonrepeat palette which pigeon holes each color into the nearest matching jef color. However, if the color is exactly identical it does not bother with this an permits repeating that color. Stops interpreted as a double color change, fail because double-color changes are being ignored by the machine.

In order to process a STOP the jef will have to switch colors to a dummy color.


I think maybe I need to rerun use cases to narrow things down better (just to ensure the results are the same) since that issue is old aged a couple of months.

Yeah, I only once in a while bother to check the inkstitch issues looking for format issues. I was permebanned a few year back after writing pyembroidery for explaining how animation worked, and thus the Lex's understanding of the topic was wrong. (#1) So even after fixing this you'll actually have to go back to your issue and ask that this be corrected in their older code.

If you have additional use cases I could run those as well..

Okay. Jef has within it a magic thread number routine that colors particular numbers particular colors. There is however a color 0 which is seen as "placeholder" and sometimes "black". If this value is actually placeholder, well, I need a placeholder to make stops. So can you try this file with placeholder.

placeholders.zip

Basically JEF has a weird color that also shows up with black. If the color chart is listed such that a stop is treated as a toggle to placeholder color. We'd could actually just use that as a color that means don't switch colors.

7placeholder is a modified 7s with 1 0 for colors rather than 1 1. 7placeholder2 is a modified 7s with 2 0 for colors rather than 2 2.

The assumption here is that maybe the reason this color is called placeholder in places and not a typically used color is that it actually has a function and if it works for this it would be pretty reasonable. A color change without a color. Which would permit stops. And the reader could properly interpret this.

If the machine has colors please tell me if the second color in 7placeholder2 shows up as white (which would be awesome) or black (which is kinda the expected).

From experiments it's clear Line 27 needs to say color_count = pattern.count_color_changes() + 1 so it does not ever make corrupt files. But, it might also be able to easily hack a stop into this code since it appears Jef may have created effective means to do this. Since swapping with a dummy color is a perfectly reasonable method of making stops, especially if swapping with the self same color is off the table.

ghost commented 3 years ago

8cNoT: 1 COLOR_CHANGE. 2nd color appears red rather than black.

8cnot-view

The hex says 2 colors.

"#","[THREAD_NUMBER]","[HEX_COLOR]","[DESCRIPTION]","[BRAND]","[CATALOG_NUMBER]","[DETAILS]","[WEIGHT]"
"$","0","#000000","Black","Jef","002","None","None"
"$","1","#ff0000","Red","Jef","225","None","None"

CSV details has Black Jef, and Red Jef colors.

  • 7s: says 01 00 00 00 01 00 00 00
  • 8cNoT: says 01 00 00 00 0A 00 00 00

This stop/color is just a color change to a different color. You said 7s did not respect the color change there to the same color. Just sewed on passed it?

Yes - the second object is red- (8cNoT). I made it red. The machine will indicate a color change and stop after sewing the 1st object which is black.

  1. Did you check that 7s does not get respected as a color change? That in the physical sew out it just fails to stop there?

Yes the machine only recognizes as stop when there is a color change. In the case of 7s there was no stop after sewing the 1st object (even though you had a 2nd color block in the file (both were #000000 same color).

Assuming 1. This means that JEF does not respect color changes to the same color. In order to successfully perform a stop command, it currently builds a nonrepeat palette which pigeon holes each color into the nearest matching jef color. However, if the color is exactly identical it does not bother with this an permits repeating that color. Stops interpreted as a double color change, fail because double-color changes are being ignored by the machine.

In order to process a STOP the jef will have to switch colors to a dummy color.

Yes that seems the case.

I think maybe I need to rerun use cases to narrow things down better (just to ensure the results are the same) since that issue is old aged a couple of months.

Yeah, I only once in a while bother to check the inkstitch issues looking for format issues. I was permebanned a few year back after writing pyembroidery for explaining how animation worked, and thus the Lex's understanding of the topic was wrong. (#1) So even after fixing this you'll actually have to go back to your issue and ask that this be corrected in their older code.

I am already aware.. No worries. I do understand that Inkstitch uses a forked pyembroidery - resolution is over there.

If you have additional use cases I could run those as well..

Okay. Jef has within it a magic thread number routine that colors particular numbers particular colors. There is however a color 0 which is seen as "placeholder" and sometimes "black". If this value is actually placeholder, well, I need a placeholder to make stops. So can you try this file with placeholder.

placeholders.zip

Basically JEF has a weird color that also shows up with black. If the color chart is listed such that a stop is treated as a toggle to placeholder color. We'd could actually just use that as a color that means don't switch colors.

7placeholder is a modified 7s with 1 0 for colors rather than 1 1.

This file in the machine - 2 colors - with a stop (to change colors) between objects

7placeholder2 is a modified 7s with 2 0 for colors rather than 2 2.

This file also 2 colors with a stop (to change colors) between objects

The assumption here is that maybe the reason this color is called placeholder in places and not a typically used color is that it actually has a function and if it works for this it would be pretty reasonable. A color change without a color. Which would permit stops. And the reader could properly interpret this.

If the machine has colors please tell me if the second color in 7placeholder2 shows up as white (which would be awesome) or black (which is kinda the expected).

Machine has a color display - Left object shows up as off white or close to #d8d8d8 (light grey). In the past I have seen css colors way off unless you use the janome palette).

From experiments it's clear Line 27 needs to say color_count = pattern.count_color_changes() + 1 so it does not ever make corrupt files. But, it might also be able to easily hack a stop into this code since it appears Jef may have created effective means to do this. Since swapping with a dummy color is a perfectly reasonable method of making stops, especially if swapping with the self same color is off the table.

Going through this again today I have other thoughts I will note, I do understand where you are going. I think (HA HA).

Even though I can apply trims all day in any combinations. It seems that the conversion to a trim from a stop placed by inkstitch sets things in motion.

Just from an observation standpoint just with the 2 objects regardless of color, seems the stop conversion to trim plays into this which leads into corruption you already noted.

1) if I apply the stop to the right object and export to jef the file works (9s) 2) if I apply the stop to the left object and export to jef its corrupt (8s)
3) if I apply the stop to the right object and export to jef the file works (9c) 4) if I apply the stop to the left object and export to jef its corrupt (8c)

This part is puzzling -

tatarize commented 3 years ago

What's going on is the data sent contains STOP commands but the stops don't have an analog in the .JEF so this is just filtered out. You're left with the trim that the stop does because trim then stop is a thing you do. Else the person needs to trip things themselves or something. In theory you might be able to not trim depending on the machine. And it would stay connected and then start new stitches without a cut. But, that would be a sort of weird operation.

In any event the writing output that inkstitch is making includes stops (sometimes). It's also duplicating the color. This is likely because it figures it'll use it like a color change, but due to a bug that number of threads itself informs the actual code written there. Which means if it gave you two threads it will write 2 there. The problem is the file is corrupt unless there are 2 color changes in the file. It counts them and says the file is bad if those don't equal up. Which is the primary bug that prima facie brought this to my attention.


The strange color 0 in JEF which is kinda written off could become a boon. If we must actually give a different color to invoke a stop with a color change. Since it continues if the same color is given again. Color 1 to Color 1 means ignore that color change. We can very much insert stops anywhere we want. If we have two colors, say 1 and 7. And we have 4 and 5 stops within these colors we can render that as: 1 0 1 0 1 7 0 7 0 7 0. Since we start with the correct color in each case and we use 0 as a semaphore which means "color change in this location is really a stop" we get the switch to a dummy color requirement perfectly. But, on the flip side, we can also detect this during the read, and our end will work without a hitch and the machine will act appropriately. Nobody actually assigns color 0 to anything since it's "PlaceHolder", "Any Color", "Duplicates color 1" "Duplicates the last color (bamboo)".

It's an entirely competent method of hacking stops into the file. This color-0-toggle stop is pretty straight forward, and reading the code we could turn around and interpret this semaphore as STOP. Giving us both read and write with 1 dummy color that isn't going to be used anywhere for anything. And we'd treat color 0 as a continuation of the previous color, which actually means we have a pretty much lossless algorithm that the machine respects. Other software might call it black or bamboo. But, it might be reasonable enough send them an email and ask that they treat the color the same way. I've done this for Wilcom a couple times when they were misreading PES files. And it's clever enough they might respect the intent for future versions of the software. But, Inkstitch would be able to do reading and writing of Jef with stops and without losing the information of your real intended colors.

tatarize commented 3 years ago

Ok. Done. Sadly the patch is a bit large. The reader and writer need to be altered a bit, and they use a different palette function which isn't used elsewhere. I tried to mark the highly relevant changes with PATCH.

These are basically all the changes to JEF Reader and JEF Writer. There's a Command Mask that's applied to various commands in modern pyembroidery which would need to be omitted if porting back into the old version.

https://github.com/EmbroidePy/pyembroidery/pull/116/files

The first two files there. Basically the same changes.

flags = stitch[2] & COMMAND_MASK can just remove the COMMAND_MASK for the older version. It's doing & 0xFF because doing the needle commands correctly which was the 1.3 upgrade needed the ability to encode all the different needles the machine could be told to switch to.

tatarize commented 3 years ago

If that's too much just change line 27 of JefWriter to color_count = pattern.count_color_changes() + 1 which would at least prevent it from writing corrupt files. Though they still wouldn't have any STOP command. Or this which would absolutely allow you to load and save stops in a way that is respected by your embroidery machine.

ghost commented 3 years ago

Thanks .. :)

I will reference other there to the committed changes here. Ultimately they need to consider implementation into their fork which would be up to them. At least line 27 to avoid machine detected corruption, and user frustration from that standpoint.

Thanks again....