I had previously submitted an update to graphics/embree. The changes that got committed differ from what I submitted.
Specifically I had two options enabled with the line
CMAKE_ON= EMBREE_BACKFACE_CULLING EMBREE_RAY_MASK
From the test compiling I did, there was no build/compile failures with these options on or off but blender expects ray mask to be enabled so I would expect runtime issues if the feature is disabled.
While blender doesn't currently expect backface culling to be on, it is a common feature used in 3d graphics.
Without seeing any build failures with these on I don't see the need for them to be enabled through port options.
> The changes that got committed differ from what I submitted.
Right, I've mentioned this in bug #239314 and asked to comment there if anything is missing. :-)
> Specifically I had two options enabled with the line
> CMAKE_ON= EMBREE_BACKFACE_CULLING EMBREE_RAY_MASK
That's why I tend to avoid CMAKE_ON/OFF, they often escape my eyesight. :-(
> blender expects ray mask to be enabled so I would expect
> runtime issues if the feature is disabled.
> While blender doesn't currently expect backface culling
> to be on, it is a common feature used in 3d graphics.
Hmm, I see. I guess it does make sense to have them turned on by default. One question, however: if they are so commonly used in 3D graphics, why are they both disabled by default in the Embree upstream?
I can only guess at the decisions made. As ray tracing is a long repetitive process, I expect every optimisation is made, both mask and backface culling would add tests to check which side of a face is hit, is there a mask, does it affect this location? Therefore I expect the defaults would offer the fastest results, if not all features.
Just thinking, I know backface culling is used to optimise real time drawing, the culling is faster than drawing the extra faces. In raytracing, there is no drawing, so it may not offer the same benefits. I think the shaders may decide what colour if any to draw rather than the ray tracing.
While I appreciate the need for the fastest library, I see a port needing to offer a more general build that caters for all (most?).
The official blender builds use embree static libs, so as not to rely on the system install being configured as expected. While I wanted to avoid that with the embree port update, if you don't want to offer a more universal build, I could bring in the embree files and build the static libs as part of the blender build.
(In reply to Shane from comment #2)
> if you don't want to offer a more universal build ...
Quite on the contrary: default packages should be more universal rather than optimized for a particular usage scenarios. I was simply wondering about the rationale upstream might had had when deciding whether to enable or disable those features.
> I could bring in the embree files and build the static libs
> as part of the blender build.
No need to, I'll flip those switches in the port as you've asked.
A commit references this bug:
Date: Fri Feb 21 07:51:24 UTC 2020
New revision: 526610
Build `graphics/embree' with two extra features enabled which are commonly
required by various 3D software, particularly, `graphics/blender'.
Should be taken care of with ports r526610. Please check and report if this is sufficient or if you want some other adjustments made to the port so we can close this PR.
Looks good to me