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

[Bug] Import PDF -> First page duplicated #1156

Open
sebojolais opened this issue Nov 5, 2024 · 9 comments
Open

[Bug] Import PDF -> First page duplicated #1156

sebojolais opened this issue Nov 5, 2024 · 9 comments
Labels

Comments

@sebojolais
Copy link
Contributor

Describe the bug

When I import a PDF on the board, the first page is duplicated and paste at the end.

To Reproduce

  • Open OpenBoard
  • Create an new document
  • Go to board view
  • Drag and drop the attached PDF file Untitled 2.pdf
  • The 2 pages of the PDF are added as expected
  • But there is a third page that is the first page of the PDF with a different scale.

This behavior happens for every kind of PDF and OB documents.

To not reproduce

  • Open OpenBoard
  • Go to document view
  • Import the attached PDF file Untitled 2.pdf
  • The new document has only 2 pages as expected

Expected behavior

We expect to add the number of pages of the PDF, no more.

Actual behavior

The Drag and Drop on the board addes the first page at the end

Context

  • reproducibility : systematic
  • OpenBoard version : last dev branch 78ee945
  • OS :
    • bug happens with : Linux Ubuntu 22.04
    • didn't check : Windows macOS
  • If you built OpenBoard from source, please provide the branch you're working on : dev

Additional context

OS detailed version is MANDATORY (if not provided, the issue will be closed as incomplete).
KDE Neon 6.2 (Ubuntu 22.04 based)

@sebojolais sebojolais added the bug label Nov 5, 2024
@letsfindaway
Copy link
Collaborator

letsfindaway commented Nov 5, 2024

Thanks for reporting, but I cannot reproduce that. I followed your steps:

  • Open OpenBoard
  • Create an new document - not explicitly necessary, this is done automatically when starting OpenBoard
  • Go to board view - also not necessary, this is the default view
  • Drag and drop the attached PDF file Untitled 2.pdf
  • The 2 pages of the PDF are added as expected
  • But there is a third page that is the first page of the PDF with a different scale. - Cannot see that.

My result is as follows:
grafik

So the first page of the newly created document stays as it was and the two pages of the PDF are added as Page 2 and Page 3. There is nothing after that.

Is there something else I have to do to reproduce this?

My environment:

  • openSUSE Leap 15.6
  • Qt 5.15 or Qt 6.6, bug cannot be reproduced with either
  • X11, KDE

@sebojolais
Copy link
Contributor Author

sebojolais commented Nov 5, 2024

Interesting, maybe it comes from my setup. Finally I do not use the dev branch but my branch with the filter document feature : https://github.com/sebojolais/OpenBoard/tree/filteringDocuments
Like every developers, I can not imagine that this bug is throws by my modifications. (this is a joke)

Below are the steps to reproduce with screenshots:

  • Open OpenBoard
  • Create an new document

image

  • Go to board view by double click on the first page of the new document.
  • Drag and drop the attached PDF file Untitled 2.pdf
  • The 2 pages of the PDF are added as expected
  • But there is a third page that is the first page of the PDF with a different scale.

image

Qt 5.15, X11, KDE

@letsfindaway
Copy link
Collaborator

letsfindaway commented Nov 5, 2024

Still cannot reproduce. If you want to debug: The relevant code is

else if (UBMimeType::PDF == itemMimeType)
{
qDebug() << "accepting mime type" << mimeType << "as PDF";
qDebug() << "pdf data length: " << pData.size();
qDebug() << "sourceurl : " + sourceUrl.toString();
QString sUrl = sourceUrl.toString();
int numberOfImportedDocuments = 0;
int currentNumberOfThumbnails = selectedDocument()->pageCount();
if(!sourceUrl.isEmpty() && (sUrl.startsWith("file://") || sUrl.startsWith("/")))
{
QStringList fileNames;
fileNames << sourceUrl.toLocalFile();
numberOfImportedDocuments = UBDocumentManager::documentManager()->addFilesToDocument(selectedDocument(), fileNames);
}
else if(pData.size()){
QTemporaryFile pdfFile("XXXXXX.pdf");
if (pdfFile.open())
{
pdfFile.write(pData);
QStringList fileNames;
fileNames << pdfFile.fileName();
numberOfImportedDocuments = UBDocumentManager::documentManager()->addFilesToDocument(selectedDocument(), fileNames);
pdfFile.close();
}
}
if (numberOfImportedDocuments > 0)
{
QDateTime now = QDateTime::currentDateTime();
selectedDocument()->setMetaData(UBSettings::documentUpdatedAt, UBStringUtils::toUtcIsoDateTime(now));
updateActionStates();
int numberOfThumbnailsToAdd = selectedDocument()->pageCount() - currentNumberOfThumbnails;
bool updateDocumentThumbnailsView = UBApplication::documentController->selectedDocument() == selectedDocument();
for (int i = 0; i < numberOfThumbnailsToAdd; i++)
{
emit addThumbnailRequired(selectedDocument(), currentNumberOfThumbnails+i);
if (updateDocumentThumbnailsView)
{
UBApplication::documentController->insertThumbPage(currentNumberOfThumbnails+i);
}
}
if (updateDocumentThumbnailsView)
{
UBApplication::documentController->reloadThumbnails();
}
}
}

