Fix: Draw: EEVEE-Next shadow flickering and other uninitialized bounds fixes #120591

Merged
Miguel Pozo merged 7 commits from pragma37/blender:pull-draw-fix-uninitialized-bounds into main 2024-04-16 17:19:29 +02:00
10 changed files with 54 additions and 16 deletions

View File

@ -22,8 +22,7 @@ void main()
}
ObjectBounds bounds = bounds_buf[index];
/* Bounds are not correct as culling is disabled for these. */
if (bounds._inner_sphere_radius <= 0.0) {
if (!drw_bounds_are_valid(bounds) || bounds._inner_sphere_radius <= 0.0) {
return;
}

View File

@ -38,10 +38,14 @@ void main()
uint resource_id = resource_ids_buf[gl_GlobalInvocationID.x];
resource_id = (resource_id & 0x7FFFFFFFu);
IsectBox box = isect_box_setup(bounds_buf[resource_id].bounding_corners[0].xyz,
bounds_buf[resource_id].bounding_corners[1].xyz,
bounds_buf[resource_id].bounding_corners[2].xyz,
bounds_buf[resource_id].bounding_corners[3].xyz);
ObjectBounds bounds = bounds_buf[resource_id];
if (!drw_bounds_are_valid(bounds)) {
return;
}
IsectBox box = isect_box_setup(bounds.bounding_corners[0].xyz,
bounds.bounding_corners[1].xyz,
bounds.bounding_corners[2].xyz,
bounds.bounding_corners[3].xyz);
int clipped = 0;
/* NDC space post projection [-1..1] (unclamped). */

View File

@ -43,6 +43,11 @@ void main()
DRW_RESOURCE_ID_VARYING_SET
ObjectBounds bounds = bounds_buf[resource_id];
if (!drw_bounds_are_valid(bounds)) {
/* Discard. */
gl_Position = vec4(NAN_FLT);
return;
}
Box box = shape_box(bounds.bounding_corners[0].xyz,
bounds.bounding_corners[0].xyz + bounds.bounding_corners[1].xyz,

View File

@ -21,6 +21,7 @@ shared int global_max;
void main()
{
IsectBox box;
bool is_valid = true;
if (resource_len > 0) {
uint index = gl_GlobalInvocationID.x;
@ -30,6 +31,7 @@ void main()
resource_id = (resource_id & 0x7FFFFFFFu);
ObjectBounds bounds = bounds_buf[resource_id];
is_valid = drw_bounds_are_valid(bounds);
box = isect_box_setup(bounds.bounding_corners[0].xyz,
bounds.bounding_corners[1].xyz,
bounds.bounding_corners[2].xyz,
@ -63,9 +65,11 @@ void main()
local_min -= abs(local_min) * 0.01;
local_max += abs(local_max) * 0.01;
/* Intermediate result. Min/Max of a compute group. */
atomicMin(global_min, floatBitsToOrderedInt(local_min));
atomicMax(global_max, floatBitsToOrderedInt(local_max));
if (is_valid) {
/* Intermediate result. Min/Max of a compute group. */
atomicMin(global_min, floatBitsToOrderedInt(local_min));
atomicMax(global_max, floatBitsToOrderedInt(local_max));
}
barrier();

View File

@ -77,6 +77,10 @@ void main()
}
ObjectBounds bounds = bounds_buf[gl_GlobalInvocationID.x];
if (!drw_bounds_are_valid(bounds)) {
/* Invalid bounding box. */
return;
}
IsectBox box = isect_box_setup(bounds.bounding_corners[0].xyz,
bounds.bounding_corners[1].xyz,
bounds.bounding_corners[2].xyz,

View File

@ -155,10 +155,10 @@ class Instance {
if (is_object_data_visible) {
if (object_state.sculpt_pbvh) {
/* Disable frustum culling for sculpt meshes. */
/* TODO(@pragma37): Implement a cleaner way to disable frustum culling. */
ResourceHandle handle = manager.resource_handle(ob_ref.object->object_to_world());
handle = ResourceHandle(handle.resource_index(), ob_ref.object->transflag & OB_NEG_SCALE);
const Bounds<float3> bounds = BKE_pbvh_bounding_box(ob_ref.object->sculpt->pbvh);
const float3 center = math::midpoint(bounds.min, bounds.max);
const float3 half_extent = bounds.max - center;
ResourceHandle handle = manager.resource_handle(ob_ref, nullptr, &center, &half_extent);
sculpt_sync(ob_ref, handle, object_state);
emitter_handle = handle;
}

View File

@ -11,6 +11,7 @@
* Each of them are reference by resource index (#ResourceHandle).
*/
#include "BLI_math_base.h"
#include "BLI_math_matrix.hh"
#include "BKE_curve.hh"
@ -163,6 +164,14 @@ inline std::ostream &operator<<(std::ostream &stream, const ObjectInfos &infos)
inline void ObjectBounds::sync()
{
#ifndef NDEBUG
/* Initialize to NaN for easier debugging of unitialized data usage. */
bounding_corners[0] = float4(NAN_FLT);
bounding_corners[1] = float4(NAN_FLT);
bounding_corners[2] = float4(NAN_FLT);
bounding_corners[3] = float4(NAN_FLT);
bounding_sphere = float4(NAN_FLT);
#endif
bounding_sphere.w = -1.0f; /* Disable test. */
}
@ -170,6 +179,14 @@ inline void ObjectBounds::sync(const Object &ob, float inflate_bounds)
{
const std::optional<blender::Bounds<float3>> bounds = BKE_object_boundbox_get(&ob);
if (!bounds) {
#ifndef NDEBUG
/* Initialize to NaN for easier debugging of unitialized data usage. */
bounding_corners[0] = float4(NAN_FLT);
bounding_corners[1] = float4(NAN_FLT);
bounding_corners[2] = float4(NAN_FLT);
bounding_corners[3] = float4(NAN_FLT);
bounding_sphere = float4(NAN_FLT);
#endif
bounding_sphere.w = -1.0f; /* Disable test. */
return;
}

View File

@ -203,6 +203,11 @@ struct ObjectBounds {
};
BLI_STATIC_ASSERT_ALIGN(ObjectBounds, 16)
inline bool drw_bounds_are_valid(ObjectBounds bounds)
{
return bounds.bounding_sphere.w >= 0.0f;
fclem marked this conversation as resolved Outdated

bounds.bounding_sphere.w == -1.0f only means the culling is disabled (which does not always means the bounding box is invalid (see snippet).

    if (bounds.bounding_sphere.w > 1e12) {
      bounds.bounding_sphere.w = -1.0;
    }

Maybe set it to -2 when bounding box is invalid.

`bounds.bounding_sphere.w == -1.0f` only means the culling is disabled (which does not always means the bounding box is invalid (see snippet). ``` if (bounds.bounding_sphere.w > 1e12) { bounds.bounding_sphere.w = -1.0; } ``` Maybe set it to `-2` when bounding box is invalid.

bounds.bounding_sphere.w == -1.0f only means the culling is disabled (which does not always means the bounding box is invalid

At the moment disabling culling effectively means the bounding box is invalid, since the only way to disable culling is to create a ResourceHandle without bounds.

Right now culling is only explicitly disabled for objects with invalid bounding boxes (curves and hair), and for the lookdev spheres.

Maybe we need a better design for this.

About the diagonal > 1e12 case, I think is fair to treat those bounds as invalid. That's going to wreak havoc on any system that relies on object bounds.

> `bounds.bounding_sphere.w == -1.0f` only means the culling is disabled (which does not always means the bounding box is invalid At the moment disabling culling effectively means the bounding box is invalid, since the only way to disable culling is to create a ResourceHandle without bounds. Right now culling is only explicitly disabled for objects with invalid bounding boxes (curves and hair), and for the lookdev spheres. Maybe we need a better design for this. About the diagonal > 1e12 case, I think is fair to treat those bounds as invalid. That's going to wreak havoc on any system that relies on object bounds.
}
/** \} */
/* -------------------------------------------------------------------- */

View File

@ -19,7 +19,7 @@ void main()
ObjectInfos infos = infos_buf[resource_id];
ObjectBounds bounds = bounds_buf[resource_id];
if (bounds.bounding_sphere.w != -1.0) {
if (drw_bounds_are_valid(bounds)) {
/* Convert corners to origin + sides in world space. */
vec3 p0 = bounds.bounding_corners[0].xyz;
vec3 p01 = bounds.bounding_corners[1].xyz - p0;
@ -48,7 +48,7 @@ void main()
/* TODO: Bypass test for very large objects (see #67319). */
if (bounds.bounding_sphere.w > 1e12) {
bounds.bounding_sphere.w = -1.0;
bounds.bounding_sphere.w = -2.0;
}
/* Update bounds. */

View File

@ -32,7 +32,7 @@ void main()
ObjectBounds bounds = bounds_buf[gl_GlobalInvocationID.x];
if (bounds.bounding_sphere.w != -1.0) {
if (drw_bounds_are_valid(bounds)) {
IsectBox box = isect_box_setup(bounds.bounding_corners[0].xyz,
bounds.bounding_corners[1].xyz,
bounds.bounding_corners[2].xyz,