frescobaldi / qpageview

page-based viewer widget for Qt5/PyQt5
https://qpageview.org/
GNU General Public License v3.0
20 stars 9 forks source link

[PyQt6] Copy to Image #29

Open igneus opened 2 months ago

igneus commented 2 months ago

In (current qt6) Frescobaldi I select a portion of the musicview by dragging the mouse with right button pressed. Then I right-click the selection to get the context menu. There I select "Copy to Image". When Type SVG, PDF or EPS is selected in the Copy to Image dialog, any interaction with the form crashes like

Traceback (most recent call last):
  File "/home/igneus/apps/qpageview/qpageview/backgroundjob.py", line 98, in _slotFinished
    self.finish()
  File "/home/igneus/apps/qpageview/qpageview/backgroundjob.py", line 90, in finish
    self.finalize(self.result)
  File "/home/igneus/apps/qpageview/qpageview/backgroundjob.py", line 129, in finalize
    callback(result)
  File "/home/igneus/apps/frescobaldi/frescobaldi_app/copy2image.py", line 291, in exportDone
    self.imageViewer.setDocument(document)
  File "/home/igneus/apps/qpageview/qpageview/view.py", line 428, in setDocument
    pages[:] = document.pages()
               ^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'pages'
bmjcode commented 2 months ago

This is a bug in the QtPdf backend. The real problem is buried in the console output:

Traceback (most recent call last):
  File "D:\Users\Benjamin Johnson\AppData\Local\Programs\Python\Python312\Lib\site-packages\qpageview\backgroundjob.py", line 69, in run
    self.result = self.work()
                  ^^^^^^^^^^^
  File "D:\Users\Benjamin Johnson\AppData\Local\Programs\Python\Python312\Lib\site-packages\qpageview\export.py", line 151, in document
    doc = self._document = self.createDocument()
                           ^^^^^^^^^^^^^^^^^^^^^
  File "D:\Users\Benjamin Johnson\AppData\Local\Programs\Python\Python312\Lib\site-packages\qpageview\export.py", line 302, in createDocument
    return svg.SvgDocument([self.data()], self.renderer())
                            ^^^^^^^^^^^
  File "D:\Users\Benjamin Johnson\AppData\Local\Programs\Python\Python312\Lib\site-packages\qpageview\export.py", line 139, in data
    self._result = self.export()
                   ^^^^^^^^^^^^^
  File "D:\Users\Benjamin Johnson\AppData\Local\Programs\Python\Python312\Lib\site-packages\qpageview\export.py", line 295, in export
    success = self.page().svg(buf, rect, self.resolution, self.paperColor)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\Users\Benjamin Johnson\AppData\Local\Programs\Python\Python312\Lib\site-packages\qpageview\page.py", line 370, in svg
    return self.output(svg, source, paperColor)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\Users\Benjamin Johnson\AppData\Local\Programs\Python\Python312\Lib\site-packages\qpageview\page.py", line 271, in output
    self.print(painter, rect, paperColor)
  File "D:\Users\Benjamin Johnson\AppData\Local\Programs\Python\Python312\Lib\site-packages\qpageview\page.py", line 540, in print
    self.renderer.draw(self, painter, k, t, paperColor)
  File "D:\Users\Benjamin Johnson\AppData\Local\Programs\Python\Python312\Lib\site-packages\qpageview\pdf.py", line 284, in draw
    p = doc.page(page.pageNumber)
        ^^^^^^^^
AttributeError: 'QPdfDocument' object has no attribute 'page'

I think the draw() function was one I copied from the Poppler backend intending to rewrite it later. Time to get on that I guess.

bmjcode commented 2 months ago

After further investigation, it turns out we had several problems here. The immediate crash was caused by the above Poppler-ism, which is fixed in 6b13edbb536b8961abb9e59379444fe2b263ff86, but fixing that revealed two further crashes depending on what image type was selected.

Copying to PNG or JPEG would crash because PdfRenderer.renderImage() wrongly assumed that paperColor is always set. This is fixed in the previously mentioned commit; copying to these formats should work correctly now.

SVG, PDF, and EPS are more complicated. 450cd9c0f9e9f76d9c0cb46ac74e1107f6ff2118 and 07f0170255b834db5da5964690d4756fb66ee2cf fix exceptions caused by renamed enum constants, but the actual functionality remains broken until I can work out how that logic works. The SVG and PDF functions are straight Qt code, but EPS relies on Poppler, so I'm not sure how readily that can be ported over.

igneus commented 2 months ago

There is one more broken feature in the Copy to Image dialog: checking the "Gray" checkbox now makes the background black.

bmjcode commented 2 months ago

I have rewritten most of the QtPdf rendering code after studying AbstractRenderer and PopplerRenderer more closely. The new version is much clearer and fixes several problems, including the grayscale background bug mentioned above.

Where we stand now:

Feature Status Notes
Screen rendering Working Uses render()
PNG and JPEG rendering Working Uses render()
SVG output Cropped correctly, but severely pixelated Uses draw() directly
Printing Cropped correctly, but severely pixelated Uses draw() directly
PDF output Not working, and I don't know why Uses draw() directly?
EPS output Not working Implementation relies on Poppler
bmjcode commented 1 month ago

After smacking my head against the wall all morning trying to fix it, I've realized the problem with printing is that Poppler supports vector output, but QtPdf does not. This did result in much cleaner drawing code, so at least it wasn't a complete waste of time.

If anyone wants to work on this further, go ahead. It's not a priority for me because I've never needed to print from Frescobaldi.

bmjcode commented 1 month ago

It turns out printing was a pretty simple fix (which is not to say that it was easy or intuitive). When we're displaying things on screen our painter coordinates are actual size; when we're printing they're scaled to the device's resolution.

Copy to SVG/PDF/EPS remains unworkable with QtPdf because it does not support vector output.