Skip to content

Shareable graphics resources#2887

Merged
sankhesh merged 26 commits intoKitware:masterfrom
sankhesh:shareable_graphics_resources
Oct 14, 2023
Merged

Shareable graphics resources#2887
sankhesh merged 26 commits intoKitware:masterfrom
sankhesh:shareable_graphics_resources

Conversation

@sankhesh
Copy link
Collaborator

@sankhesh sankhesh commented Aug 8, 2023

Context

Add ability to cache graphics resources at the context level and have mappers re-use them if they are looking at the same data in the same render window.

Results

This ability opens up graphics resource sharing across viewports in applications where the same dataset is being rendered by different mappers. An example of this is provided with a QuadView that has three image reslice views and a volume view, all looking at the same 3D vtkImageData. Without this change, each mapper would be uploading the whole dataset to the GPU.

Changes

  • Documentation and TypeScript definitions were updated to match those changes

PR and Code Checklist

  • semantic-release commit messages
  • Run npm run reformat to have correctly formatted code

Testing

  • This change adds or fixes unit tests
  • Tested environment:
    • vtk.js:
    • OS:
    • Browser:

@sankhesh sankhesh requested review from floryst and jadh4v August 8, 2023 13:50
Copy link
Contributor

@floryst floryst left a comment

Choose a reason for hiding this comment

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

Exciting work! We will need to update the types for certain classes (e.g. ImageProperty, VolumeProperty, IStatistics, etc.).

@sankhesh sankhesh force-pushed the shareable_graphics_resources branch from f4686a5 to 2f9100b Compare August 8, 2023 22:31
Copy link
Collaborator

@jadh4v jadh4v left a comment

Choose a reason for hiding this comment

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

Some subjective comments, but overall looks good to me. Really cool work!

@bruyeret
Copy link
Contributor

Very good work, I hope it will be merged soon!
Do you think it would be possible to factorize the code in the mappers?
I am talking about the modifications to buildBufferObjects in the VolumeMapper and ImageResliceMapper

@sankhesh sankhesh force-pushed the shareable_graphics_resources branch from 0d6e860 to 04147be Compare October 3, 2023 16:56
This allows re-using and sharing across viewports/mappers.
Demonstrate sharing of scalar, color and opacity textures.
Demonstrate allocated GPU memory counting of textures.
…esources

After detaching the program and before setting it to null, the program must be deleted from the
context. This change also ensures that the shaders attached to the program are detached and deleted
at cleanup time.
This change allows consumer applications to release any graphics memory
allocated by vtk-js while keeping the context alive. This is an
idempotent invocation that allows existing props/mappers to re-allocate
resources in the next render call.
Allow re-allocation when the shaders, textures, etc. are freed at the context level.
@sankhesh sankhesh force-pushed the shareable_graphics_resources branch from 706cabe to f256fb5 Compare October 11, 2023 21:30
@sankhesh sankhesh requested a review from floryst October 13, 2023 14:56
Copy link
Contributor

@floryst floryst left a comment

Choose a reason for hiding this comment

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

LGTM

@sankhesh sankhesh added this pull request to the merge queue Oct 14, 2023
Merged via the queue into Kitware:master with commit 5e6191a Oct 14, 2023
@sankhesh sankhesh deleted the shareable_graphics_resources branch October 17, 2023 17:21
@github-actions
Copy link

🎉 This PR is included in version 28.13.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot added the released Automated label label Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released Automated label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants