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

Adding week 14 blog #697

Merged
merged 6 commits into from Oct 13, 2022
Merged

Adding week 14 blog #697

merged 6 commits into from Oct 13, 2022

Conversation

xtanion
Copy link
Member

@xtanion xtanion commented Sep 28, 2022

blog is visible in blog.html

Copy link
Contributor

@m-agour m-agour left a comment

Choose a reason for hiding this comment

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

Hello @xtanion ,
Good work, and good blog post. Please see my comments below, thanks.

What did you do this week?
--------------------------

- This week, I started with Multiple actor support in skinning, Managed to do it
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be: , and managed ...


- Creating a PR for morphing code.

- Multi primitive(actor) support in morphing.
Copy link
Contributor

@m-agour m-agour Sep 29, 2022

Choose a reason for hiding this comment

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

Maybe missing a space primitive (actor)

Copy link
Contributor

@ganimtron-10 ganimtron-10 left a comment

Choose a reason for hiding this comment

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

Hello @xtanion ,
PTAL at the below comments.
Thanks!


- We merged two PRs, `#689`_ (colors from Material) and `#643`_ (simple animations).

- Added ability to load morphing information and create timelines from it. Here's a preview of the ``AnimatedMorphCube`` and ``AnimatedMorphSphere``models:
Copy link
Contributor

Choose a reason for hiding this comment

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

space between AnimatedMorphSphere and models:

--------------------------

- This week, I started with Multiple actor support in skinning, Managed to do it
successfully (see brainStem Model)
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great if you either add a link to the video to see the brainStem working in the fury or directly integrate the video here itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, I'll add it

--------------------------

- This week, I started with Multiple actor support in skinning, Managed to do it
successfully (see brainStem Model)
Copy link
Contributor

Choose a reason for hiding this comment

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

BrainStem instead

- This week, I started with Multiple actor support in skinning, Managed to do it
successfully (see brainStem Model)

- Implementing multiple animation channels support (as seen in the ``Fox`` model). The ``get_skin_timelines()`` method now returns a dictionary of all animation channels with Timeline as their value.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.
Add a link to a video or add it here so that reader would get an idea about what you are specifying.

Copy link
Member Author

Choose a reason for hiding this comment

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

Multiple animation channels are in the Fox model itself, maybe adding the link to the Fox model can help


.. raw:: html

<iframe id="player" type="text/html" width="1280" height="720" src="https://user-images.githubusercontent.com/74976752/192871376-881fbdd6-2fab-4a7f-9f4f-07663d93561c.mp4" frameborder="0"></iframe>
Copy link
Contributor

Choose a reason for hiding this comment

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

The video is overlapping the right side legends.

msedge_1Rk3a1jvB8

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 @xtanion,

Your videos are too big, we need to scroll and it is hard to understand what is going on.
You might need to record them again with a smaller resolution to fit the blog post. Otherwise, you can find a trick to scale them down.

Apart from that, all is good. Let me know when you fix the videos issue

What did you do this week?
--------------------------

- This week, I started with Multiple actor support in skinning and managed to do it
Copy link
Contributor

Choose a reason for hiding this comment

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

mutiple -> no need of upper case.

@xtanion
Copy link
Member Author

xtanion commented Oct 13, 2022

Done @skoudoro , PTAL
Thanks

@codecov
Copy link

codecov bot commented Oct 13, 2022

Codecov Report

Merging #697 (7737d3c) into master (2226dc2) will decrease coverage by 0.27%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #697      +/-   ##
==========================================
- Coverage   50.41%   50.13%   -0.28%     
==========================================
  Files         120      120              
  Lines       27160    27884     +724     
  Branches     3001     2961      -40     
==========================================
+ Hits        13693    13981     +288     
- Misses      13006    13436     +430     
- Partials      461      467       +6     
Impacted Files Coverage Δ
fury/fury/animation/interpolator.py 83.46% <0.00%> (-16.54%) ⬇️
fury/fury/tests/test_gltf.py 87.95% <0.00%> (-12.05%) ⬇️
fury/fury/gltf.py 78.72% <0.00%> (-7.69%) ⬇️
fury/fury/tests/test_actors.py 90.48% <0.00%> (-1.39%) ⬇️
fury/fury/actor.py 88.18% <0.00%> (-0.65%) ⬇️
fury/fury/utils.py 91.96% <0.00%> (-0.47%) ⬇️
fury/fury/ui/elements.py 89.66% <0.00%> (-0.37%) ⬇️
fury/fury/stream/tools.py 90.94% <0.00%> (-0.22%) ⬇️
fury/fury/ui/tests/test_elements_callback.py 96.87% <0.00%> (-0.07%) ⬇️
fury/fury/shaders/tests/test_base.py 93.04% <0.00%> (-0.04%) ⬇️
... and 24 more

@skoudoro
Copy link
Contributor

Thank you for the update. LGTM. merging

@skoudoro skoudoro merged commit e0f1613 into fury-gl:master Oct 13, 2022
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.

None yet

4 participants