Skip to content
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

Cached export for incremental #2400

Merged
merged 8 commits into from
Oct 17, 2023
Merged

Cached export for incremental #2400

merged 8 commits into from
Oct 17, 2023

Conversation

Dherse
Copy link
Sponsor Collaborator

@Dherse Dherse commented Oct 16, 2023

See #2393

crates/typst/src/export/pdf/mod.rs Outdated Show resolved Hide resolved
crates/typst/src/export/render.rs Outdated Show resolved Hide resolved
crates/typst/src/export/svg.rs Outdated Show resolved Hide resolved
Copy link
Member

@laurmaedje laurmaedje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The memoize annotations on render and svg functions still need to be removed (but you probably know that).

Comment on lines 229 to 232
// If the frame is in the cache, skip it.
if export_cache.is_cached(frame) {
continue;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just thinking that this might be problematic if the file has been deleted in the meantime. Maybe we could check that it is still present with the correct mtime?

Copy link
Sponsor Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a check that the file exist, but I did not add the modified time check as I feel like this is
a) beyond scope since we'd need to keep track of time which I feel is unnecessary complexity
b) unnecessary, if the person messes up their files, it's on them, and otherwise the file would be overwritten anyway and they would lose their modification

Unless you feel very strongly about this, I'd leave it as is

@Dherse
Copy link
Sponsor Collaborator Author

Dherse commented Oct 17, 2023

The memoize annotations on render and svg functions still need to be removed (but you probably know that).

Damn, I did it then I messed up my git and didn't migrate it 😠

@laurmaedje laurmaedje merged commit c0dbb90 into typst:main Oct 17, 2023
3 checks passed
@laurmaedje
Copy link
Member

Thank you again!

@Dherse Dherse deleted the memoize-export branch November 27, 2023 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants