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.

Sign up for GitHub

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

Jump to bottom

Fix lighting issue #514

Merged
merged 2 commits into from
Oct 21, 2024
Merged

Fix lighting issue #514

merged 2 commits into from
Oct 21, 2024

Conversation

arjxn-py
Copy link
Member

light.mov

@arjxn-py arjxn-py added the bug PR that fixes a bug label Oct 21, 2024
@trungleduc
Copy link
Member

what happens if we keep using the MeshPhongMaterial?

@arjxn-py
Copy link
Member Author

what happens if we keep using the MeshPhongMaterial?

metalness & roughness properties doesn't seem to be there in MeshPhongMaterial

@martinRenou
Copy link
Member

The standard material uses PBR so it's actually good we're switching to this!

@trungleduc
Copy link
Member

what happens if we keep using the MeshPhongMaterial?

metalness & roughness properties doesn't seem to be there in MeshPhongMaterial

i know, but are they needed if we use zero decay? Could you post a screenshot of using MeshPhongMaterial?

@arjxn-py
Copy link
Member Author

what happens if we keep using the MeshPhongMaterial?

metalness & roughness properties doesn't seem to be there in MeshPhongMaterial

i know, but are they needed if we use zero decay? Could you post a screenshot of using MeshPhongMaterial?

on it

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 21, 2024 •
edited
Loading

Integration tests repot: appsharing.space

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 21, 2024 •
edited
Loading

Preview PR at appsharing.space

@arjxn-py
Copy link
Member Author

i know, but are they needed if we use zero decay? Could you post a screenshot of using MeshPhongMaterial?

phong.mov

@trungleduc

@trungleduc
Copy link
Member

Thanks, indeed it looks better with the MeshStandardMaterial

@SylvainCorlay SylvainCorlay merged commit b2187bc into jupytercad:main Oct 21, 2024
9 checks passed
martinRenou
martinRenou reviewed Oct 21, 2024
View reviewed changes
packages/base/src/3dview/mainview.tsx
@@ -227,7 +227,8 @@ export class MainView extends React.Component<IProps, IStates> {

this._scene.add(new THREE.AmbientLight(0xffffff, 0.5)); // soft white light

this._cameraLight = new THREE.PointLight(0xffffff, 1);
this._cameraLight = new THREE.PointLight(0xffffff, 100);
this._cameraLight.decay = 5;
Copy link
Member

Choose a reason for hiding this comment

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

I thought we would be using decay 0 with an intensity of 1?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh Shoot 😬 , this is my bad I think i pushed the wrong changes as I was changing them rapidly. So Sorry

Copy link
Member

Choose a reason for hiding this comment

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

No worries!

@arjxn-py arjxn-py mentioned this pull request Oct 21, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinRenou martinRenou martinRenou left review comments

@trungleduc trungleduc trungleduc approved these changes

Assignees
No one assigned
Labels
bug PR that fixes a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@arjxn-py @trungleduc @martinRenou @SylvainCorlay