-
Notifications
You must be signed in to change notification settings - Fork 182
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
Render a video on a Cube #322 #862
base: master
Are you sure you want to change the base?
Conversation
Hello @robinroy03, Thank you for updating! Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, Comment last updated at 2024-05-01 10:27:17 UTC |
Hi @robinroy03, Thank you for this, I will try tomorrow and give you an update |
during this time, can you fix all the pep8 issues? |
…vements, fixed the bug when video ends
@skoudoro I've fixed all the pep8 issues and have placed some of my thoughts in the code as comments. (and also in the PR description) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @robinroy03,
Thank you for this. This is a good start.
The goal was to show, a different video at each face of the cube. if you want to use the same video, they should start at different time point.
it is ok to use the texture, but you need to control the different part of your texture.
After, I will be able to look deeper at the code.
Thank you in advance for your future update.
Hi @skoudoro, Are there any inbuilt VTK functions that do this? (mapping different textures to different sides of a cube) Are there any resources for UV-Mapping textures in VTK? (I referred both VTKUsersGuide and VTKTextbook, found no examples) |
I found this import vtk
from fury import actor, window
from fury.io import load_cubemap_texture
tx = load_cubemap_texture(fnames = [
"negx.jpg",
"negy.jpg",
"negz.jpg",
"posx.jpg",
"posy.jpg",
"posz.jpg"
])
scene = window.Scene()
cubeSource = vtk.vtkCubeSource()
mapper = vtk.vtkPolyDataMapper()
mapper.SetInputConnection(cubeSource.GetOutputPort())
cube_actor = vtk.vtkActor()
cube_actor.SetTexture(tx)
cube_actor.SetMapper(mapper)
scene.add(cube_actor)
window.show(scene) Can I get some help regarding the general direction? Or should I take 6 PlaneSources and merge them? |
Please check the new commit, I've used 6 planes to make the cube. I can use Use a cubemap texture from here to test. If this is ok, I'll finish this code with some more methods such as Demo Video: FURY.0.9.0.2024-03-03.15-53-38.mp4This code is not rendering a video, but it'll be done in |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #862 +/- ##
==========================================
+ Coverage 84.43% 84.46% +0.03%
==========================================
Files 44 44
Lines 10537 10559 +22
Branches 1423 1432 +9
==========================================
+ Hits 8897 8919 +22
Misses 1266 1266
Partials 374 374
|
…use it is redundant when get_actor() alreadt exists
@skoudoro |
…mplify code, modified the tests for the coordinate change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @robinroy03,
Thank you for this. See my initial comment. My main concern is about the use of a class. I think everything can be simpler and easier to maintain with function
…ade the tutorial more descriptive, and included the changes skoudoro requested during review
I've made the changes. For |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @robinroy03,
After skimming the code , it looks much better! Please, check our other tutorials to see how it is formatted. Currently, your tutorial will not render well. You can try to render it locally.
I will try to look deeper on your code later today. Thanks for this
you need to add it in
for now, run it multiple times consecutively |
…on_cube. Updated _valid_examples for doc generation
All the said modifications are done. I also committed the new file I think this is a bug, but |
…ed inwards. This might affect if we introduce some optimizations), also some minor QOL changes. Deleted vtkprop because it was unused, it was introduced previously but now it is not needed
Hi @skoudoro, PTAL. Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @robinroy03,
Thank you for this.
6 actors is a lot. I am not sure we would like to have this in actor.py
. Maybe the function will be only in the tutorial. I will need the opinion of other members of the team.
See above some comments
…tuple optional, uint8 constraint removal, note about 6 actors
All the requested changes are made. Should I modify the function to be placed inside the tutorial? We'd discussed using 6 different actors to render the cube multiple times. |
Thanks!
I do not know yet
I know... the fact that we talk about it multiple time show that I am not completely satisfy with this solution because I can see long term issue with it. I have hard time to block time to dig in the texture actor to see if there are alternatives or something that we are missing. |
ok, I'll also keep researching whether there are better ways to do this. |
Render a video on a cube (#322)
This PR adds a new actor -
texture_on_cube
tofury/actor.py
which takes 6 different texture arguments for the different sides of a cube. It also adds the tutorial and tests for the same.How it is done
I took 6 planes and merged them to form a cube.
How is video rendered
Video is rendered by changing the texture repeatedly.
Simple static texture demo:
FURY.0.1.0.dev5378+gea0fb16.2024-03-17.15-17-32.mp4