-
-
Notifications
You must be signed in to change notification settings - Fork 21.2k
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
Expose copy_effects
compute shader in Mobile backend
#85793
Conversation
6a1b18b
to
608d41a
Compare
I forgot about other checks too. Edit: deleted them now. |
I can take a look at what functions (without raster version) can be exposed if not all of them need to be exposed. Edit: At a first glance removing more checks was unnecessary, at least by looking at functions in the rendering server. |
The way I see it is that only copy_cubemap_to_panorama would be of any use. Should I leave other 2 functions as is (with a check that is)? Note: I haven't tested it on a phone yet, I need to build android template. |
No need to add the checks back. They were there to ensure we didn't accidentally try to run a shader that wasn't compiled. Since we are now initializing those shaders anyway, we don't need the checks and its better not to have them |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Tested locally to confirm that it works in the test project
The lightmapper_rd module isn't built on Android, so it won't work there either way. |
That's true however you can try to use copy_cubemap_to_panorama via RenderingServer. |
copy_effects
compute shader in Mobile backend
Thanks! |
Cherry-picked for 4.2.2. |
This PR exposes compute version (the only one currently) of copy effects allowing for using some of the RenderingServer's functions on Mobile backend eg. sky_bake_to_panorama.
Baking Lightmaps on development machine works (not considering other bugs).
Note: I encourage to test it on your phone. This is a quick change/hack, feedback necessary.
Fixes: #55868