-
Notifications
You must be signed in to change notification settings - Fork 24
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
Fixes FreeTextAnnotation #137
Conversation
if (!annotation.hasAppearanceStream()) { | ||
resetAppearanceShapes(); | ||
} | ||
resetAppearanceShapes(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebuilding a FreeText annotation is fairly complicated and sometimes doesn't look as good as the original. Does the fix rely on this change? If it does there might be something else wrong with the generated content stream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's basically the whole fix. I've noticed that as soon as you move (or change any property of) the annotation, it appears, and given that mouseDragged called resetAppearanceShapes, I simply removed the condition.
Like you said, I see now that it's not a good fix, and there's probably a problem with the stream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any chance you have time to put a test procedure together with a sample file that I can take a look at? I half to admit that 'if' block has come and gone over the years.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHub can't upload the file for some reason, so here is a Dropbox link. I could also send it by mail if you prefer.
It's simply a blank pdf file with a FreeText annotation created with IcePdf in the middle of the page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I grabbed a copy of the file made quite a few edits to the file using a build off master. I didn't have trouble seeing the edits on open. Anything I'm missing in the test case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's strange. I just sent you a mail with a recording of the problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently I need a better monitor. I my dev box I've been resizing the page so that I can see the whole page. If I don't resize and just scroll down I don't see the annotation. I'm thinking this is likely a validate/repaint issue. Will keep looking.
Looking |
Turns out there is some old code in the FreeTextAnnotationComponent.java around line 86 that removes any text operations followed by a complete reset of the annotation shapes. This is what's keeping in the initial paint from happening. This code is really contradictory as it appears to breaking all rendering for existing appearance streams. I'm pretty sure it can be safely removed at this point as I think it's left over from a time when we tried get some of the previously draw commands from annotations that used stuff outside of the annotation spec. Honestly not sure way it's still here. But anyways with the code removed, line 130 should be safe to revert. I like the rest of the changes as it cleans up some loose ends. |
Marking as fixed for now. Will be creating a new issue to address an issue I was seeing when changing a freetext annotations font. The edit takes place but on write the page is garbage collected and the new changes are applied. |
Fixes #136