Skip to content

NF: Adding point actor and all its requirements.#968

Merged
maharshi-gor merged 7 commits intofury-gl:v2from
ManishReddyR:Adding-PoinrActor
Apr 8, 2025
Merged

NF: Adding point actor and all its requirements.#968
maharshi-gor merged 7 commits intofury-gl:v2from
ManishReddyR:Adding-PoinrActor

Conversation

@ManishReddyR
Copy link
Copy Markdown
Contributor

Hello.
Added the point actor with required geometry, material and test cases.

point_nc

point_normal

Thank you

@ManishReddyR
Copy link
Copy Markdown
Contributor Author

Hello,
Modified point actor and also added the maker actor. Here is the screenshot of the marker actor.

Screenshot 2025-03-15 at 9 13 19 PM

Thanks

Comment thread fury/geometry.py Outdated
Comment thread fury/material.py Outdated
Comment thread fury/tests/test_actor.py Outdated
Comment thread fury/actor.py
Comment thread fury/actor.py
Comment thread fury/geometry.py
Comment thread fury/actor.py
Copy link
Copy Markdown
Contributor

@maharshi-gor maharshi-gor left a comment

Choose a reason for hiding this comment

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

I have left a lot of comments on this

image

Also @ManishReddyR why do we see the borders changing its colors while moving the scroll wheel

@ManishReddyR
Copy link
Copy Markdown
Contributor Author

I have left a lot of comments on this

image

Also @ManishReddyR why do we see the borders changing its colors while moving the scroll wheel

Its due to the over lapping of multiple actors at a time.

Copy link
Copy Markdown
Contributor

@maharshi-gor maharshi-gor left a comment

Choose a reason for hiding this comment

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

The PR looks good but there are some docstring changes required and some methods needs refractoring.

Comment thread fury/actor.py Outdated
Comment thread fury/actor.py Outdated
Comment thread fury/actor.py Outdated
Comment thread fury/actor.py Outdated
Comment thread fury/actor.py Outdated
Comment thread fury/tests/test_actor.py Outdated
Comment thread fury/tests/test_actor.py Outdated
Comment thread fury/tests/test_actor.py Outdated
Comment thread fury/material.py Outdated
Comment thread fury/material.py Outdated
@ManishReddyR
Copy link
Copy Markdown
Contributor Author

Thanks for review @maharshi-gor

I made the changes which you have suggested.

Copy link
Copy Markdown
Contributor

@maharshi-gor maharshi-gor left a comment

Choose a reason for hiding this comment

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

Just fix one small comment on the marker actor method parameters.

Comment thread fury/actor.py Outdated
@maharshi-gor
Copy link
Copy Markdown
Contributor

@ManishReddyR Thank you for the work. LGTM! Merging.

@maharshi-gor maharshi-gor merged commit a431a72 into fury-gl:v2 Apr 8, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants