Code review comment for lp:~tomasgroth/openlp/mupdf

Revision history for this message
Jonathan Corwin (j-corwin) wrote :

(Note comment only, I've not looked at the code)

I too think it is nice to keep it in the Presentation plugin. The user is dealing with a PDF file as far as they are concerned and transporting the PDF in the service file rather than lots of images is in my view the correct way of doing this. Importing a PDF in Presentations and having to look for it in the image plugin would confuse many. Also by moving the PDF around it protects us from adding it on one computer with a 640x480 resolution moving to a 1900x1080 system and ending up having to deal with images in an resolution that don't scale nicely.

In the future some other app (e.g. Impress) might support the opening of PDF files and in this case the user would then be able to choose between mupdf or something else. If we're only storing the images then that wouldn't be so easy.

Yes it would be nice if we could just hook into the already existing image handling functionality of OpenLP for the display, but I wouldn't want us to tear the code base to pieces to get this to work. (I don't know the difficulties of this). I don't see any harm in making it a two phase task anyway and to do this later if we find a nice way to do it.

Finally an idea has just come to mind that in the future we could also support an "Export to Images" function on the presentation whereby we could copy the thumbnail images (created at output resolution) to the an Image Plugin group, to at least cater for those situations where the user knows the presentation application isn't installed at the destination system. However this would be outside the scope of this particular branch and would be a whole new feature.

« Back to merge proposal