Skip to content

Add progress report for vtkF3DAssimpImporter#2703

Open
Hy-LeTuan wants to merge 7 commits intof3d-app:masterfrom
Hy-LeTuan:fbx-importer
Open

Add progress report for vtkF3DAssimpImporter#2703
Hy-LeTuan wants to merge 7 commits intof3d-app:masterfrom
Hy-LeTuan:fbx-importer

Conversation

@Hy-LeTuan
Copy link
Contributor

Changes

Add progress reporting to vtkF3DAssimpImporter in the Assimp plugin through attaching a derived class of Assimp::ProgressHandler on the native Assimp reporter and use the parent importer to invoke progress report event.

Continuation of this issue: #22

@Hy-LeTuan Hy-LeTuan marked this pull request as ready for review December 9, 2025 04:27
@Hy-LeTuan Hy-LeTuan changed the title add progress handler for assimp importer Add progress report for vtkF3DAssimpImporter Dec 9, 2025
@Hy-LeTuan
Copy link
Contributor Author

@mwestphal could you check this out please?

Copy link
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

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

some questions :)

@mwestphal
Copy link
Member

Hi @Hy-LeTuan

Need any help moving forward ?

@Hy-LeTuan
Copy link
Contributor Author

I think everything is all done? All tests passed on my machine

@mwestphal
Copy link
Member

I think everything is all done? All tests passed on my machine

Ill review, next time please ping me when you need a review :)

@Hy-LeTuan
Copy link
Contributor Author

Yes sir I'll ping you next time!

Copy link
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

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

LGTM! Ill run CI

@mwestphal
Copy link
Member

\ci full

@mwestphal
Copy link
Member

Looks like TestDefaultConfigFileAssimpDAE is unhappy :)

@mwestphal
Copy link
Member

Needs any help moving forward @Hy-LeTuan ?

@mwestphal
Copy link
Member

Still working on this @Hy-LeTuan ?

@Hy-LeTuan
Copy link
Contributor Author

There was a problem with a default test that is preventing the PR to be merged. I tried solving it but am still not sure what's wrong and you said you'd take a closer look so I'm waiting on your comments sir.

@mwestphal
Copy link
Member

There was a problem with a default test that is preventing the PR to be merged. I tried solving it but am still not sure what's wrong and you said you'd take a closer look so I'm waiting on your comments sir.

I completely forgot! I will take a look

@mwestphal mwestphal self-assigned this Feb 18, 2026
@mwestphal mwestphal self-requested a review February 18, 2026 07:45
@github-actions
Copy link

Style Checks CI failed:

@mwestphal
Copy link
Member

There was a problem with a default test that is preventing the PR to be merged. I tried solving it but am still not sure what's wrong and you said you'd take a closer look so I'm waiting on your comments sir.

Its caused the the ProgressEvent somehow, I need to investigate more

@mwestphal
Copy link
Member

@Hy-LeTuan

There is a bug in VTK! It's a bit of a specific one though, in short, rendering the progress widget on top of the scene seems to render lights again, using the default light kit, which makes the final render completely incorrect.

In order to workaround that just modify the TestDefaultConfigFileAssimpDAE to use --loading-progress=false with a link to the issue:

#2900

Copy link
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

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

needs a rebase on master and a small test change

Copy link
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

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

nope

@mwestphal mwestphal assigned Hy-LeTuan and unassigned mwestphal Feb 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants