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

Modified Arrow actor to use repeat primitive by default #552

Merged
merged 9 commits into from
Apr 13, 2022

Conversation

m-agour
Copy link
Contributor

@m-agour m-agour commented Mar 8, 2022

Modified Arrow actor to use repeat primitive by default as specified in issue #530.

Test code

import numpy as np  
from fury import window, actor  
  
scene = window.Scene()  
  
center_rs = np.array([[0, 0, 0]])  
center_rp = np.array([[0, 1, 0]])  
direction = np.array([[0, 0, 0]])  
color_rs = np.array([[1, 0, 0]])  
color_rp = np.array([[0, 1, 0]])  
  
rpactor = actor.arrow(center_rp, direction, colors=color_rp, scales=1)  
rsactor = actor.arrow(center_rs, direction, colors=color_rs, repeat_primitive=False)  
  
scene.add(rpactor)  
scene.add(rsactor)  
  
window.show(scene, size=(600, 600), reset_camera=False, title='Comparing')

image

@pep8speaks
Copy link

pep8speaks commented Mar 8, 2022

Hello @m-agour! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 51:66: W292 no newline at end of file

Comment last updated at 2022-04-08 23:33:36 UTC

@skoudoro
Copy link
Contributor

skoudoro commented Mar 8, 2022

Hi @m-agour,

Thank you for this! This is a busy month, I will review your PR ASAP. Maybe @guaje, @Garyfallidis, or other maintainers will be available to review your PR earlier.

During this time, can you make sure to add/fix:

  • pep8
  • Unit test (check the other primitives tests)
  • update or add a tutorial
  • check orientation
  • add on this PR different screenshots.

Thanks

Added scales.
Fixed prim_arrow orientation.
@m-agour
Copy link
Contributor Author

m-agour commented Mar 10, 2022

Hello @skoudoro,
Taking your advice, I've checked the orientation of the arrow actor and found two problems:

  1. By setting the direction of the repeate_source based arrow to (0, 0, 0), I found that the original direction of the arrow is (1, 0, 0). so I edited the prim_arrow to point at that direction.
  2. The second problem is that the rotation of the repeat_sources is not the same as the repeat_primitive rotation for the same arrow object. I will write about it in more detail in a separate issue.

@m-agour
Copy link
Contributor Author

m-agour commented Apr 8, 2022

Hello @skoudoro .
I've made some progress. Would you take a look at it and let me know if anything is missing?

@skoudoro
Copy link
Contributor

Thank you for this @m-agour. merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants