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

Add previews for more file formats in start page using f3d #17069

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

andesfreedesign
Copy link
Contributor

The purpose of this modification is to be able to display on the Start Page, the thumbnails of 3D files of other formats, in addition to FreeCAD's own (FCStd)

thumbnail_start
Here you can see the thumbnail of a .stl and .step file that I imported for testing...and even the thumbnail of the Schenkel.stp file, from "Examples" is displayed

For this you must have the F3D viewer installed on the system
https://f3d.app/
"F3D is a fast and minimalist 3D viewer desktop application. It supports many file formats, from digital content to scientific datasets (including glTF, USD, STL, STEP, PLY, OBJ, FBX, Alembic), can show animations and support thumbnails and many rendering and texturing options including real time physically based rendering and raytracing."

Captura de pantalla_2024-10-07_02-06-55
Here you can view thumbnails of some of the file formats...

And it is multiplatform (Windows, MacOs, GNU/Linux)
Captura de pantalla_2024-10-07_11-24-18

.arg(qfi.baseName()); // Temporary path for the thumbnail

QString command =
QString(QLatin1String("f3d --config=thumbnail --load-plugins=occt --verbose=quiet "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems like it just assumes this is installed and in PATH, do we need it at all? isn't freecad itself capable of generating these thumbnails? Also is this happening in the background? because otherwise it could be quite the slowdown to generate these thumbnails.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, FreeCAD only generates its own thumbnails for FCStd files...if you look closely at the Examples files, the Schenkel.step thumbnail is not displayed
For those who work with FreeCAD importing files from other formats, it is very useful to be able to view the thumbnails in "Recent Files"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it currently doesn't do it, my point was that it should be possible to make it although I understand if you are not interested in implementing that. My other points still stand though

Copy link
Member

@adrianinsaval adrianinsaval Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if f3d will be used it's path should be configurable because on mac and windows it's not as common to have 3rd party executables in your PATH.

@maxwxyz maxwxyz added this to the 1.0 milestone Oct 7, 2024
@adrianinsaval adrianinsaval modified the milestones: 1.0, Post 1.0 Oct 7, 2024
else {
// If it is not a FreeCAD file, generate thumbnail using F3D
QString thumbnailPath = QString(QLatin1String("%1/%2_thumbnail.png"))
.arg(QLatin1String("/tmp"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the path should not be hardcoded, you should be using Application::getTempPath() I believe

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we be removing Application::getTempPath() if it's better to use the one provided by qt?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we be removing Application::getTempPath()

No. Application::getTempPath() points to a different directory (can be even customized by the user) than QDir::tempPath() which is not cleaned up by the system after a reboot.

And since the currently created thumbnails of FCStd files are saved to /tmp (under Linux) it makes no sense for me to choose a different directory for f3d thumbnails.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a pending PR #18095 that creates the thumbnails in a directory as specified by freedesktop.org: https://specifications.freedesktop.org/thumbnail-spec/0.8.0

@luzpaz
Copy link
Contributor

luzpaz commented Oct 14, 2024

Just adding the F3D repology badge to see its spread within the downstream packaging ecosystem:

Packaging status

Edit: I've opened a ticket request to update the OpenMandriva F3D package to the latest stable (OpenMandrivaAssociation/distribution#2993)

@@ -190,16 +190,50 @@ void DisplayedFilesModel::addFile(const QString& filePath)
if (!qfi.isReadable()) {
return;
}

// Check if FreeCAD can open the file by its extension
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code below is self-explanatory enough to make this comment obsolete. The same applies to the comments up to line 204

.arg(filePath); // Input file

// Run the f3d command to generate the thumbnail
int result = std::system(command.toStdString().c_str());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use system() to run an external process because this will freeze the host application. Use https://doc.qt.io/qt-6/qprocess.html instead.

@luzpaz
Copy link
Contributor

luzpaz commented Oct 15, 2024

Just stating the obvious, by accepting this PR we are adding yet another dependency to our long list of dependencies. Also F3D is under the BSD-3 license. Do we need to mention this in the About dialog too ?

@adrianinsaval
Copy link
Member

Just stating the obvious, by accepting this PR we are adding yet another dependency to our long list of dependencies. Also F3D is under the BSD-3 license. Do we need to mention this in the About dialog too ?

As I understand it f3d would be an optional dependency, but if we will distribute freecad with f3d included we probably should

@adrianinsaval adrianinsaval changed the title Update DisplayedFilesModel.cpp Add previews for more file formats in start page using f3d Oct 15, 2024
@adrianinsaval
Copy link
Member

see #17121 for an example of calling a binary from PATH by default or from a configured location if available.

@maxwxyz
Copy link
Collaborator

maxwxyz commented Nov 17, 2024

any news on this PR?

@wwmayer
Copy link
Contributor

wwmayer commented Nov 20, 2024

This PR should be re-checked after #17876 has been merged.

@maxwxyz
Copy link
Collaborator

maxwxyz commented Nov 24, 2024

@andesfreedesign ping. Also has some conflicts.

@andesfreedesign
Copy link
Contributor Author

@andesfreedesign ping. Also has some conflicts.

Hi
These days I will modify the code with the recommendations that you made me...

@luzpaz
Copy link
Contributor

luzpaz commented Nov 28, 2024

@andesfreedesign next merge meeting is Friday and next Monday. Perhaps you can get the conflicts fixed by then ?

@chennes
Copy link
Member

chennes commented Dec 6, 2024

@andesfreedesign can you address the issues raised by @adrianinsaval and @wwmayer (related to setting explicitly the location of f3d, and to not using std::system). Thanks!

@maxwxyz
Copy link
Collaborator

maxwxyz commented Dec 8, 2024

@andesfreedesign ping for feedback

@maxwxyz
Copy link
Collaborator

maxwxyz commented Dec 13, 2024

@andesfreedesign please address the issues

@hasecilu
Copy link
Contributor

I have some fixes here if you want to test it, https://github.com/hasecilu/FreeCAD/tree/feat/f3d_thumbnails

@maxwxyz
Copy link
Collaborator

maxwxyz commented Dec 16, 2024

@andesfreedesign ping for feedback. I'll label this PR as stale so we won't tag you each week. If you come back to this PR just push or comment and we remove the label.

@maxwxyz maxwxyz added the Status: Stale Stale and might be closed automatically. label Dec 16, 2024
@maxwxyz maxwxyz removed the Status: Stale Stale and might be closed automatically. label Dec 20, 2024
@maxwxyz
Copy link
Collaborator

maxwxyz commented Jan 4, 2025

waiting for review

@adrianinsaval
Copy link
Member

For some reason this was not working for me even though I had f3d installed and on PATH, I'll have a closer look tomorrow and try to figure out what is happening

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

Successfully merging this pull request may close these issues.

7 participants