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

Directional light as new default #3239

Closed
jkrumbiegel opened this issue Sep 17, 2023 · 2 comments · Fixed by #3113
Closed

Directional light as new default #3239

jkrumbiegel opened this issue Sep 17, 2023 · 2 comments · Fixed by #3113
Labels
enhancement Feature requests and enhancements

Comments

@jkrumbiegel
Copy link
Member

Right now we don't have the light type directional light at all in GLMakie and WGLMakie. This is a light like the sun, like a point light at infinite distance so that all rays are parallel. This is nice because it's easy to light any object without worrying about anything but the light angle.

Therefore, I think we should add this light type to Makie and actually make it the default. Right now, the default is a point light that is placed at eye position. This is not great I think, because lighting from eye position means there aren't any visible shadow areas, which reduces the three-dimensionality of the lit surfaces. I understand that this choice was made because it was the only simple choice available. For any other position of a point light, one would probably need to take object positions and sizes into account. Not so with a directional light. A good default could be a light coming from top right of eye position.

I already tried tinkering with this a little bit and hit #3238 in the process. The glmakie light pipeline appeared much more hardcoded to one point and one ambient light to me than I had assumed prior given the lights Vector being able to hold an arbitrary number of arbitrary light types. So this might need a larger refactor. Or we add direction-ness as a boolean Switch to PointLight which would change how the Point3f it holds is interpreted, as a position or a direction.

@jkrumbiegel jkrumbiegel added the enhancement Feature requests and enhancements label Sep 17, 2023
@SimonDanisch
Copy link
Member

I had assumed prior given the lights Vector being able to hold an arbitrary number of arbitrary light types

Yeah it's pretty much hardcoded and the light vector was introduced mostly for RPRMakie and for some future, where we have time to implement better lighting for GLMakie

@ffreyer
Copy link
Collaborator

ffreyer commented Sep 18, 2023

If we want to get directional lights working without rewriting a bunch of stuff we could just use a far away PointLight as an approximation. We'd probably need something like lightposition = lookat - 1e4 * norm(eyeposition - lookat) * normalize(direction) for that.

SimonDanisch added a commit that referenced this issue Nov 17, 2023
Continues #2831 !
Still needs to check, if I rebased correctly and didn't incorrectly
apply some of the changes!

## Merged PRs
- #2598
- #2746
- #2346
- #2544
- #3082
- #2868
- #3062
- #3106
- #3281
- #3246

## TODOS

- [x] fix flaky test `@test GLMakie.window_size(screen.glscreen) ==
scaled(screen, (W, H))`
- [x] Merge axis type inferences from #2220 
- [x] Test on different resolution screens, IJulia, Pluto, VSCode,
Windowed
- [x] rebase to only have merge commits from the PRs 
- [x] investigate unexpected speed ups
- [x] reset camera settings from tests
- [ ] check doc image generation
- [x] rethink default near/far in Camera3D (compatability with OIT)
- [x] merge #3246
- [x] fix WGLMakie issues/tests:
- [x] fix line depth issues (see tests: ~~hexbin colorrange~~ (not new),
LaTeXStrings in Axis3, Axis3 axis reversal)
  - [x] fix lighting of surface with nan points (fixed in #3246)
- ~~volume/3D contour artifacts (see 3D Contour with 2D contour
slices)~~ not new
  - ~~artifacting in "colorscale (lines)"~~ not new
- [x] GLMakie:
  - [x] slight outline in "scatter image markers" test
  - ~~clipping/z-fighting in "volume translated"~~ not new
- [x] CairoMakie:
  -  ~~Artfiacting in `colorscale (lines)"~~ not new
  - ~~markersize in "scatter rotations" changed?~~ not new
  - ~~color change in "colorscale (poly)"~~ not new
  - ~~transparency/render order of "OldAxis + Surface"~~ not new
  - ~~render order in "Merged color mesh"~~ not new
  - ~~render order of "Surface + wireframe + contour"~~ not new
- [x] Check "SpecApi in convert_arguments" (colors swapped?)


## Fixes the following errors

- fixes #2721 via #2746
- fixes #1600 via #2746
- fixes #1236 via #2746
- fixes MakieOrg/GeoMakie.jl#133 via #2598
- closes #2522
- closes #3239 via #3246
- fixes #3238 via #3246
- fixes #2985 via #3246
- fixes #3307 via #3281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests and enhancements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants