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

Remove vtk8 support #19518

Merged
merged 28 commits into from
May 29, 2024
Merged

Remove vtk8 support #19518

merged 28 commits into from
May 29, 2024

Conversation

biagas
Copy link
Contributor

@biagas biagas commented May 15, 2024

Description

Remove VTK 8 support from build_visit, xml2cmake, source and CMake .

Resolves #19450

Type of change

  • [ ] Bug fix
  • [ ] New feature
  • [ ] Documentation update
  • Other
    Code cleanup

How Has This Been Tested?

Ran build_visit on pascal, compiled against generated TP libs, ran full regression suite with success.

Compiled on Windows with success.

Checklist:

  • I have commented my code where applicable.
  • [ ] I have updated the release notes.
  • [ ] I have made corresponding changes to the documentation.
  • [ ] I have added debugging support to my changes.
  • [ ] I have added tests that prove my fix is effective or that my feature works.
  • I have confirmed new and existing unit tests pass locally with my changes.
  • [ ] I have added new baselines for any new tests to the repo.
  • [ ] I have NOT made any changes to protocol or public interfaces in an RC branch.

Add reason for having FindPySide  commented out.
Add a toggle option to replace VISIT_OSPRAY.
Copy link
Member

@JustinPrivitera JustinPrivitera left a comment

Choose a reason for hiding this comment

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

This: image makes me very happy. This looks like it was a ton of work, great job!

src/plots/Volume/QvisVolumePlotWindow.C Show resolved Hide resolved
src/plots/Volume/avtVolumeFilter.C Show resolved Hide resolved
@biagas
Copy link
Contributor Author

biagas commented May 23, 2024

@markcmiller86 were you going to review this? If not I will merge as soon as the checks have finished.

Copy link
Member

@markcmiller86 markcmiller86 left a comment

Choose a reason for hiding this comment

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

@biagas based on what I saw here, I think there is a small but nonetheless interesting software engineereing story to tell about how and why it was necessary to enable VisIt be built with both VTK 8 and VTK 9 for a transitional period. I think the work you needed to do to enable that, support it for the transition period and then remove it is a really good example of the kinds of critical software engineering activities are necessary in keeping up to date with dependencies. I think this would make a nice short article for bssw.io and I'd be happy to get you started with it.

Also, for the PR review, I made a number of comments and questions.

src/CMake/FindVisItVTK.cmake Outdated Show resolved Hide resolved
src/CMake/PluginVsInstall.cmake.in Show resolved Hide resolved
src/plots/Volume/QvisVolumePlotWindow.C Show resolved Hide resolved
src/plots/Volume/avtVolumeRenderer.C Show resolved Hide resolved
src/plots/Volume/avtVolumeRenderer.C Show resolved Hide resolved
src/tools/dev/xml/GenerateCMake.h Show resolved Hide resolved
src/tools/dev/xml/GenerateCMake.h Show resolved Hide resolved
We don't support 9.3 or higher currently.
Don't know if there will be a newer 9.2 release, but make allowances just in case.
@markcmiller86
Copy link
Member

@biagas there were two unresponded/unreslolved comments here. Both relate, loosely though, to previous comments. I wasn't sure if you consider those resolved too or are still considering?

@biagas
Copy link
Contributor Author

biagas commented May 29, 2024

@biagas there were two unresponded/unreslolved comments here. Both relate, loosely though, to previous comments. I wasn't sure if you consider those resolved too or are still considering?

Since they were related to previous comments, I thought we were done with them.

@biagas biagas merged commit 0dbbed8 into develop May 29, 2024
4 checks passed
@biagas biagas deleted the task/biagas/remove_vtk8_support branch May 29, 2024 19:47
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.

Remove support for VTK-8
3 participants