search

EmbroidePy / pyembroidery

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

Phc Load Issue. #36

Closed tatarize closed 5 years ago

tatarize commented 5 years ago

@mgua I checked the load issue you posted in inkstitch/pyembroidery#68 and it does seem to be a real issue. Odds are good I move around the file in some invalid way that is exposed by the later version but held true for Phc version 1.

tatarize commented 5 years ago

Upon review, your EM0000007 example really does look like that. And the corrections made here to EmbroidePy/pyembroidery to fix inkstitch/pyembroidery#59 cover the error. In the reader, line 36 reads:

f.seek(bytes_in_section3 + 0x12, 1)

and it should read:

f.seek(bytes_in_section3 + 0x1D, 1)

At the B characters between those positions are PEC stitch header info, and a few set bits caused it to read that as a long stitch (when it's not a stitch at all) which caused it to enter the pec block off by a character which causes a completely invalid read event.

@lexelby make that change in your version and it'll rectify @mgua's issue as well as close part of inkstitch/pyembroidery#59.

The other part of 59 is that PHB line 28 reads:

f.seek(color_count2 + 23, 1)

and should read:

f.seek(color_count2 + 21, 1)

It reads that 2 bytes into the PEC stitch block which means if the first element required a long-encoded value it missed that fact and read the 2nd stitch, which is sometimes the last half of the long stitch and first part of the new stitch causing desync.

@mgua due to #1, you may have to copypasta this comment over there or otherwise get lex's attention to fix this old bug on his end.

mgua commented 5 years ago

Thank you very much for you super quick reaction, effectiveness and competence.

Marco

From: tatarize notifications@github.com To: EmbroidePy/pyembroidery pyembroidery@noreply.github.com Cc: mgua mgua@tomware.it, Mention mention@noreply.github.com Date: 28/10/2018 00:09 Subject: Re: [EmbroidePy/pyembroidery] Phc Load Issue. (#36)

Upon review, your EM0000007 example really does look like that. And the corrections made here to EmbroidePy/pyembroidery to fix inkstitch/pyembroidery#59 cover the error. In the reader, line 36 reads: f.seek(bytes_in_section3 + 0x12, 1) and it should read: f.seek(bytes_in_section3 + 0x1D, 1) At the B characters between those positions are PEC stitch header info, and a few set bits caused it to read that as a long stitch (when it's not a stitch at all) which caused it to enter the pec block off by a character which causes a completely invalid read event. @lexelby make that change in your version and it'll rectify @mgua's issue as well as close part of inkstitch/pyembroidery#59. The other part of 59 is that PHB line 23 reads: f.seek(color_count2 + 23, 1) and should read: f.seek(color_count2 + 21, 1) It reads that 2 bytes into the PEC stitch block which means if the first element required a long-encoded value it missed that fact and read the 2nd stitch, which is sometimes the last half of the long stitch and first part of the new stitch causing desync. @mgua due to #1, you may have to copypasta this comment over there or otherwise get lex's attention to fix this old bug on his end. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

tatarize commented 5 years ago

@mgua Be sure to either copy my comments from here to your inkstitch/pyembroidery#68 or reference the issue from here over there and tag @lexelby. After writing pyembroidery for the project I got banned for correcting Lex on how graphical systems refresh while he was in a mood (#1). So while I fixed this particular error months ago, you still likely have to make sure he pays enough attention to the correction to get it put into inkstitch or it will stay broken.