search

fpw / avitab

X-Plane plugin that displays a tablet to aid VR usage
GNU Affero General Public License v3.0
299 stars 58 forks source link

Add suport for non-PDF files served by chart providers. #207

Closed mjh65 closed 2 months ago

mjh65 commented 3 months ago

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.

mjh65 commented 3 months ago

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.

mjh65 commented 2 months ago

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 commented 2 months ago

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.

mjh65 commented 2 months ago

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.