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

Fixed the rotation matrix in repeat_primitive. #555

Merged
merged 19 commits into from
Apr 13, 2022

Conversation

m-agour
Copy link
Contributor

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

Fixed the issue of the rotation matrix that I mentioned in issue #554.
What I did was assume all primitives have a normal vector pointing in X-axis (1, 0, 0), then the matrix to align this normal vector of the primitives to the direction vector specified.
I tested the matrix on my arrow using repeat_primitive and repeat_source and got a match.

import numpy as np
from fury import window, actor

scene = window.Scene()

centers = np.zeros([3, 3])
dirs = np.identity(3)
colors = np.identity(3)
scales = np.array([2.1, 2.6, 2.5])

rpactor = actor.arrow(centers, dirs, colors=colors, scales=1.5)
rsactor = actor.arrow(centers, dirs, colors=colors, repeat_primitive=False)

cen2 = np.random.rand(5, 3)
dir2 = np.random.rand(5, 3)
cols2 = np.random.rand(5, 3)

rpactor2 = actor.arrow(cen2, dir2, colors=cols2, scales=1.5)
rsactor2 = actor.arrow(cen2, dir2, colors=cols2, repeat_primitive=False)

scene.add(rpactor)
scene.add(rpactor2)
scene.add(rsactor)
scene.add(rsactor2)

window.show(scene, size=(600, 600), reset_camera=False, title='Comparing')

image

As shown in the figure above, they align perfectly now.

Copy link
Contributor

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Hi @m-agour,

Apologize for the late answer. Good catch and thank you for this!

Can you update the tests in fury/test/test_primitive.py

Thank you

@m-agour
Copy link
Contributor Author

m-agour commented Mar 30, 2022

Hello @skoudoro ,
Ok, I will update the tests in test_primitive.py

@skoudoro
Copy link
Contributor

skoudoro commented Apr 4, 2022

Before going further in your test, can you merge #551 in your PR (or rebase).

Both are linked. thank you @m-agour

@m-agour
Copy link
Contributor Author

m-agour commented Apr 7, 2022

Hello @skoudoro .
Sorry for the late reply, I just finished my midterms and will get right into it.

@skoudoro
Copy link
Contributor

Thank you @m-agour for adding the tests, merging

@skoudoro skoudoro merged commit 9d88b2d into fury-gl:master Apr 13, 2022
@skoudoro skoudoro added the type:Bug Fix Something isn't working label Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Bug Fix Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants