Skip to content

Sys bindings update #22

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

Open
wants to merge 65 commits into
base: wgpu-example-viewer
Choose a base branch
from

Conversation

matthiascy
Copy link

@matthiascy matthiascy commented Feb 9, 2023

Hi @Twinklebear,

I updated the sys bindings from v3.12.1 to v3.13.5, device/scene creation now returns a Result<Device/Scene, Error> to make sure that the device/scene is created successfully. Sys binding generation script is also updated due to rust-lang/rust-bindgen#1812.

By the way, embree4 is released yeasterday. Do we plan to update? Probably we should finish what left to implement for embree3. I'm quite experienced with embree3, maybe I can help?

@matthiascy
Copy link
Author

Commit bf678b0 tries to resolve #3

To underline that a scene is bound to a specific device,
the scene creation function is refactored as a member function
of device.
- Buffer can be referenced by creating BufferView or BufferViewMut
- Different part of Buffer can be referenced by creating BufferSlice
- BufferView and BufferViewMut can be created from BufferSlice
- Shared data between different geometry objects now resides in BufferGeometry
@Twinklebear
Copy link
Owner

Thanks @matthiascy ! I'll take a look through the changes this week.

As for Embree4, we should update to it, but I think it'll be pretty easy for CPU support since they've added the GPU support without making many API breaking changes. So CPU code can be updated with just a few smaller changes.

Adding support for GPU ray tracing w/ Embree 4 would be super cool, but pretty challenging. I'm not sure how we'd be able to build Rust code using Embree on the GPU to SYCL/SPV kernels that we can run with Intel's Level Zero runtime. The Intel GPU RT intrinsics used by Embree are some extensions in SYCL/OpenCL, so while there's stuff like rust-gpu (https://github.com/EmbarkStudios/rust-gpu) , I don't know how much we could re-purpose or use it since their target is Vulkan shaders. Certainly something to think about though.

@matthiascy
Copy link
Author

Hi @Twinklebear,

I refactored the Buffer part to have much flexible buffer access:

pub struct Buffer {
    pub(crate) device: Device,
    pub(crate) handle: RTCBuffer,
    pub(crate) size: BufferSize,
}

Now buffer only keeps a copy of Device, a handle to raw buffer and its size in bytes. To get access to its data, we first create a BufferSlice(BufferSlice::Buffer specifically), note that the BufferSlice doesn't contain any type parameter. It only keeps the information about Buffer it's referenced to, offset and size.

#[derive(Debug, Clone, Copy)]
pub enum BufferSlice<'src> {
    /// Slice into a [`Buffer`] object.
    Buffer {
        buffer: &'src Buffer,
        offset: usize,
        size: BufferSize,
    },
    /// Slice into memory managed by Embree.
    Internal {
        ptr: *mut ::std::os::raw::c_void,
        size: BufferSize,
        marker: PhantomData<&'src mut [::std::os::raw::c_void]>,
    },
    /// Slice into user borrowed/owned memory.
    User {
        ptr: *const u8,
        offset: usize,
        size: BufferSize,
        marker: PhantomData<&'src mut [u8]>,
    },
}

To access the underlying data, we then use BufferView or BufferViewMut which are created directly from BufferSlice, at this step, user can provide a type parameter to tell how the data should be accessed, the corresponding methods are

impl<'src> BufferSlice<'src> {
    pub fn view<T>(&self) -> Result<BufferView<'src, T>, Error>
    pub fn view_mut<T>(&self) -> Result<BufferViewMut<'src, T>, Error>
}

BufferView and BufferViewMut only keep the raw pointer and other informations.

BufferSlice are also used in geometry object. The common part of a geometry object is encapsulated in BufferGeometry:

pub struct BufferGeometry<'buf> {
    pub(crate) device: Device,
    pub(crate) handle: RTCGeometry,
    kind: GeometryType,
    vertex_attribute_count: Option<u32>,
    pub(crate) attachments: HashMap<BufferUsage, Vec<AttachedBuffer<'buf>>>,
}

the attachments field keeps the information about bound buffers, indexed by it's usage (type), AttachedBuffer

