Skip to content

Commit

Permalink
Fix transforming surface normals (#3722)
Browse files Browse the repository at this point in the history
* Fix transforming surface normals

Fixes #3702

* Fix missing rotation in `normalmatrix`

See #3722 (comment)

* Add reference test "Ellipsoid marker sizes" in ReferenceTests/src/tests/examples3d.jl

* Add a record in CHANGELOG.md

* change 1:3 to Vec(1,2,3)

* tweak changelog entry

---------

Co-authored-by: ffreyer <frederic481994@hotmail.de>
  • Loading branch information
singularitti and ffreyer authored Mar 21, 2024
1 parent a679147 commit d50dad0
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- Fixed `contourf` bug where n levels would sometimes miss the uppermost value, causing gaps [#3713](https://github.com/MakieOrg/Makie.jl/pull/3713).
- Added `scale` attribute to `violin` [#3352](https://github.com/MakieOrg/Makie.jl/pull/3352).
- Use label formatter in barplot [#3718](https://github.com/MakieOrg/Makie.jl/pull/3718).
- Fix the incorrect shading with non uniform markerscale in meshscatter [#3722](https://github.com/MakieOrg/Makie.jl/pull/3722)

## [0.20.8] - 2024-02-22

Expand Down
5 changes: 3 additions & 2 deletions CairoMakie/src/primitives.jl
Original file line number Diff line number Diff line change
Expand Up @@ -931,10 +931,11 @@ function draw_mesh3D(
ctx = screen.context
projectionview = Makie.space_to_clip(scene.camera, space, true)
eyeposition = scene.camera.eyeposition[]
i = Vec(1, 2, 3)
normalmatrix = transpose(inv(model[i, i]))

i = Vec(1, 2, 3)
local_model = rotation * Makie.scalematrix(Vec3f(scale))
normalmatrix = transpose(inv(model[i, i] * local_model[i, i])) # see issue #3702

# pass transform_func as argument to function, so that we get a function barrier
# and have `transform_func` be fully typed inside closure
vs = broadcast(meshpoints, (transform_func,)) do v, f
Expand Down
2 changes: 1 addition & 1 deletion GLMakie/assets/shader/particles.vert
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ void main(){
o_id = uvec2(objectid, index+1);
vec3 s = _scale(scale, index);
vec3 V = vertices * s;
vec3 N = normals;
vec3 N = normals / s; // see issue #3702
vec3 pos;
{{position_calc}}
o_color = get_particle_color(color, intensity, color_map, color_norm, index, len);
Expand Down
7 changes: 7 additions & 0 deletions ReferenceTests/src/tests/examples3d.jl
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,13 @@ end
scatter(RNG.rand(20), RNG.rand(20), markersize=RNG.rand(20) .* 20, color=colors)
end

@reference_test "Ellipsoid marker sizes" begin # see PR #3722
pts = Point3f[[0, 0, 0], [1, 0, 0]]
markersize = Vec3f[[0.5, 0.2, 0.5], [0.5, 0.2, 0.5]]
rotations = [qrotation(Vec3f(1, 0, 0), 0), qrotation(Vec3f(1, 1, 0), π / 4)]
meshscatter(pts; markersize, rotations, color=:white, diffuse=Vec3f(-2, 0, 4), specular=Vec3f(4, 0, -2))
end

@reference_test "Record Video" begin
f(t, v, s) = (sin(v + t) * s, cos(v + t) * s, (cos(v + t) + sin(v)) * s)
t = Observable(0.0) # create a life signal
Expand Down
2 changes: 1 addition & 1 deletion WGLMakie/assets/particles.vert
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ void main(){
// get_* gets the global inputs (uniform, sampler, position array)
// those functions will get inserted by the shader creation pipeline
vec3 vertex_position = get_markersize() * to_vec3(get_position());
vec3 N = get_normals();
vec3 N = get_normals() / get_markersize(); // see issue #3702
rotate(get_rotations(), vertex_position, N);
vertex_position = to_vec3(get_offset()) + vertex_position;
vec4 position_world = model * vec4(vertex_position, 1);
Expand Down

0 comments on commit d50dad0

Please sign in to comment.