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

Add mimetype to pclx files #1698

Merged
merged 2 commits into from
Mar 7, 2022
Merged

Add mimetype to pclx files #1698

merged 2 commits into from
Mar 7, 2022

Conversation

scribblemaniac
Copy link
Member

This includes a special "mimetype" file at the beginning of the zip archive of pclx files containing the mime type used for pclx files. Many common zip-based file formats (epub, openraster, openoffice, krita, and more) use this specific approach to help distinguish their files from other zip files. This provides a unique file signature for pclx projects and allows them to be identified by other programs such as the "file" utility on linux, and possibly file association on other platforms as well. This should not impact the reading of project files in any way since the file is outside the data directory and in fact won't even appear in the temporary directory.

@scribblemaniac scribblemaniac added 🔹 Minor PR (only one reviewer required) Enhancement File Save labels Mar 1, 2022
@scribblemaniac
Copy link
Member Author

Apparently fmemopen is not a thing on Windows. I will have to look at some alternate way of doing this on Windows.

@scribblemaniac scribblemaniac changed the title Add mimetype to pclx files [WIP] Add mimetype to pclx files Mar 3, 2022
This makes it possible to do cross-platform writing of data to the
zip archive without writing the data to the hard drive first.

Currently this is used to write the special mimetype file, but
without having looked into it, it could be useful for faster writing
of the main xml file or even frame data already in memory (cached
or dirty).
@scribblemaniac scribblemaniac changed the title [WIP] Add mimetype to pclx files Add mimetype to pclx files Mar 4, 2022
@scribblemaniac
Copy link
Member Author

Alright, I fixed it with a different solution. Should be cross-platform now.

@MrStevns
Copy link
Member

MrStevns commented Mar 6, 2022

Regarding the miniz changes, wouldn't it be better to apply these changes to the fork we have and then bump the version? https://github.com/pencil2d/miniz

@scribblemaniac
Copy link
Member Author

There are no changes to miniz here, only to the QMiniz wrapper which is not a part of that fork. I initially did it this way to make updating the library as simple as possible, but with some of the latter changes I did, it ended up being C++ specific, so it could not be done this way in the miniz library which is purely c-based.

Looking at that fork, there doesn't seem to be any reason for us to use that anymore. There's only one different commit, which claims to change fopen_s for _wfopen_s. However, it doesn't actually appear to do that. Moreover, it appears that change has already been done upstream.

@MrStevns
Copy link
Member

MrStevns commented Mar 6, 2022

🤦‍♂️ ah you're right, I failed to see that it was the wrapper you modified. I have no objections in that case 👍

@chchwy
Copy link
Member

chchwy commented Mar 7, 2022

I just realized the miniz source in https://github.com/pencil2d/miniz is not exactly the code we use in the mina repo (damn it that was my bad).

I made two changes in our midified version:

  1. replace fopen_s by _wfopen_s for Unicode file path support.
  2. convert the UTF8 file paths to UCS2 because that's the character encoding used by _wfopen_s

As for what @scribblemaniac said, yes, the upstream has implemented the same thing in this PR as what I did. We should consider using the upstream implementation instead of ours. However, the PR is not in a released version yet.

@chchwy
Copy link
Member

chchwy commented Mar 7, 2022

I have tested this PR and it works as expected. Going to merge.

@chchwy chchwy merged commit 0467c63 into pencil2d:master Mar 7, 2022
@scribblemaniac
Copy link
Member Author

@chchwy Thanks for looking into that.

@scribblemaniac scribblemaniac deleted the mimetype branch March 22, 2022 01:39
@MrStevns
Copy link
Member

FYI after this change, I can't unzip the project on mac os anymore with the standard "archive utility"
image

@chchwy chchwy added this to the 0.7.0 milestone May 20, 2022
@MrStevns MrStevns modified the milestones: 0.7.0, v0.6.7 Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement File Save 🔹 Minor PR (only one reviewer required)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants