Skip to content

Conversation

@The-Lennzer
Copy link

@The-Lennzer The-Lennzer commented Jan 5, 2026

…tAttributes

Issue: #2138

Describe your changes

  • added interpolation to mesh animations in QuakeMDL
  • interpolation NOT added to Texture animations
  • used vtkInterpolateDataSetAttributes for interpolation as suggested and took care of RAII
  • added new import: #include <vtkInterpolateDataSetAttributes.h>
  • added new vtkInternals struct member: vtkSmartPointer<vtkInterpolateDataSetAttributes> Interpolator

DRAFT PR for reviewing code not to be merged!


// Calculate interpolation factor (alpha)
double alpha = 0.0;
if (frameIndex0 != frameIndex1 && (t1 - t0) > 1e-9)
Copy link
Member

Choose a reason for hiding this comment

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

why 1e-9 specifically ?

Copy link
Author

Choose a reason for hiding this comment

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

In order to make sure that the two frames are unique. 1e-9 is really strict. I think we can remove this check as well if you think that's okay.


vtkIdType ActiveAnimation = -1;

vtkSmartPointer<vtkInterpolateDataSetAttributes> Interpolator;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
vtkSmartPointer<vtkInterpolateDataSetAttributes> Interpolator;
vtkNew<vtkInterpolateDataSetAttributes> Interpolator;

Copy link
Author

Choose a reason for hiding this comment

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

will apply 👍🏼

{
this->Internals->Texture->SetInputData(this->Internals->GroupSkins[animIndex][frameIndex]);
// For texture animations, use nearest neighbor approach
// Not interpolating here since texture interpolation is complex(?)
Copy link
Member

Choose a reason for hiding this comment

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

yes its fine and out of scope

Copy link
Author

Choose a reason for hiding this comment

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

OK 👍🏼

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 and remarks

@mwestphal
Copy link
Member

please resolve adressed discussions and ask me for a review @The-Lennzer when its ready :)

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.

2 participants