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 tubes for rendering streamlines #1239

Merged

Conversation

nicolemarsaglia
Copy link
Contributor

Apply the VTK-m Tube filter to the output of the Streamline filters to allow for rendering

…g' of https://github.com/Alpine-DAV/ascent into task/2023_12_vtkm_add_tubes_for_particle_advec_rendering
else
dz = dist_z/num_seeds_z;

double epsilon = 0.1;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was struggling to render seeds that are directly on the boundary, so I've been offsetting all my points my epsilon.

Note for later for picking a better epsilon:
vtkm::Float64 lx = coordBounds.X.Length();
vtkm::Float64 ly = coordBounds.Y.Length();
vtkm::Float64 lz = coordBounds.Z.Length();
vtkm::Float64 mag = vtkm::Sqrt(lx * lx + ly * ly + lz * lz);
// same as used in vtk ospray
constexpr vtkm::Float64 heuristic = 1000.;
radius = static_castvtkm::Float32(mag / heuristic);
m_tube_size = radius;

Copy link
Member

Choose a reason for hiding this comment

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

So I understand: Is it that the box is exactly aligned a data set boundary causing numeric sadness?

If so, the user could adjust the box to not be aligned. Box params being correct is something user can control.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cyrush

Exactly, anything on the boundary needs to be moved slightly or else we lose the particles.

I guess the check should be if the particle is on the boundary and then adjust, rather than adjusting ALL points like I am.

Here are boundary points that are not and are adjusted using epsilon, respectively.
box_boundary_uniform_0000
box_boundary_uniform_0001

@cyrush cyrush added this to the 0.9.3 milestone Apr 19, 2024
@cyrush
Copy link
Member

cyrush commented Apr 24, 2024

looks great, can we change the file names of the tests to be a more descriptive:

for example, from:

src/tests/ascent/line_seeds.cpp

to

src/tests/ascent/t_ascent_particle_advection_line_seeds.cpp

@nicolemarsaglia
Copy link
Contributor Author

looks great, can we change the file names of the tests to be a more descriptive:

for example, from:

src/tests/ascent/line_seeds.cpp

to

src/tests/ascent/t_ascent_particle_advection_line_seeds.cpp

oops, I meant to delete these. But they should also pass the windows tests no problem. I'll add them . Good catch!

Copy link
Member

@cyrush cyrush left a comment

Choose a reason for hiding this comment

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

Looks good, lets get this merged!

I added two comments, if we want to follow up on those lets create a ticket to remember.

Thanks for getting this all working!

// scene.AddRender(render);
// scene.AddRenderer(&tracer);
// scene.Render();
//
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 turn this back on?
If so, lets add a ticket to track.

// vtkh::Scene scene;
// scene.AddRender(render);
// scene.AddRenderer(&tracer);
// scene.Render();
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 turn this back on?

@nicolemarsaglia
Copy link
Contributor Author

Looks good, lets get this merged!

I added two comments, if we want to follow up on those lets create a ticket to remember.

Thanks for getting this all working!

Sounds good! I'll make a ticket for the warpx tests

@nicolemarsaglia nicolemarsaglia merged commit e3e4732 into develop Apr 29, 2024
20 checks passed
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.

None yet

2 participants