makie.surface generates incomplete meshes

[ChenZhu-Xie]

Discourse: There’s a hole in a sphere generated by makie.surface

Unlike mesh(spheremesh) method, current surface(x,y,z) method leaves a circular hole at the north pole of the ball, see:

• https://discourse.julialang.org/t/transparent-surfaces-in-makie-jl/15775/2?u=xczphysics
• https://discourse.julialang.org/t/cairomakie-invert-normals-true-not-working/104496
• https://discourse.julialang.org/t/how-to-get-a-semi-transparent-sphere-in-makie/117944/2?u=xczphysics

and sometimes leave an 1. circular hole / 2. irregular hole composed of several black triangular faces at the south pole as well, see:

1. https://discourse.julialang.org/t/how-to-create-a-3d-transparent-cylinder-with-glmakie-or-plots-if-possible/92099?u=xczphysics
2. https://discourse.julialang.org/t/drawing-an-ellipsoid-to-visualize-a-tensor/31286/11?u=xczphysics

a MWE would be

# https://discourse.julialang.org/t/how-to-get-a-semi-transparent-sphere-in-makie/117944/2?u=xczphysics

using Colors, GeometryBasics #versions 0.10.5, 0.12.11, 0.9.20
using GLMakie
GLMakie.activate!()
# A sphere
n = 13

function gan_sphere_points(num, Θ_L, Θ_R)
    r = 1.0f0
    Θ = range(Θ_L, Θ_R, num)
    Φ = range(-π, π, num)
    x = [r * sin(θ) * cos(ϕ) for ϕ in Φ, θ in Θ]  # 这里的 ϕ,θ 顺序会影响 可否穿透球表面获取位置
    y = [r * sin(θ) * sin(ϕ) for ϕ in Φ, θ in Θ]
    z = [r * cos(θ) for ϕ in Φ, θ in Θ]

    # return [x, y, z]

    # 应用旋转变换
    rotation_angle = -π/3  # 45度
    rotation_matrix = [cos(rotation_angle) 0 -sin(rotation_angle);
                    0                  1 0;
                    sin(rotation_angle) 0 cos(rotation_angle)]

    x_rotated = rotation_matrix[1,1] .* x + rotation_matrix[1,3] .* z
    y_rotated = y
    z_rotated = rotation_matrix[3,1] .* x + rotation_matrix[3,3] .* z

    return [x_rotated, y_rotated, z_rotated]
end

sphere_points = gan_sphere_points(n, 0, π)
fig, ax, p = GLMakie.surface(sphere_points...,
    color=sphere_points[3], invert_normals=true, transparency=true)

fig

• what version of Makie are you running? (v0.21.5)
• can you reproduce the bug with a fresh environment ? (]activate --temp; add Makie)
• What platform + GPU are you on? (Windows / Nvidia)

[SimonDanisch]

This doesn't look like a whole, but rather incorrect normals

[ChenZhu-Xie]

This doesn't look like a whole, but rather incorrect normals

I didn't know much about computer graphics, so I wasn't sure what proper noun to use to describe the object, so... I chose a more vivid one :)

[asinghvi17]

For what it's worth, I also get this when using a planar to spherical transform_func. Except the poles are basically gaps then, you can see through them.

[ffreyer]

I'm pretty sure this isn't new. Maybe I'll come across another issue pointing this out in the coming days.

My guess would be that because the points fall on top of each other the shader ends up calculating normal = cross(p2 - p1, p3 - p1) = cross(0, 0) = 0

[ffreyer]

Related: #3624

Hmm, weird, I can't reproduce this with GLMakie and WGLMakie. Maybe there's some GPU dependency in this too

[asinghvi17]

I can semi-reproduce on M1...it looks subtly different, though. Can't see any of the triangle artifacts at all though.

[ffreyer]

#4735 has added more aggressive/complete nan checks which I believe fixed this. If this persists in the next release we can reopen the issue

[asinghvi17]

Unfortunately not fixed on M1 (tested on latest master, commit de99964)

[SimonDanisch]

@ffreyer I guess we'll need x != x as a nan check (or !(x <= 0.0 || x > 0.0))? I'm not sure why on earth some platforms implement isnan in GLSL, but make it so that it doesn't work....
@asinghvi17 could you try that out by replacing isnan in the shaders with something like that (dont forget GLMakie.closeall()` after updating any shader)?

[ffreyer]

Hmm, maybe I was barking at the wrong tree. The nan checks in normal gen are for nan positions, but we don't have those here

Makie.jl/GLMakie/assets/shader/surface.vert

Lines 94 to 98 in de99964

	if (!isnan(s0.z)) {
	bool check1 = isinbounds(off1, size) && !isnan(s1.x) && !isnan(s1.y) && !isnan(s1.z);
	bool check2 = isinbounds(off2, size) && !isnan(s2.x) && !isnan(s2.y) && !isnan(s2.z);
	bool check3 = isinbounds(off3, size) && !isnan(s3.x) && !isnan(s3.y) && !isnan(s3.z);
	bool check4 = isinbounds(off4, size) && !isnan(s4.x) && !isnan(s4.y) && !isnan(s4.z);

Maybe it's a div by 0 in normalize?

Makie.jl/GLMakie/assets/shader/surface.vert

Line 106 in de99964

return (invert_normals ? -1.0 : 1.0) * normalize(result);

maybe that needs something like

float n = length(result);
return n < 1e-20 ? result : (invert_normals ? -1.0 : 1.0) / n * result;

[asinghvi17]

@ffreyer your suggested fix works, now I get this:

I also tried replacing isnan with !(x <= 0.0 || x > 0.0) but that had no effect.

Should we just add that line to the surface shader, or does this also merit changes in mesh?

[ffreyer]

Yea, just add the normalize replacement then. Mesh shouldn't have anything to do with it.

For isnan, could you check if this test produces the right image? If that doesn't reproduce the isnan replacement would probably fix it

Makie.jl/ReferenceTests/src/tests/primitives.jl

Lines 638 to 691 in de99964

	@reference_test "Surface with NaN points" begin
	# This is supposed to verify a couple of things:
	# - cells with nan in positions are not drawn
	# - colors align to cell centers (via color checkerboard)
	# - all normals are valid and interpolate correctly (lighting)
	data = [x^2 + y^2 for x in range(-2, 0, length=11), y in range(-2, 0, length=11)]
	cs = reshape([(:red, :blue)[mod1(i, 2)] for i in eachindex(data)], size(data))
	f = Figure(size = (500, 1000), backgroundcolor = RGBf(0.3, 0.3, 0.3), figure_padding = (0,0,0,0))
	# Test NaN in positions
	for i in 1:3, j in 1:2
	if j == 1
	xs = collect(1.0:11.0)
	ys = collect(1.0:11.0)
	else
	xs = Float32[x for x in 1:11, y in 1:11]
	ys = Float32[y for x in 1:11, y in 1:11]
	end
	zs = copy(data)
	# shift to second row if matrix
	if i == 1
	xs[(3:6) .+ (j-1) * 44] .= NaN
	elseif i == 2
	ys[(3:6) .+ (j-1) * 44] .= NaN
	elseif i == 3
	zs[4, 3:6] .= NaN
	end
	a, p = surface(f[i, j], xs, ys, zs, color = cs, nan_color = :red, axis = (show_axis = false,))
	a.scene.lights = [AmbientLight(RGBf(0, 0, 0)), DirectionalLight(RGBf(2,2,2), Vec3f(0.5, -1, -0.8))]
	# plot a wireframe so we can see what's going on, and in which cells.
	m = Makie.surface2mesh(to_value.(p.converted)...)
	wireframe!(a, m, depth_shift = -1f-3, color = RGBf(0,0.9,0), linewidth = 1)
	end
	# Test NaN in color
	cs = copy(data)
	cs[4, 3:6] .= NaN
	for i in 1:2
	nan_color = ifelse(i == 1, :transparent, :red)
	a, p = surface(f[4, i], 1..11, 1..11, data, color = cs, colormap = [:white, :white];
	nan_color, axis = (show_axis = false,))
	a.scene.lights = [AmbientLight(RGBf(0, 0, 0)), DirectionalLight(RGBf(2,2,2), Vec3f(0.5, -1, -0.8))]
	m = Makie.surface2mesh(to_value.(p.converted)...)
	wireframe!(a, m, depth_shift = -1f-3, color = RGBf(0,0.9,0), linewidth = 1)
	end
	colgap!(f.layout, 0.0)
	rowgap!(f.layout, 0.0)
	f
	end