Skip to content
This repository has been archived by the owner on Apr 28, 2021. It is now read-only.

Fix Light rendering #188

Merged
merged 1 commit into from
Jul 27, 2017
Merged

Fix Light rendering #188

merged 1 commit into from
Jul 27, 2017

Conversation

TsurHerman
Copy link
Contributor

#187
Change the Light rendering such that the light position is given in eye space as is the common practice.
Previous implementation had light position given in model space which makes it extra hard to render a few models in the same world
with some fixed lighting.

Change the Light rendering such that the light position is given in eye space as is the common practice.
Previous implementation had light position given in model space which makes it extra hard to render a few models in the same world
with some fixed lighting.
@SimonDanisch SimonDanisch merged commit c3e7a31 into JuliaGL:master Jul 27, 2017
@rleegates
Copy link

This still looks a bit off, at least on my version, any thoughts on this?

@TsurHerman
Copy link
Contributor Author

@rleegates what do you mean by a bit off?

@rleegates
Copy link

rleegates commented Aug 16, 2017

Well, it appears as though there is a constant offset between the camera and the light:


For both images, the camera is on the same side of the plane, they are simply taken at different angles. If the light were positioned at or behind the camera, both images should look similar, instead one is very dark, while the other is not. In particular it looks as if the light is positioned far above the camera. In the first image the blue area is closer to the camera, while in the second image the red area is closer to the camera.

@TsurHerman
Copy link
Contributor Author

@rleegates great example, might be something fishy in the light model ..
from the code I am almost certain that the light position is given in camera coordinates system.

you render object should have a field uniforms ,a Dict with uniform variables or signals.
what is the value of robj.uniforms[:light] ?
Especially : robj.uniforms[:light][4] which should indicate the light position in camera space?

@nlichtenberg
Copy link

If the light position was given in camera coordinates, a Vec3f0(0) position should light everything from the camera point of view.
However, I can't produce the expected results with that.

light position set to Vec3f0(0):
selection_999 043

Setting the light position to Vec3f0(0,0,-1000) yields
selection_999 044
which is strange, because a negative z value indicates that the light is somewhere in front of the camera.

Is there a flipped sign in the light model?

@SimonDanisch
Copy link
Member

I think the whole thing is messed up :D I had a first go at fixing it and will see if I can come up with some more tests! And I quite dislike that the light moves with the camera! It doesn't seem like it's what people want per default!

@TsurHerman
Copy link
Contributor Author

whethet the light is given in model space or world space or camera space is a convention .. I personally agree that light should be given in world coordinates , but I can see examples where it is more comfortable and natural to have it in either model space or camera space.

I may be wrong but it seems that the convention in old OpenGL was that it is given in Camera space.

@rleegates
Copy link

What's the status here?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants