embree: Update to 4.3.1

[Chubercik]

Fixes #78332.

https://github.com/embree/embree/releases/tag/v4.3.1

That was a big one, I'm gonna leave commits be for now for that extra development info, if everything seems A-OK, I'll push a rebase.

I'd like to ask for an extra careful review of config files - I did the update and resolved any encountered issues, so everything compiles just fine on my end, but I haven't riffled through Godot's codebase for how the lib is used, so maybe certain #defines should be uncommented.

Important info about moving from version 3 to version 4 of the embree library can be found here:

• https://github.com/embree/embree?tab=readme-ov-file#upgrading-from-embree-3-to-embree-4

[Chubercik]

For reference, the config files that Godot gets from running modules/raycast/godot_update_embree.py originate from here:

• thirdparty/embree/kernels/config.h: https://github.com/embree/embree/blob/daa8de0e714e18ad5e5c9841b67c1950d9c91c51/kernels/config.h.in
• thirdparty/embree/include/embree4/rtcore_config.h: https://github.com/embree/embree/blob/daa8de0e714e18ad5e5c9841b67c1950d9c91c51/kernels/rtcore_config.h.in

[Chubercik]

For reference, the config files that Godot gets from running modules/raycast/godot_update_embree.py originate from [...]

Now that I think about it, maybe a better way of going about this would be to just pull them from upstream, move them into their correct positions in the file tree and apply changes through git diffs..

Waiting for someone to weigh their opinions on this.

[Angular-Angel]

Pulled the PR for this and compiled it, tried running https://github.com/godotengine/godot-demo-projects/tree/master/3d/occlusion_culling_mesh_lod with it. Worked perfectly so far as I can tell.

[akien-mga]

Looks great to me!

[akien-mga]

For reference, the config files that Godot gets from running modules/raycast/godot_update_embree.py originate from [...]

Now that I think about it, maybe a better way of going about this would be to just pull them from upstream, move them into their correct positions in the file tree and apply changes through git diffs..

Waiting for someone to weigh their opinions on this.

That might be a good way to highlight what changes we actually make to the default config yeah.
We could have our customization in a patch file.

[AThousandShips]

Might be a bit finicky, so you can wait until my PR has been merged :) Hopefully it can be merged quickly and we can move forward with this one

[AThousandShips]

There, please rebase :)

[Chubercik]

Will do, I'm out right now so gimme like ≈1h 😅

Done-and-done 😎

[AThousandShips]

You merged into it instead of rebasing it seems, please do:

git reset --hard 15553c7
git rebase -i master
git push -f

[AThousandShips]

You first need to update your master branch to match the upstream, your branch is now behind and doesn't include the fix

Edit: Seems it works anyway by using the main branch even if yours doesn't include it, so it's fine

[Chubercik]

Checked out the same example project as @Angular-Angel, everything seems to be working as expected:

LOD & OC off	LOD & OC on

There appears to be some instability far off in the distance, but that might've already been present with Embree 3 and is not to blame on the update (needs confirmation):

[akien-mga]

There appears to be some instability far off in the distance, but that might've already been present with Embree 3 and is not to blame on the update (needs confirmation):

That would be worth testing with 4.3-dev5 to confirm it's not a regression.

[Chubercik]

Tested with 4.3-dev5, said instability is also present and therefore not a result of this update :)

[Calinou]

The instability is likely fixed by #86121.

[Chubercik]

@Calinou you're right, it's gone or at least it's unperceivable on that PR's build (a4f879c).

[fire]

Careful review of the comment thread suggests that the embree upgrade works and that the instability is gone.

Pair with #86121

[akien-mga]

Thanks!

[Calinou]

This PR causes a performance regression as seen in the demo project when run at a low resolution (--resolution 640x480), where it's bottlenecked by occlusion culling:

Before	After

I tested this on a Linux editor build with optimize=speed lto=full. When occlusion culling is disabled, performance is strictly identical on both builds. The only different commit is the Embree update, both were built on the same base commit otherwise.

Should we report it upstream? I'm not sure how to create a minimal project for Embree developers to take a look at.

PC specifications

• CPU: Intel Core i9-13900K
• GPU: NVIDIA GeForce RTX 4090
• RAM: 64 GB (2×32 GB DDR5-5800 C30)
• SSD: Solidigm P44 Pro 2 TB
• OS: Linux (Fedora 39)

[Chubercik]

This PR causes a performance regression as seen in the demo project when run at a low resolution (--resolution 640x480)

Is the regression only present at a low resolution?

[Calinou]

Is the regression only present at a low resolution?

It's more noticeable at low resolutions, because rendering at a low resolution avoids being GPU-bound.

[Chubercik]

Unless going from

void RaycastOcclusionCull::Scenario::_raycast(uint32_t p_idx, const RaycastThreadData *p_raycast_data) const {
	RTCIntersectContext ctx;
	rtcInitIntersectContext(&ctx);
	ctx.flags = RTC_INTERSECT_CONTEXT_FLAG_COHERENT;

	rtcIntersect16((const int *)&p_raycast_data->masks[p_idx * TILE_RAYS], ebr_scene[current_scene_idx], &ctx, &p_raycast_data->rays[p_idx]);
}

to

void RaycastOcclusionCull::Scenario::_raycast(uint32_t p_idx, const RaycastThreadData *p_raycast_data) const {
	RTCRayQueryContext context;
	rtcInitRayQueryContext(&context);
	RTCIntersectArguments args;
	rtcInitIntersectArguments(&args);
	args.flags = RTC_RAY_QUERY_FLAG_COHERENT;
	args.context = &context;
	rtcIntersect16((const int *)&p_raycast_data->masks[p_idx * TILE_RAYS], ebr_scene[current_scene_idx], &p_raycast_data->rays[p_idx], &args);
}

could be the cause of regression, I feel like it's gotta be upstream because there are no more Godot-side changes.

The changes above were required due to Embree 4's new API; two more source files had to go through similar updates.