Add support for macOS (and other GPUs without hardware ray tracing support) #46

expenses · 2022-02-03T14:57:39Z

Checklist

I have read the Contributor Guide
I have read and agree to the Code of Conduct
I have added a description of my changes and why I'd like them included in the section below

Description of Changes

This contains the changes in my branch mentioned here: #37.

I've marked it as a draft because there are a few things to sort out and because I haven't tested it on a GPU with hardware ray tracing yet.

Related Issues

#37

crates/lib/kajiya-backend/src/vulkan/device.rs

crates/lib/kajiya-backend/src/vulkan/shader.rs

crates/lib/kajiya/src/renderers/csgi.rs

crates/lib/kajiya/src/world_renderer.rs

h3r2tic

This is pretty cool! ❤️ I thought it would require way more surgery to get rid of ray-tracing 😅 I've tested the branch, and it works in both modes in my 6800XT! 🥳

Besides the individual comments on the code, my higher-level thoughts on this are that we should probably dynamically check for whether ray tracing should be enabled. It's my fault really for suggesting this approach with the ray-tracing feature. It makes some sense for a purely developer-facing thing, but if anyone does ship a binary made with kajiya, they shouldn't need to ship a variant compiled with and without ray tracing.

Now, we don't strictly need to do that in this PR, as this is a pretty cool first version, but the current checks for the ray-tracing feature are a bit scattered, and apply to things which aren't purely about ray tracing -- that does need a bit of cleanup.

crates/lib/kajiya-backend/src/vulkan/device.rs

h3r2tic · 2022-02-04T05:58:21Z

crates/lib/kajiya-backend/src/vulkan/image.rs

+        let limits = self.pdevice.properties.limits;
+
+        let max_dimension = if desc.extent[2] > 1 {
+            limits
+                .max_image_dimension1_d
+                .min(limits.max_image_dimension2_d)
+                .min(limits.max_image_dimension3_d)
+        } else if desc.extent[1] > 1 {
+            limits
+                .max_image_dimension1_d
+                .min(limits.max_image_dimension2_d)
+        } else {
+            limits.max_image_dimension1_d
+        };
+
+        for i in 0..3 {
+            let dimension = desc.extent[i];
+
+            if dimension > max_dimension {
+                log::warn!(
+                    "Dimension {} ({}) exceeds max dimension {}. Adjusting to the max. {:?}",
+                    i,
+                    dimension,
+                    max_dimension,
+                    desc
+                );
+                desc.extent[i] = max_dimension;
+            }
+        }


I don't think this should be here 😅 The image creation function cannot just go and create something else, as it will most likely break things up the chain, where code depends on the specific sizes of images. Best case some rendering is glitchy; worst case, we get infinite loops and driver crashes.

Where does this trigger for you? It should probably be higher-level logic that deals with those limits 👀

Ah, just checked and the problematic volume texture is coming from CSGI because the indirect cascade texture is

[ VOLUME_DIMS * TOTAL_SUBRAY_COUNT as u32), VOLUME_DIMS, VOLUME_DIMS, ]

or [3456, 64, 64].

Can this be 1x1x1 as well?

Yup, I believe it can!

crates/lib/kajiya-backend/src/vulkan/shader.rs

h3r2tic · 2022-02-04T06:08:24Z

crates/lib/kajiya/src/renderers/csgi.rs

@@ -261,6 +261,7 @@ impl CsgiRenderer {
        //
        // Either is going to leave the GPU quite under-utilized though, and would be a nice fit for async compute.

+        #[cfg(feature = "ray-tracing")]


This is a bit messy; ideally the entire csgi renderer should be unused when ray-tracing is disabled. I suspect you're only after the resources that it creates -- and I also suspect it's those resources that are too large on your macbook, so that you ended up clamping texture size.

We can side-step all that by not disabling the whole renderer, and only creating placeholder resources for where it needs to be passed in -- something like a black 1x1x1 volume.

…n check

h3r2tic

Good cleanup! Found one gotcha, but otherwise works nicely 😀

A few nitpicks too 👀

crates/lib/kajiya-backend/src/vulkan/device.rs

h3r2tic · 2022-02-05T14:39:49Z

crates/lib/kajiya-backend/src/vulkan/image.rs

+        let limits = self.pdevice.properties.limits;
+
+        let max_dimension = if desc.extent[2] > 1 {
+            limits
+                .max_image_dimension1_d
+                .min(limits.max_image_dimension2_d)
+                .min(limits.max_image_dimension3_d)
+        } else if desc.extent[1] > 1 {
+            limits
+                .max_image_dimension1_d
+                .min(limits.max_image_dimension2_d)
+        } else {
+            limits.max_image_dimension1_d
+        };
+
+        for i in 0..3 {
+            let dimension = desc.extent[i];
+
+            if dimension > max_dimension {
+                log::warn!(
+                    "Dimension {} ({}) exceeds max dimension {}. Adjusting to the max. {:?}",
+                    i,
+                    dimension,
+                    max_dimension,
+                    desc
+                );
+                desc.extent[i] = max_dimension;
+            }
+        }


crates/lib/kajiya-backend/src/vulkan/device.rs

crates/lib/kajiya/src/renderers/rtdgi.rs

h3r2tic · 2022-02-05T15:08:23Z

crates/lib/kajiya/src/renderers/rtr.rs

+        let mut resolved_tex = rg.create(
+            gbuffer_depth
+                .gbuffer
+                .desc()
+                .usage(vk::ImageUsageFlags::empty())
+                .format(vk::Format::R16G16B16A16_SFLOAT),
+        );
+
+        let (temporal_output_tex, history_tex) = self
+            .temporal_tex
+            .get_output_and_history(rg, Self::temporal_tex_desc(gbuffer_desc.extent_2d()));
+
+        let (mut ray_len_output_tex, ray_len_history_tex) =
+            self.ray_len_tex.get_output_and_history(
+                rg,
+                ImageDesc::new_2d(vk::Format::R16G16_SFLOAT, gbuffer_desc.extent_2d())
+                    .usage(vk::ImageUsageFlags::SAMPLED | vk::ImageUsageFlags::STORAGE),
+            );


Technically those could also be R8G8B8A8_UNORM, but it would also be possible to plug in SSR here, so not a big deal.

crates/lib/kajiya/src/world_renderer.rs

crates/lib/kajiya-backend/src/vulkan/device.rs

h3r2tic

Sorry for the delay, I was too deep in the trenches of digital color management for a bit 🙈

This looks great, and works great! ❤️ Tested on AMD and NV, also the DLSS branch, and everything runs fine. Nice work 😀

virtualritz · 2022-05-21T15:13:07Z

Shouldn't the README be updated re. supported/required GPUs to run this?

h3r2tic · 2022-05-21T16:32:58Z

Yup, will be there in the upcoming GI branch: #57

expenses and others added 11 commits January 1, 2022 17:54

It works.. I guess

60f78a6

Getting a device timeout

5bc2a18

Adjust script

dccf1a4

Switch to universal libdxcompiler.dynlib

6599d2e

Merge remote-tracking branch 'embark/main' into rtx-off

b63ad2b

Comment out rust post combine shader

e02ce93

Use a while loop in the rust post combine shader instead of a for loop

08c9fb7

Switch back to the rust shader for post combine

83504f8

Check updated rust shaders into this branch

0ceb4fe

Merge remote-tracking branch 'origin/main' into rtx-off

5abe8ed

Change commented out lines into cfg statements

2f2f04b

expenses commented Feb 3, 2022

View reviewed changes

crates/lib/kajiya-backend/src/vulkan/device.rs Outdated Show resolved Hide resolved

expenses commented Feb 3, 2022

View reviewed changes

crates/lib/kajiya-backend/src/vulkan/shader.rs Outdated Show resolved Hide resolved

expenses commented Feb 3, 2022

View reviewed changes

crates/lib/kajiya/src/renderers/csgi.rs Outdated Show resolved Hide resolved

expenses commented Feb 3, 2022

View reviewed changes

crates/lib/kajiya/src/world_renderer.rs Outdated Show resolved Hide resolved

Run cargo fmt

a9e1893

expenses mentioned this pull request Feb 3, 2022

Recompile libdxcompiler.so to remove depencency on libtinfo.so #47

Closed

h3r2tic requested changes Feb 4, 2022

View reviewed changes

Remove the ray-tracing feature and replace it with a runtime extensio…

e84ecff

…n check

h3r2tic requested changes Feb 5, 2022

View reviewed changes

Ashley Ruglys added 3 commits February 5, 2022 16:57

Apply suggestions

0763b4b

Merge remote-tracking branch 'origin/main' into rtx-off-polished

781060a

Apply suggestions, should be good to merge as a MVP afaik

40a97b0

expenses requested a review from h3r2tic February 10, 2022 14:25

expenses marked this pull request as ready for review February 11, 2022 17:31

Make clippy happy

6a776a7

h3r2tic approved these changes Feb 27, 2022

View reviewed changes

h3r2tic merged commit 79b7a74 into EmbarkStudios:main Feb 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for macOS (and other GPUs without hardware ray tracing support) #46

Add support for macOS (and other GPUs without hardware ray tracing support) #46

expenses commented Feb 3, 2022

h3r2tic left a comment

h3r2tic Feb 4, 2022

h3r2tic Feb 5, 2022

expenses Feb 5, 2022

h3r2tic Feb 5, 2022

h3r2tic Feb 4, 2022

h3r2tic left a comment

h3r2tic Feb 5, 2022

h3r2tic Feb 5, 2022

h3r2tic left a comment

virtualritz commented May 21, 2022

h3r2tic commented May 21, 2022

Add support for macOS (and other GPUs without hardware ray tracing support) #46

Add support for macOS (and other GPUs without hardware ray tracing support) #46

Conversation

expenses commented Feb 3, 2022

Checklist

Description of Changes

Related Issues

h3r2tic left a comment

Choose a reason for hiding this comment

h3r2tic Feb 4, 2022

Choose a reason for hiding this comment

h3r2tic Feb 5, 2022

Choose a reason for hiding this comment

expenses Feb 5, 2022

Choose a reason for hiding this comment

h3r2tic Feb 5, 2022

Choose a reason for hiding this comment

h3r2tic Feb 4, 2022

Choose a reason for hiding this comment

h3r2tic left a comment

Choose a reason for hiding this comment

h3r2tic Feb 5, 2022

Choose a reason for hiding this comment

h3r2tic Feb 5, 2022

Choose a reason for hiding this comment

h3r2tic left a comment

Choose a reason for hiding this comment

virtualritz commented May 21, 2022

h3r2tic commented May 21, 2022