Skip to content

Fix CAMERA_CULL AND-on-same-object regression vs Cycles - #1

Open
liminallyspaced wants to merge 1 commit into
mainfrom
plugwalk/distance-cull-aggressive
Open

Fix CAMERA_CULL AND-on-same-object regression vs Cycles#1
liminallyspaced wants to merge 1 commit into
mainfrom
plugwalk/distance-cull-aggressive

Conversation

@liminallyspaced

Copy link
Copy Markdown
Owner

Summary

  • Fixes a draft bug that set both object.cycles.use_camera_cull and object.cycles.use_distance_cull on the same CAMERA_CULL scatter names and claimed OR semantics.
  • Cycles intern/cycles/blender/object_cull.cppANDs when both object flags are on (keeps nearby off-frustum for reflections). Dual flags on the Classroom scatter list would regress the cull slice of the 41% plate.
  • Correct shape (kind stays CAMERA_CULL): scene enables simplify + camera + distance cull; payload["objects"] get camera-only; disjoint payload["distance_objects"] (far tiny via min_camera_distance ≥ margin) get distance-only. Missing distance RNA → camera path alone.

Test plan

  • python3 tests/test_speed_solver.py — ALL TESTS PASSED
  • Apply tests assert camera list never gets use_distance_cull; distance list never gets use_camera_cull; scene both flags on; missing RNA OK
  • Planner partition test: near tiny → camera-only, far tiny → distance-only, disjoint
  • blender_manifest.toml still 0.3.5 (not bumped)
  • Optional: Aggressive Make it Fast smoke on official Classroom when 2080 is free (warn Nick first); store claims stay 41%/52%

Authoritative report: docs/research/PLUGWALK-2026-08-30-14.md §1.

Cycles object_cull.cpp ANDs when both object cull flags are set on the
same name (keeps nearby off-frustum for reflections). Dual flags on the
CAMERA_CULL scatter list would regress the Classroom cull slice of the
41% plate.
Split into disjoint sets: payload objects get use_camera_cull only;
distance_objects (far tiny via min_camera_distance) get use_distance_cull
only. Scene may enable both flags; missing distance RNA stays camera-only.
Manifest stays 0.3.5; no store % change.

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:2320888cd9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 701 to +702
if jrnl.set_prop(obj, "cycles.use_camera_cull", True, SPEED_TAG):
# Never set use_distance_cull on camera-only names (AND regression).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Clear stale distance flags before camera-culling objects

When an object already has cycles.use_distance_cull=True (for example from a saved/manual distance-cull setup), this branch still writes cycles.use_camera_cull=True after the scene distance flag has been enabled. That leaves both object flags on a camera-only name, so Cycles uses AND semantics and nearby off-frustum scatter will not be camera-culled—the regression the new partition is meant to avoid. Please clear or skip the opposite per-object flag before tagging camera-only objects, and mirror that protection for distance-only objects.

Useful? React with 👍 / 👎.

Comment threadCHANGELOG.md
Comment on lines +19 to +20
scatter remains eligible. No measured %; store plates stay Classroom 41% /
loft 52%.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Spell benchmark percentages as measured pairs

This new release note prints the 41%/52% benchmark shorthand without the baseline-to-fast timings and measurement context, and the same shorthand is added in the research docs. The repository guidance permits benchmark numbers only as measured pairs, so rendered or copied notes no longer carry the required proof context; either omit these percentages here or spell out the full measured pairs.

AGENTS.md reference: AGENTS.md:L52-L58

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@liminallyspaced