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

Point light color is ignored in GLMakie #3238

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

Point light color is ignored in GLMakie #3238

jkrumbiegel opened this issue Sep 17, 2023 · 2 comments · Fixed by #3246 or #3113
Labels
bug GLMakie This relates to GLMakie.jl, the OpenGL backend for Makie.

Comments

@jkrumbiegel
Copy link
Member

using GLMakie
using FileIO

GLMakie.closeall()

brain = load(assetpath("brain.stl"))

f = Figure()

ls = LScene(f[1, 1]; scenekw = (; lights = Makie.AbstractLight[PointLight(Point3f(100, 100, 100), RGBf(1, 0, 0))]))

mesh!(ls, brain, color = :white)

f

Gives this:

grafik

No hint of red to be seen. I've followed the tracks a bit and think the problem is that in mesh.frag we have

void main(){
    vec4 color;
    // Should this be a mustache replace?
    if (fetch_pixel){
        color = get_pattern_color(image);
    }else{
        color = get_color(image, o_uv, color_norm, color_map, matcap);
    }
    {{light_calc}}
    write2framebuffer(color, o_id);
}

and in surface.jl we have

function light_calc(x::Bool)
    if x
        """
        vec3 L      = normalize(o_lightdir);
        vec3 N      = normalize(o_normal);
        vec3 light1 = blinnphong(N, o_camdir, L, color.rgb);
        vec3 light2 = blinnphong(N, o_camdir, -L, color.rgb);
        color       = vec4(ambient * color.rgb + light1 + backlight * light2, color.a);
        """
    else
        ""
    end
end

And then we have

vec3 blinnphong(vec3 N, vec3 V, vec3 L, vec3 color){
    float diff_coeff = max(dot(L, N), 0.0);

    // specular coefficient
    vec3 H = normalize(L + V);

    float spec_coeff = pow(max(dot(H, N), 0.0), shininess);
    if (diff_coeff <= 0.0 || isnan(spec_coeff))
        spec_coeff = 0.0;

    // final lighting model
    return vec3(
        diffuse * diff_coeff * color +
        specular * spec_coeff
    );
}

So color in blinnphong which to me should be the light color seems to be the texture color instead unless I'm misunderstanding.

@ffreyer
Copy link
Collaborator

ffreyer commented Sep 17, 2023

No, the color here should be the objects color. Ambient, diffuse and specular used to act as a combined material property and light color. For more realistic/user friendly lighting we should probably split of the light color into another vec3 multiplier. Probably just light_color * blinnphong(...);?

@jkrumbiegel
Copy link
Member Author

Yeah at least right now light color doesn't seem to have an effect at all. It was a bit difficult for me to follow the variables because of the glsl structure

@t-bltg t-bltg added the GLMakie This relates to GLMakie.jl, the OpenGL backend for Makie. label Oct 6, 2023
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
bug GLMakie This relates to GLMakie.jl, the OpenGL backend for Makie.
Projects
None yet
3 participants