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

Update shader compilation to GLSL 1.50 #10

Closed
wants to merge 1 commit into from

Conversation

Doridian
Copy link

@Doridian Doridian commented Sep 8, 2024

This is a partial fix for #8

The main change is enforcing the minimum GLSL version to be 150 (the --qt6 flag defaults to...quite a fair bit lower versions).
The motivator behind this is that it seems lots of the advanced shaders (and also as seen in the included footer/header in this repo), use texelFetch

Full support for this has only been added in 150: https://docs.gl/sl4/texelFetch

As no machine running Plasma6 should be limited to versions of GLSL this old, I think it is fair to apply this? Ultimately the judgement is up to you of course :)

Also requires a slight patch to the shaders to remove #version. I wonder if this should just be automated (remove all #version statements from shader, add one at the top that is the highest version requested? Fail if we detect a difference?)

This does make a lot of the other shaders "just work". See one of the examples:
image

@Doridian Doridian changed the title Update shaders and shader compilation Update shader compilation to GLSL 1.50 Sep 8, 2024
@flafflar
Copy link
Owner

flafflar commented Sep 9, 2024

Hi @Doridian, thanks for the pull request!

I'm currently writing from a laptop that only supports GLSL 1.20, so I think that the defaults that Qt selected are pretty reasonable. I know this is probably an edge case, but it would be a pity if I couldn't run panon on my own computer :')

I think the best solution here is to rework the shader system from scratch. The qsb tool is supposed to be used ahead-of-time, and using it during runtime was a temporary hack I made to make the widget working. If shaders are distributed as qsb files, which is what Qt intends apparently, then each individual shader can set the minimum version it supports when being built. I would really like to hear your opinion on this idea :)

@Doridian
Copy link
Author

Doridian commented Sep 9, 2024

@flafflar I am surprised a machine that can only run GLSL 1.20 is capable of running Plasma 6 with panon at all (but then again I also run Plasma 6 on my ThinkPad T420s, so maybe shouldn't be too surprised?).
My thought process when I did this was mostly "Panon is a flashy visual thing, what are the chances anyone runs this unless they got GPU power to spare". Well, as usual, I got proven wrong straight away x3

Yeah, I think pre-QSBing shaders is a great option. Saves time and then the dependency on QSB could be made optional/removed.

I'll see tomorrow (unless you've been struck by inspiration and can't wait to implement this yourself, of course) about doing some research into what the best way might be to implement this, but I can't imagine it should be too hard

@flafflar
Copy link
Owner

flafflar commented Sep 9, 2024

I'll see tomorrow (unless you've been struck by inspiration and can't wait to implement this yourself, of course) about doing some research into what the best way might be to implement this, but I can't imagine it should be too hard

Sure, I've already started some work on the qsb branch, you can contribute there if you want. If you have another way of doing this, I'm open to suggestions :)

@Doridian
Copy link
Author

Doridian commented Sep 9, 2024

I'll see tomorrow (unless you've been struck by inspiration and can't wait to implement this yourself, of course) about doing some research into what the best way might be to implement this, but I can't imagine it should be too hard

Sure, I've already started some work on the qsb branch, you can contribute there if you want. If you have another way of doing this, I'm open to suggestions :)

Collaboration on the same feature is always a little difficult but yeah that should work just fine, given it shouldn't be that big of a feature :3

Also thanks for getting started on the part I know the least about (been a couple years since I touched CMakeFiles in any notable way)

@flafflar
Copy link
Owner

Are you kidding me? You're the one helping me, I should be thanking you :3:3
We'll figure it out about the collaboration, I don't worry a lot

@Doridian Doridian closed this Sep 11, 2024
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.

2 participants