Skip to content
This repository has been archived by the owner on Jul 14, 2021. It is now read-only.

Postprocessing refactor #107

Merged
merged 14 commits into from
Feb 2, 2021
Merged

Conversation

ffreyer
Copy link
Collaborator

@ffreyer ffreyer commented Jun 4, 2020

Here's a draft for MakieOrg/Makie.jl#604.

I moved most of the postprocessing stuff into a new file and wrapped it up in a PostProcessor object. Beyond ssao, fxaa and copying I also added an "empty" postprocessor, which doesn't do anything when executed. Depending on some global variables enable_SSAO and enable_FXAA either the empty or the ssao/fxaa postprocessor ends up in a list in Screen. The renderloop then executes postprocessors from that list, in order.

So if for example screen.postprocessors[1] is an empty postprocessor, the renderloop will do nothing instead of the SSAO stuff. This is the case when enable_SSAO[] = false. I also added a bit to the GLVisualizeShader so that the position and normal buffers are removed from fragment_ouput when enable_SSAO = false.

@SimonDanisch
Copy link
Member

Nice :) I will need a bit of time to review this ;)

@ffreyer
Copy link
Collaborator Author

ffreyer commented Jun 9, 2020

I might make some more changes this weekend. I noticed, for example, that render_frame still attaches all 4 draw buffers even if only two are active/exist.

@@ -45,7 +45,7 @@ function get_texture!(atlas)
end

# TODO
const enable_SSAO = Ref(true)
const enable_SSAO = Ref(false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, with all the issue, it seems to be a good idea to only turn it on when actually used!

@SimonDanisch
Copy link
Member

Let me know once this is ready :)

@ffreyer
Copy link
Collaborator Author

ffreyer commented Aug 28, 2020

I think I'm done with this.

It would be cool to turn on/off ssao (and maybe fxaa) on the fly in the future, i.e. to cause a shader recompilation when the scene demands it. That would also be nice to control shading and I think it would be needed for depth peeling. But I didn't want to do this here.

@ffreyer ffreyer marked this pull request as draft October 14, 2020 09:40
@ffreyer ffreyer marked this pull request as ready for review January 30, 2021 14:30
@ffreyer
Copy link
Collaborator Author

ffreyer commented Jan 30, 2021

AbstractPlotting tests pass locally for me, on current AbstractPlotting#master (JuliaPlots/AbstractPlotting.jl@95d3375)

@ffreyer
Copy link
Collaborator Author

ffreyer commented Jan 30, 2021

]test GLMakie fails locally on idx = N - 1 though it passes here and if I run it manually in a terminal. So I don't think it's an issue.

Should be ready to merge

otherwise the background may get darkened
@pbouffard
Copy link

Just gave it a try, works great. Thank you!!

@SimonDanisch SimonDanisch merged commit 6b0e392 into JuliaPlots:master Feb 2, 2021
@SimonDanisch
Copy link
Member

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants