Add suport for non-PDF files served by chart providers. #207
Conversation
There was a problem hiding this comment.
That's interesting, thanks for the investigation. Are the charts served with a proper MIME type so that it could maybe copied from the web server's reply?
Otherwise there could be several other types that are supported by mupdf and only failing due to the invalid MIME type and it would be weird if AviTab had to figure out the format.
Good question. I only looked at the raw buffer that was passed into the function, but I'll do some further investigation. Definitely makes sense to use the MIME type from the web server, and seems unlikely that it isn't available. |
This is the most recent version that does not require an update to mbedtls.
This renaming reflects the more generic capabilities of the underlying MuPDF (fitz) rendering library, and is preparation for enhancements to the ChartFox provider module to support downloading of charts in formats other than PDF.
The PDFViewer class isn't actually used in Avitab. It could be renamed to DocumentViewer in keeping with the other refactoring in this set of updates, but it seems cleaner to simply remove it.
Various symbols including the word PDF are changed to document to better reflect the use/purpose of the named entity.
Preference groups named pdfreading are changed to docreading to reflect the more general capabilities of the related apps. The preference file version number is incremented, and code to upgrade the preference file from version 1 to version 2 is implemented.
Define LocalFileSource and DownloadedSource as specializations of DocumentSource, and move LocalFile-specific methods and data into LocalFileSource class. This is in preparation for downloaded content type, which will be specific to the DownloadedSource class.
Obtain the content type header from the curl get request and use this string as the parameter to the MuPDF fitz rasterizer.
mjh65
force-pushed
the
pr_support_png_charts
branch
from
d8341eb
to
72ed3ac
Compare
|
I have re-implemented this update based on the earlier review comments. The new PR is somewhat more complicated, although most of the changes are refactoring or cosmetic renaming so that the final functional update is relatively trivial and easy to follow. A newer version of the curl submodule was required in order to acess the header fields to get the content type. Since there is a dependency on the mbedtls module, and updating this turned out to be non-trivial, I have selected the latest version of the curl module that does not require any changes to the mbedtls module. Since the purpose of this PR is to support downloaded charts that are not PDF formatted, I have renamed classes and files that had 'PDF' in their names to the more general 'Document', and similarly variables are renamed to reduce the possibility of confusion. I have also removed 2 files that are not linked into the build and appear to be redundant. One of the settings in the json preferences file was called pdfreading. Since this applies to all documents, it has been renamed to docreading, and code is added to support upgrading of the preferences file on first run of Avitab after this PR has been included in the build. The original PDFSource class, now renamed as DocumentSource overloads downloaded documents and local file documents, with some parts of the class only relevant to local files. The final refactoring update was to create subclasses of DocumentSource to represent LocalFileSource and DownloadedSource documents. The actual functional change that obtains the content type of the downloaded document and passes this to the rasterizer is in the final commit and is relatively trivial! |
mjh65
changed the title
|
Closing this PR temporarily, since the updated curl libraries are causing Avitab link errors in MinGW (PR was initially developed on Mac, and has also been built under Ubuntu/WSL). Will resubmit the PR once the link issues are resolved. |
|
Reopening this PR as the MinGW build issue is now fixed. The fix was retro-fitted to the first commit which updates the version of the curl library. (Branch was force-pushed to keep the history of the merge clean). One other (probably) obvious note is that the curl dependency needs a thorough clean after updating before rebuilding. Although it takes a little longer, I ran the clean_dependencies script and then rebuilt all the 3rd party libraries. |
While using the ChartFox interface for CYOW I found that the charts were not displayed (Cannot open document: ...) and traced this issue to the rasterizer assuming that charts are in PDF format. This fix detects the magic number for PNG files at the start of the download buffer and modifies the MIME type argument accordingly.
I guess there may also be other chart file formats that are not handled correctly, but since I have not done an exhaustive search I did not add code to support these speculatively. The same technique used here can be extended if these problems are detected later.