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

NF: Adding actor for frustum, TEST: Adding uni tests for frustum actor #963

Open
wants to merge 10 commits into
base: v2
Choose a base branch
from

Conversation

ManishReddyR
Copy link

@ManishReddyR ManishReddyR commented Jan 25, 2025

Tried to add actor and uni tests for frustum actor

@ManishReddyR ManishReddyR changed the title NF: Adding actor for frustum? NF: Adding actor for frustum Jan 25, 2025
@ManishReddyR ManishReddyR changed the title NF: Adding actor for frustum NF: Adding actor for frustum, TEST: Adding uni tests for frustum actor Jan 25, 2025
@ManishReddyR
Copy link
Author

frustum_mc frustum_sc Snapshots

@skoudoro
Copy link
Contributor

Hi @ManishReddyR,

you have some conflict that need to be fixed. I suppose you need to merge the branch v2 or rebase on branch v2.
Also, new design to reduce duplication, see #962 to update your version.

Thank you for the future update

@ManishReddyR
Copy link
Author

Hello @skoudoro
Do I need to comment out or remove the snapshots while uni testing.

Thank you

Delete fury/wgsl/mesh2.wgsl
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 @ManishReddyR,

Thank you for the update. Looks good overall, see below the last comments

colors : ndarray (N,3) or (N, 4) or tuple (3,) or tuple (4,), optional
RGB or RGBA (for opacity) R, G, B and A should be at the range [0, 1].
scales : int or ndarray (N,3) or tuple (3,), optional
The size of the box in each dimension. If a single value is provided,
Copy link
Contributor

Choose a reason for hiding this comment

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

frustum instead of box

>>> scene = window.Scene()
>>> centers = np.random.rand(5, 3) * 10
>>> colors = np.random.rand(5, 3)
>>> square_actor = actor.square(centers=centers, colors=colors)
Copy link
Contributor

Choose a reason for hiding this comment

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

frustum_actor. instead

>>> centers = np.random.rand(5, 3) * 10
>>> colors = np.random.rand(5, 3)
>>> square_actor = actor.square(centers=centers, colors=colors)
>>> scene.add(square_actor)
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

npt.assert_array_almost_equal(mean_vertex, centers[0])

assert frustum_actor.prim_count == 1
scene.remove(frustum_actor)
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, add the snapshot test since it works now.

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.

2 participants