-
Notifications
You must be signed in to change notification settings - Fork 24
Handle edge case where .rm lines have only one point (requiring 2) #63
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
Handle edge case where .rm lines have only one point (requiring 2) #63
Conversation
Unfortunately I can only try and reproduce it on v3, not on v2 since my tablet is already updated to version 3. I tried to reproduce it, and managed to do so. I put down a large amount of dots on the page.
Edit: I'm still investigating, can't make the conclusion just yet Edit2: The file wasn't uploaded to the ReMarkable api yet, that's the only logical conclusion I can make here. |
Ok.. I'm not sure. I tried placing a very large amount of dots on a new document. Both ReMarkable's official software as well as Remarks render the document correctly without any exceptions. Offical pdf: dots.pdf Remarks pdf: dots _remarks.pdf I cannot seem to reproduce the original error where the points array had only one point in it, it might have been an issue with v2? Since the original document is a v2 document. My idea is to skip the single-point "line segments" and print a warning whenever this does occur so we can investigate further if it ever pops up again. |
I can attach the associated v2 notebook, since it is my own notebook without anything private in it. So I will add that test case in this PR as well |
@lucasrla It's ready. I wasn't able to reproduce the exact issue, but I did add the original document where the remarks software would crash as a test. Let me know if you have any comments |
Not sure I follow what you meant by not being able to reproduce the issue. If I comment out the two continues, I do get a crash: python -m remarks v2_notebook_complex/ v2_notebook_complex-out-$(date +"%Y%m%d_%H%M%S")/ --per_page_targets svg png md pdf --modified_pdf
Found 1 documents in "v2_notebook_complex/", will process them now
File: "Gosper.notebook" (bce041cf-ce31-4ede-ad11-2e53db0f6c77)
...
shapely.errors.GEOSException: IllegalArgumentException: point array must contain 0 or >1 elements For now, I believe that what you did (logging.warning and continue) is fine. I would just change the messaging to be more verbose but (in my opinion) clearer and more actionable: logging.warning("- Found a line segment with one or fewer lines, will ignore it. Please report this at https://github.com/lucasrla/remarks/pull/63")
logging.warning("- Found a line segment with only a single point, will ignore it. Please report this at https://github.com/lucasrla/remarks/pull/63") |
It does look like an eraser there! I've never really tested the eraser tool thoroughly. So I won't be surprised at all that remarks has issues with it... |
I'm unable to reproduce the issue in v3 is what I meant. My hypothesis was that whenever we encounter a line segment with only one point, ReMarkable would render a dot, whereas we initially crashed, and otherwise ignore it. But it's impossible to tell with the gosper notebook, since it has those lines all over the place. So my approach was to try and make a new notebook, place a large amount of dots and hope to reproduce the error in v3 that way. No luck. |
Agreed, this would be more actionable. For my own understanding, the lines consist of segments, right? So it should be worded the other way around?
And not
|
I'd say that the parser "sees" just segments. I think introduced the concept of lines in order to take advantage of Shapely. As far I remember, lines were always created out of segments. But in terms of the text message, let's keep things simple for users: # drawing.py
logging.warning("- Found a segment with a single point, will ignore it. ...")
# parsing.py
logging.warning("- Found a segment with a single point, will ignore it. ...") BTW, it just occurred to me, do you know if the two warnings are redundant? That is, their conditions are the same and they're always printed out together? If so, let's remove one of them. Thanks |
Sorry but I think I'm still missing something!
By "in v3" you mean using the rmscene parser for v6 .rm lines created within reMarkable software >= 3.0? |
The gosper file is made on ReMarkable software version v2.x. v3 is my tablet's version, producing v6 .rm files. I get the confusion now :p |
I just opened new issue to get a fresh URL for reports from users. We covered other topics here. I'd put #64 in the warning. Thanks again! |
Yes, if the one in parser.py gets executed, the one in drawing.py gets executed as well. Given the .rm-v5 gosper notebook (`v2_notebook_complex), it prints out a very large amount of warnings. |
Prevented it. I think it's ready to merge |
I encountered an edge case within one of my documents where
drawing.py
andparsing.py
would crash because certain stroke segments contained only a single point.This change at the very least prevents the crash, but I don't think I'm handling it correctly just yet.
When I convert the relevant page using the e-mail pdf functionality on my ReMarkable, I get this:
Gosper.pdf
Using Remarks, it looks vastly different. It's hard to make out from this document what's going on, because remarks clearly displays an amount of strokes that the official software does not. (Could be the eraser?) Although that's a different problem.
(Note, this export comes from my local version of remarks, not from the most recent master branch in this repo)
Gosper-remarks.pdf
I think the best course of action is to try and reproduce the problem from scratch. Simply putting down a bunch of dots and hope we reproduce it that way