In line 1484 the number of added pages is calculated. To actually analyze the import you have to further follow into UBDocumentManager::documentManager()->addFilesToDocument().

Edit: can you check which if path you take in line 1458? For me it is the first. In the second using the temporary file I see a problem that the file is not closed before importing, but afterwards. This might cause that some data is not written? Just an idea...

Second Edit: I just modified the code so that it has to use the else path. Indeed then nothing is imported because the data is not on disk. The file must be closed before the import. But this is another problem and I'm not sure whether the second path is ever executed.

Third edit: I just fixed that other problem occurring when you dropped a PDF URL to the board. See #1157.

@sebojolais
Copy link
Contributor Author

Hi,
Thank you for the useful information.
It seems that for me in the
int UBDocumentManager::addFilesToDocument(std::shared_ptr<UBDocumentProxy> document, QStringList fileNames)
The loop foreach (UBImportAdaptor *adaptor, mImportAdaptors) turns twice.

  • First with an Adaptor UBImportPDF with 2 pages
  • Second one with UBImportImage with only one page

@letsfindaway
Copy link
Collaborator

The loop foreach (UBImportAdaptor *adaptor, mImportAdaptors) turns twice.

  • First with an Adaptor UBImportPDF with 2 pages
  • Second one with UBImportImage with only one page

This is quite interesting. The UBImportImage creates the list of supported formats here:

QStringList UBImportImage::supportedExtentions()
{
QStringList formats;
for ( int i = 0; i < QImageReader::supportedImageFormats().count(); ++i )
{
formats << QString(QImageReader::supportedImageFormats().at(i)).toLower();
}
return formats;
}

So it seems that for your environment the QImageReader also supports reading a PDF document and creating an image from the first page, while this is not the case for me. I think one should leave the foreach loop iterating over the available import adapters once the document was successfully imported by one of them.

@sebojolais
Copy link
Contributor Author

sebojolais commented Nov 6, 2024

[EDIT] With Qt 5.15
In my side, the QImageReader::supportedImageFormats() returns 98 formats.

image

And the PDF is present:

image

I want to understand why it is the case in order to understand if there is a chance that it happens for everybody.

@sebojolais
Copy link
Contributor Author

And with Qt 6.7, I got 20 image formats. with also the PDF format:

image

@letsfindaway
Copy link
Collaborator

I want to understand why it is the case in order to understand if there is a chance that it happens for everybody.

I think I can explain this.

By default, Qt does not support too many image formats. See supportedImageFormats() for a quite short list. But this list can be extended by plugins.

There is a collection of plugins adding more formats, Some of them come with Qt. On my system the packages are called libqt5-qtimageformats and qt6-imageformats for Qt5 and Qt6. Package names may be different on another distribution, or the libraries may even be contained in one of the core packages.

But also KDE adds more plugins for the image reader. On Ubuntu (and probably Neon) they are called kimageformat-plugins and kimageformat6-plugins. These plugins add many more formats. See https://github.com/KDE/kimageformats.

As part of the WebEngine Qt also offers a PDF renderer. This renderer also comes with an image format plugin for PDF. Maybe it is contained in some package called libqpdf... in Ubuntu. If - for whatever reason - this package is installed, then PDF is offered as an image format in the list.

So finally it very much depends on the environment, and various components can add image formats.

In your environment it seems that there are much more installed format plugins for Qt5 than for Qt6. This explains the different length of the list.

So back to your question: Yes, there is a chance that this happens for everybody. At least on Linux, I cannot speak for Windows and MacOS, but generally the plugin architecture for image formats is the same there. And on Linux it might depend on some other software the user has installed, independent of OpenBoard. On the other operating systems we bundle Qt with the application and have therefore better control over the installed plugins.

@sebojolais
Copy link
Contributor Author

Thank you for your detailed answer and for the PR #1158.
🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants