search

fury-gl / fury

FURY - Free Unified Rendering in pYthon.
https://fury.gl
Other
241 stars 181 forks source link

Continuing glTF/PBR Implementation #854

Open m-agour opened 9 months ago

m-agour commented 9 months ago

This PR is based on the previous work in PR #715 . Trying to improve the PBR-based glTF models rendering in FURY.

It's been some time since my last contribution, and I'm excited to jump back in. Currently, the PBR materials are working but without Environmental lighting and reflection as in the screenshot below:

image

I propose merging this to establish a solid ground from which we move on with more advanced lighting for the PBR glTF models.

pep8speaks commented 9 months ago

Hello @m-agour, Thank you for updating!

Line 279:80: E501 line too long (84 > 79 characters) Line 439:80: E501 line too long (84 > 79 characters) Line 459:80: E501 line too long (80 > 79 characters) Line 465:80: E501 line too long (84 > 79 characters)

Line 188:80: E501 line too long (84 > 79 characters) Line 189:80: E501 line too long (84 > 79 characters) Line 190:80: E501 line too long (88 > 79 characters)

To test for issues locally, pip install flake8 and then run flake8 fury.

Comment last updated at 2024-06-11 19:32:34 UTC
m-agour commented 9 months ago

So, Tests are failing even though objects are being rendered, I suspect its because I modified the lightening so when it tests for a specific color it might not be there anymore. i.e. this duck produced by the test code triggers an error even though it seems fine. image

skoudoro commented 9 months ago

Thank you for this @m-agour.

I will be able to look into this only after December 27. I will keep you updated.

Thanks again!

m-agour commented 9 months ago

Hello @skoudoro, please take your time. I'll keep enhancing this it and troubleshooting the tests meanwhile. Happy holidays!

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 54.32099% with 37 lines in your changes missing coverage. Please review.

Project coverage is 84.08%. Comparing base (b38afc1) to head (d326f75).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/fury-gl/fury/pull/854/graphs/tree.svg?width=650&height=150&src=pr&token=wrshJ6dyDs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fury-gl)](https://app.codecov.io/gh/fury-gl/fury/pull/854?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fury-gl) ```diff @@ Coverage Diff @@ ## master #854 +/- ## ========================================== - Coverage 84.37% 84.08% -0.29% ========================================== Files 44 44 Lines 10553 10620 +67 Branches 1432 1445 +13 ========================================== + Hits 8904 8930 +26 - Misses 1270 1302 +32 - Partials 379 388 +9 ``` | [Files](https://app.codecov.io/gh/fury-gl/fury/pull/854?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fury-gl) | Coverage Δ | | |---|---|---| | [fury/gltf.py](https://app.codecov.io/gh/fury-gl/fury/pull/854?src=pr&el=tree&filepath=fury%2Fgltf.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fury-gl#diff-ZnVyeS9nbHRmLnB5) | `76.08% <54.32%> (-3.50%)` | :arrow_down: |
skoudoro commented 3 months ago

Hi @m-agour,

Can you fix the 2 CI's "Codespell" and "Code format" before starting the review. Maybe a rebase of this PR will be enought to fix it, I did not check. Thank you in advance

m-agour commented 3 months ago

Hi @skoudoro , I will rebase today and ping you after all tests pass