-
Notifications
You must be signed in to change notification settings - Fork 260
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
Implement feature #697053 #1
Implement feature #697053 #1
Conversation
My contact e-mail is anon-contact@yandex.ru. |
You forgot |
Err, I guess g has file scope only. I still think history_item is more appropriate than mark, as there exist lots of marks with different meanings in the codebase (but I will grant your request anyway). |
d68f966
to
be4453b
Compare
platform/x11/pdfapp.c
Outdated
return result; | ||
} | ||
|
||
void pdfapp_restore_mark(pdfapp_t *app, mark_t state) |
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.
s/state/mark/
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.
You surely consider the naming to be important. Fixed in X11, OpenGL version already had the right name.
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.
Note for future:
Don't squash review fixes in base commit over and over again, just wait until review is accepted. And it's better to do fixes with git commit --fixup <base_commit>
.
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.
Got it.
be4453b
to
0ef6c01
Compare
I have implemented a variant of this in commit 5af32f6. I remember the scrolled page position, independent of zoom level. If I change the zoom level, I want it to stay changed, so putting the zoom in the history stack is not something I want. |
The purpose of a "snapshot" save is to allow us to dump the current state of a PDF document (including edits, but excluding undo/redo history) in such a way that the version in memory remains unchanged. There are a couple of use cases for this: 1) Load a form, fill in some fields, print it. In order to do the print, we need to save the document as a standard valid PDF to send to a remote print service. After printing, if we then edit the document some more and save it out, we only want to see 1 incremental section used, rather than 2 (i.e. the saving for printing should not cause the 'underlying' document to be updated). 2) When running as an app on a mobile device, when we are put into the background, we need to save our state so that if the app is killed and later restarted, we can pick up where we left off. Again this should not involved writing a new incremental section to the document. This commit solves for case #1. Case #2 will require this, plus both the ability to save undo/redo history, and the ability to 'reopen' the last incremental update.
This is the implementation of feature request #697053.
The feature is implemented for both X11 and OpenGL apps. 'm'arking now remembers the position, zoom rate and rotation state. These properties are restored on 't'aking.