Skip to content

Conversation

@Waqar-ukaea
Copy link
Collaborator

@Waqar-ukaea Waqar-ukaea commented Oct 24, 2025

Not really a required PR but I feel like it makes sense from a place of wanting to keep things organised. There are a number of files which are embree specific which have currently been living in include/xdg or src when they could be moved to their respective embree/ directory.

The only changes here are moving files to embree/ and changing headers/Cmake configuration to respect these changes. So from git's perspective lots of "renamed" files since their paths have changed.

@Waqar-ukaea Waqar-ukaea force-pushed the rearrange-embree-files branch from b401c21 to 33d4613 Compare October 27, 2025 11:13
@Waqar-ukaea Waqar-ukaea changed the title Move embree specific files into embree directory Making it possible to compile without Embree enabled Oct 29, 2025
@Waqar-ukaea
Copy link
Collaborator Author

I started this PR just trying to re-organise the structure of the Embree specific headers and source files but I then started taking it further by making the changes necessary to see if I could get XDG to compile without Embree enabled. This is proving to be a little more difficult than I had hoped as the GPRTRayTracer class is still missing a lot of the features that the EmbreeRayTracer class has.

I think it would be worth spending some more time getting the GPRT implementation up to speed before attempting to try and make it possible to compile XDG without Embree. Either way, I think there are a few nice QOL changes I've made in this PR which are separate enough to be split out into smaller PRs.

@Waqar-ukaea Waqar-ukaea added the Embree Embree specific changes which have no effect on the existing GPRT interface label Nov 20, 2025
Copy link
Collaborator

@gonuke gonuke left a comment

Choose a reason for hiding this comment

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

Although it seems like this is still on hold (maybe?), everything that is here seems pretty logical and valuable. I made a single comment about indentation (for readability) in the CMakeLists file.

@pshriwise
Copy link
Collaborator

Made a few more updates here so that tests which can only be run with the Embree backend are skipped when built with GPRT. This won't be evidenced by the current CI setup, but after #183 goes in I'll extend the CI matrix further to configure with Embree only , GPRT only, and Embree + GPRT

Copy link
Collaborator Author

@Waqar-ukaea Waqar-ukaea left a comment

Choose a reason for hiding this comment

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

Couple suggests/minor points but i'm happy with the additional changes made here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Embree Embree specific changes which have no effect on the existing GPRT interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants