Bug 244190 - Adjustment to graphics/embree
Summary: Adjustment to graphics/embree
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Alexey Dokuchaev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-02-17 11:58 UTC by Shane
Modified: 2020-02-26 09:56 UTC (History)
0 users

See Also:
bugzilla: maintainer-feedback? (danfe)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Shane 2020-02-17 11:58:03 UTC
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.
Comment 1 Alexey Dokuchaev freebsd_committer 2020-02-17 14:39:48 UTC
> 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?
Comment 2 Shane 2020-02-21 03:34:47 UTC
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.
Comment 3 Alexey Dokuchaev freebsd_committer 2020-02-21 05:45:20 UTC
(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.
Comment 4 commit-hook freebsd_committer 2020-02-21 07:52:14 UTC
A commit references this bug:

Author: danfe
Date: Fri Feb 21 07:51:24 UTC 2020
New revision: 526610
URL: https://svnweb.freebsd.org/changeset/ports/526610

Log:
  Build `graphics/embree' with two extra features enabled which are commonly
  required by various 3D software, particularly, `graphics/blender'.

  PR:	244190

Changes:
  head/graphics/embree/Makefile
Comment 5 Alexey Dokuchaev freebsd_committer 2020-02-21 07:57:15 UTC
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.
Comment 6 Shane 2020-02-26 09:56:51 UTC
Looks good to me