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

Remove stencil buffer #2389

Merged
merged 3 commits into from Nov 3, 2022
Merged

Remove stencil buffer #2389

merged 3 commits into from Nov 3, 2022

Conversation

ffreyer
Copy link
Collaborator

@ffreyer ffreyer commented Nov 1, 2022

Description

I wanted to give fixing the scene id problem (#665) an attempt and tried removing all the stencil code bits to see what actually breaks... which seems to be nothing? Locally all tests still passed. I also tested all combinations of ssao, fxaa and transparancy with

fig = Figure()
ax = Axis(fig[1, 1])
scatter!(ax, rand(10000), color = rand(RGBf, 10000))
top = campixel(ax.blockscene)
text!(
    top, Point2f(400), text = "Testing", strokewidth=2f0, strokecolor = :white, 
    ssao = true, fxaa = !true, transparency = true
)

and get the same ordering as before.

From my understanding the stencil code restricts rendering to scene areas which is also what glViewport and glScissor do... So I guess it's redundant? Translating Axis and LScene content is still correctly cut off at least.

Fixes #665. (Well not yet but we can remove/freely type screenid if we remove stencil tests)

@MakieBot
Copy link
Collaborator

MakieBot commented Nov 1, 2022

Compile Times benchmark

Note, that these numbers may fluctuate on the CI servers, so take them with a grain of salt. All benchmark results are based on the mean time and negative percent mean faster than the base branch. Note, that GLMakie + WGLMakie run on an emulated GPU, so the runtime benchmark is much slower. Results are from running:

using_time = @ctime using Backend
# Compile time
create_time = @ctime fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @ctime Makie.colorbuffer(display(fig))
# Runtime
create_time = @benchmark fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @benchmark Makie.colorbuffer(display(fig))
using create display create display
GLMakie 39.19s (38.89, 39.33) 0.17+- 25.44s (25.05, 25.71) 0.23+- 22.98s (22.66, 23.25) 0.20+- 25.68ms (24.96, 26.80) 0.59+- 62.97ms (62.00, 63.58) 0.65+-
master 38.98s (38.81, 39.33) 0.19+- 25.47s (25.21, 25.88) 0.27+- 23.03s (22.62, 23.76) 0.42+- 25.72ms (24.84, 26.24) 0.54+- 63.40ms (62.39, 65.24) 1.01+-
evaluation +0.55%, 0.22s slower X (1.18d, 0.05p, 0.18std) -0.10%, -0.03s invariant (-0.11d, 0.85p, 0.25std) -0.22%, -0.05s invariant (-0.15d, 0.78p, 0.31std) -0.17%, -0.04ms invariant (-0.08d, 0.88p, 0.57std) -0.68%, -0.43ms invariant (-0.51d, 0.36p, 0.83std)
CairoMakie 33.62s (32.37, 35.45) 0.96+- 23.72s (22.96, 24.88) 0.71+- 3.32s (3.20, 3.42) 0.07+- 24.76ms (24.31, 25.85) 0.53+- 28.80ms (27.90, 30.61) 0.86+-
master 33.44s (32.60, 34.89) 0.77+- 23.69s (22.97, 24.49) 0.59+- 3.49s (3.27, 3.74) 0.18+- 25.23ms (24.57, 26.61) 0.69+- 29.03ms (28.04, 29.86) 0.74+-
evaluation +0.53%, 0.18s invariant (0.21d, 0.71p, 0.86std) +0.13%, 0.03s invariant (0.05d, 0.93p, 0.65std) -5.32%, -0.18s faster✅ (-1.29d, 0.04p, 0.13std) -1.89%, -0.47ms invariant (-0.76d, 0.18p, 0.61std) -0.82%, -0.24ms invariant (-0.29d, 0.59p, 0.80std)
WGLMakie 34.73s (34.25, 35.18) 0.29+- 29.72s (29.40, 30.40) 0.35+- 44.63s (43.28, 49.46) 2.18+- 24.59ms (23.46, 25.52) 0.67+- 121.92ms (117.44, 135.21) 6.30+-
master 34.66s (34.37, 35.08) 0.24+- 29.33s (28.93, 29.55) 0.21+- 44.62s (43.65, 47.46) 1.42+- 23.44ms (23.10, 24.04) 0.30+- 81.60ms (71.85, 91.57) 7.29+-
evaluation +0.20%, 0.07s invariant (0.25d, 0.65p, 0.27std) +1.30%, 0.39s slower X (1.34d, 0.03p, 0.28std) +0.01%, 0.01s invariant (0.00d, 1.00p, 1.80std) +4.67%, 1.15ms slower X (2.22d, 0.00p, 0.48std) +33.07%, 40.31ms slower❌ (5.92d, 0.00p, 6.80std)

@SimonDanisch
Copy link
Member

I remember, I had to resort to use the stencil buffer for a pretty obscure reason... There is a chance, that the reason went away, but there's also a chance, that it's only needed for a very annoying corner case.
Would be really cool to just get rid of it for good :D

@ffreyer
Copy link
Collaborator Author

ffreyer commented Nov 1, 2022

Got introduced here: JuliaPlots/GLMakie.jl#35

I'll check if I find any issues with picking. Overlapping scenes seem fine so far...

@SimonDanisch
Copy link
Member

Hm, I also tried the most common suspects and they do seem to work fine, as far as I can tell...
Jeez, really a reminder to comment odd decisions much better -.-

@ffreyer
Copy link
Collaborator Author

ffreyer commented Nov 1, 2022

This also looks the same as before and DataInspector works fine.

fig = Figure()
for i in (1:2, 2:3), j in (1:2, 2:3)
    ax = Axis(fig[i, j])
    scatter!(ax, rand(Point2f, 1000), color = rand(RGBf, 1000))
    s1 = campixel(ax.scene)
    mesh!(s1, Rect2f(100, 100, 200, 200), color = RGBAf(0.3, 0.1, 0.5, 0.3), transparency = true)
    translate!(s1, 0, 0, 100)
    s2 = campixel(ax.scene)
    mesh!(s2, Rect2f(200, 200, 200, 200), color = RGBAf(0.7, 0.3, 0.1, 0.3), transparency = !true)
    xlims!(ax, 0, 1)
    ylims!(ax, 0, 1)
end
DataInspector(fig)
fig

Didn't see any problems rendering 192 Menus generating 385 scenes either. None with Toggles or sliders either (though those don't hit > 256 scenes)

@jkrumbiegel
Copy link
Collaborator

Love that I kept repeating "it's not trivial to remove the 256 limit" and now this 😂

@ffreyer ffreyer marked this pull request as ready for review November 2, 2022 10:18
@SimonDanisch SimonDanisch merged commit bc8096b into master Nov 3, 2022
@SimonDanisch SimonDanisch deleted the ff/no-stencil branch November 3, 2022 09:50
t-bltg pushed a commit to t-bltg/Makie.jl that referenced this pull request Dec 31, 2022
* remove stencil buffer

* bump screenID to UInt16

* remove commented out code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inexact error when plotting >256 subplots
4 participants