#[derive(Debug)]
pub(crate) struct AttachedBuffer<'src> {
    slot: u32,
    format: Format,
    stride: usize,
    source: BufferSlice<'src>,
}

keeps track of the BufferSlice and access related information, thus we can bind different part of a Buffer for different usage.

The methods in embree to bind a buffer to geometry object rtcSetGeometryBuffer, rtcSetNewGeometryBuffer and rtcSetSharedGeometryBuffer are arranged as follows:

rtcSetGeometryBuffer and rtcSetSharedGeometryBuffer are placed behind set_buffer of BufferGeometry.
rtcSetNewGeometryBuffer is behind set_new_buffer as it returns the pointer to the data of the created buffer internally managed by Embree, for this reason our BufferSlice has the variant named as BufferSlice::Internal.

BufferSlice::Buffer is used for rtcSetGeometryBuffer. As for rtcSetNewGeometryBuffer, we have BufferSlice::User which encapsulates a data pointer created by user (from slice):

impl<'src> BufferSlice<'src> {
    pub fn from_slice<'slice, T, S: RangeBounds<usize>>(slice: &'slice [T], bounds: S) -> Self { 
         ... 
         BufferSlice::User { ... }
    }
}

Of course, user can get direct BufferView/Mut from Buffer:

pub fn mapped_range<S: RangeBounds<usize>, T>(&self, bounds: S) -> BufferView<'_, T>;
pub fn mapped_range_mut<S: RangeBounds<usize>, T>(&mut self, bounds: S) -> BufferViewMut<'_, T>;

As for geometries, we just create tuple structs:

pub struct TriangleMesh(BufferGeometry<'static>);
pub struct QuadMesh(BufferGeometry<'static>);

then impl Deref.

I adopted 3 examples from embree's tutorial, the minimal one doesn't have graphic output. Here are screenshots of triangle and triangle_geometry.

Screenshot from 2023-02-15 17-58-10
Screenshot from 2023-02-15 17-55-38

Copy link
Owner

@Twinklebear Twinklebear left a comment

Choose a reason for hiding this comment

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

Thanks a lot @matthiascy this looks awesome! I put some notes through the PR about smaller things, and just had some higher-level questions/notes as well:

  • Do you also plan or think we can remove the Arc<dyn Geometry> from the scene? Maybe to be just an Rc? Or remove entirely? It may not be possible since we're using trait objects w/ runtime types/inheritance, but I'm a bit rusty
  • With the improvements to the way buffers are set on objects, should we even keep the ::unanimated implementations for the different Geometries? I don't see those being a common codepath for real applications which would likely be passing BufferSlice::User to us, so I think we could just remove them. Removing them also results in the API following Embree's C API more closely, where you make a geometry, then attach buffers to it, commit it, etc.
  • The macOS failure looks like it just needs to have the cp command change to cp -r to copy the dirs properly.
  • The Linux failure looks like it's just formatting

@matthiascy
Copy link
Author

  • Do you also plan or think we can remove the Arc<dyn Geometry> from the scene? Maybe to be just an Rc? Or remove entirely? It may not be possible since we're using trait objects w/ runtime types/inheritance, but I'm a bit rusty

Myabe we can remove dyn Geometry entirely. There is not need to keep Geometry trait, knowing that most methods are implemented on BufferGeometry. We can rename BufferGeometry as Geometry then the scene accepts references(& or Rc or Arc) to the Geometry. As concrete geometry types deref to &Geometry, attach_geometry can accept &Geometry instead of using trait objects.
For the moment, BufferGeometry is non-clonable and non-copyable.

  • If we use shared reference &, then we are enforing lifetime dependency between Scene and geometry object, but according to Embree, after committing a geometry object, it can then be released, so this does not describe perfectly the relationship between geometries and a scene.
  • If we use Rc and Arc: because that the BufferGeometry maintains information like attached buffers, we need to put these fields behind a Arc<Mutex<..>> to make it sharable across threads but only one can modify it at a time.

Or following the idea of encapsulating some fields inside Arc<Mutex<...>>, we can make BufferGeometry clonable, then when attaching to the scene, we still use &Geometry as input prameter but clone it to store inside the scene.

attach_geometry(&mut self, mesh: &Geometry) -> u32 {
   ...
   self.geometry.insert(id, mesh.clone());
}

The Scene can also be implemented this way, clonable but control the access to some field, then we allow sharing Scene between threads(this is needed, it is thread-safe to call intersect/ocluded function from multiple threads). Don't know if it would be possible to discard the field of attached geometries. If so, Scene would be even more simple.

pub struct Scene {
    pub(crate) handle: RTCScene,
    pub(crate) device: Device,
    geometry: Arc<Mutex<HashMap<u32, BufferGeometry>>>,
}
  • With the improvements to the way buffers are set on objects, should we even keep the ::unanimated implementations for the different Geometries? I don't see those being a common codepath for real applications which would likely be passing BufferSlice::User to us, so I think we could just remove them. Removing them also results in the API following Embree's C API more closely, where you make a geometry, then attach buffers to it, commit it, etc.

I think we can just remove it.

Last thing, should we keep track of the mapped range of each Buffer to make sure there is no overlap?

#[derive(Debug)]
pub struct Buffer {
    pub(crate) device: Device,
    pub(crate) handle: RTCBuffer,
    pub(crate) size: BufferSize,
    pub(crate) mapped_ranges: Mutex<Vec<Range>>
}

@matthiascy
Copy link
Author

Hi @Twinklebear,

I finished most parts of Embree3 except BVH, and we now have 6 working examples: minimal, triangle, triangle_geometry, dynamic_scene, instancing, displacement_geometry.

@Twinklebear
Copy link
Owner

Awesome, thanks for all your work @matthiascy ! I should have time to review the PR in the next few days.

@matthiascy
Copy link
Author

We now have a new example: intersection_filter, still need to implement the stream version.
Screenshot from 2023-03-02 18-40-03

@matthiascy
Copy link
Author

Updates:

  1. Replaced old image to TiledImage to enable parallel rendering per image tile in examples.
  2. Intergrated egui to enable UI displaying.
  3. Fixed window can't be resized.
  4. Implemented EyeLight, UV, Normal, CPUCycles, GeometryID and GeometryPrimitiveID shading mode. (missing Occlusion, TexCoords, TexCoordsGrid, AmbientOcclusion)

Screenshot from 2023-03-13 11-11-23
Screenshot from 2023-03-13 11-11-30
Screenshot from 2023-03-13 11-11-38
Screenshot from 2023-03-13 11-11-49
Screenshot from 2023-03-13 11-11-56
Screenshot from 2023-03-13 11-12-02
Screenshot from 2023-03-13 11-12-06

@Twinklebear
Copy link
Owner

Thanks again @matthiascy ! Sorry that my "next few days" note above has turned out to be untrue, I've been tied up with some other projects.

Do you plan to add those other renderers or additional features for this PR? I was also waiting a bit as I saw you were still pushing new stuff.

I have a paper deadline March 31 so I may not have enough time to look through everything properly until after that, if there were additional things you wanted to add. We could look at updating to Embree 4 as well, the changes on the CPU side are not too big (though there are some breaking changes).

@matthiascy
Copy link
Author

As v3.13.5 is the last version of embrre3, I'm planning to implement all features of embree3 along with examples, and then we can release a stable version, otherwise we'll always have something incomplete. Those who still wish to use embree3 can download this version, which includes the complete implementation with tutorials. By implementing tutorials, we can also test our abstraction in Rust, and iterate the design and ergonomics.

Following that, we can start updating to embree4. As for SYCL, it's a specification, maybe we can start a project to implement it in Rust with the help of other Rust GPU related repos like rust-gpu, rust-cuda, nvptx, and so on.

@Twinklebear
Copy link
Owner

Hi @matthiascy , I just wanted to check in if this was ready for a review through yet? It sounded like you planned to add a lot more to get up to 3.13.5. No pressure if you're busy with other stuff, I just wanted to make sure you weren't blocked waiting for me to review.

@matthiascy
Copy link
Author

Hi @Twinklebear, your review is not causing any delays or blocking my work. However, because of other projects, I now have limited time to work on it. I'll try to finish it once my present project is done. :)

@Twinklebear
Copy link
Owner

No problem @matthiascy ! No pressure, and thanks for all your hard work so far 😄

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