feat(integration.ltcgi): drive LTCGI realtime area light from the media player#897
Open
towneh wants to merge 2 commits into
Open
feat(integration.ltcgi): drive LTCGI realtime area light from the media player#897towneh wants to merge 2 commits into
towneh wants to merge 2 commits into
Conversation
…ia player Adds com.basis.integration.ltcgi: BasisLTCGIVideoAdapter feeds a BasisMediaPlayer's live output texture into LTCGI's dynamic blur chain, so a screen lit by the player emits matching realtime area light. The assembly compiles to nothing unless both com.basis.mediaplayer and at.pimaker.ltcgi are present (asmdef define constraints), so it is inert until both are installed.
Swap the three init-guard Debug.LogWarning calls for BasisDebug.LogWarning with LogTag.Video, matching the sibling integration packages, so the warnings respect the project-wide logging toggle. Adds the BasisDebug assembly reference.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
com.basis.integration.ltcgi— an optional bolt-on that drives LTCGI realtime area lighting from a Basis Media Player, so a video screen casts coloured light that matches whatever is playing.BasisLTCGIVideoAdaptersubscribes toBasisMediaPlayer.OnOutputTextureChanged, blits each frame into a RenderTexture it owns (the player exposes a plainTextureand never owns a RenderTexture; LTCGI samples one shared video texture through its blur chain), and hands that RT toLTCGI_UdonAdapter._SetVideoTexture. LTCGI's blur chain re-prefilters it every frame, so the emitted light tracks the video live. It also re-asserts the feed if LTCGI's global shader state is reset at runtime (e.g. a Desktop/VR mode switch), so the lighting self-heals.Modelled on the existing
com.basis.integration.audiolink/com.basis.integration.ytdlppackages: an embedded package that is inert until its dependencies are present — the assembly compiles to nothing unless bothcom.basis.mediaplayerandat.pimaker.ltcgiare in the project (asmdef define constraintsBASIS_MEDIAPLAYER_EXISTS+LTCGI_EXISTS). Safe to land without forcing LTCGI into the project.Scope: package source only — no manifest change.
at.pimaker.ltcgiis intentionally not wired into the project manifest yet; the URP-compatible LTCGI surface shaders this relies on are being upstreamed to PiMaker's repo, after which the dependency can be added separately.Required checks
All boxes below must be ticked before this PR can merge. If a check is genuinely N/A, tick it anyway and explain under Notes.
TransformAccessArrayor are otherwise batched. I have not added per-frametransform.position/transform.rotation/transform.localPositioncalls inside loops. Whenever I need both position and rotation, I use the combined APIs —SetPositionAndRotation/SetLocalPositionAndRotationfor writes,GetPositionAndRotation/GetLocalPositionAndRotationfor reads — instead of two separate property accesses; the combined call does one local-to-world matrix traversal instead of two.Resources.Load, no direct asset references that pull large content into memory on scene load.GetComponent/AddComponentwhere avoidable — Where unavoidable, the result is cached on a field, and anyGetComponent<T>is replaced withTryGetComponent<T>(out var x)— bareGetComponentwill be denied.TryGetComponentis the modern API (Unity 2019.2+) and skips the Editor-only GC allocationGetComponentcauses when a component is missing: Unity wraps thenullreturn in a managed "fake null" object so its overloaded==operator can still detect destroyed C++ objects, and constructing that wrapper allocates;TryGetComponentreturns aboolplusoutparameter and never builds the wrapper. None of these calls run insideUpdate,LateUpdate,FixedUpdate, jobs, or other per-frame code paths.BasisEventDriver— Any new per-frame work hooks intoBasisEventDriverrather than adding standaloneUpdate/LateUpdate/FixedUpdatecallbacks on a MonoBehaviour.BasisEventDriveris bulletproof, or guarded bytry/catch—BasisEventDriverruns the single per-frame tick that drives the whole framework (network apply, local player sim, blendshapes, JigglePhysics, nameplates, and more) as one sequential chain. An unhandled exception anywhere in that chain aborts the rest of the tick, so every step after the throwing one is silently skipped for that frame. New work added to the driver must either be guaranteed not to throw, or be wrapped in atry/catchthat contains the failure and surfaces it throughBasisDebug— logged once / rate-limited, never every frame (see the existingHVRBasisBuiltInAddresses.Simulate()guard for the pattern). Expect this to be scrutinized closely in review.{ get; set; }properties or access lockdowns — Public fields are fine; Basis is meant to be read and modified freely, so don't wall things offprivate/internalwithout a real reason. Don't wrap a field in{ get; set; }when the accessors do nothing — property accessors have a real performance cost vs direct field access, and the lead maintainer prefers plain fields (or a method / setter-only property when only the setter needs logic) over a noop-getter pair. For.Instancesingletons, callers reassigningType.Instanceis allowed; if that would break your code, log a warning or throw — don't block the assignment. Locking down access is not your call.BasisLocalCameraDriver— Code that needs the local camera (transform, projection, rig data, etc.) pulls it fromBasisLocalCameraDriverrather than looking one up itself. Don't roll a separate camera discovery path.BasisDebug— All new logging calls go throughBasisDebug.Log/BasisDebug.LogWarning/BasisDebug.LogError(with an appropriateLogTag) instead ofUnityEngine.Debug.Log/Debug.LogWarning/Debug.LogError.BasisDebugroutes through Basis's tagged, color-coded logger and respects the project-wideLoggingDisabledtoggle so logging can be killed at runtime; bareDebug.Logcalls bypass that and will be denied.FindObjectOfType/FindObjectsOfType/GameObject.Find/FindGameObjectsWithTagto locate what it depends on. References are wired in — registered through an existing manager/driver, injected at init, or passed in by the caller — rather than discovered by scanning the scene at runtime. If a scene scan is genuinely unavoidable, justify it under Notes.newon reference types, no LINQ, nostringconcatenation/interpolation, no boxing, noforeachover interface-typed collections. Allocate once at init and reuse the buffer.BasisDebug. Hot-path logging floods the console and incurs cost on every frame regardless of whether the message is filtered out downstream. If a hot-path log is needed while iterating, gate it behind#if UNITY_EDITORand remove (or leave gated) before merge..Count(lists) /.Length(arrays) into a localintbefore the loop instead of re-reading the property each iteration. PreferT[](with a separate length int when the array is over-sized) overList<T>where the data is hot — Unity's mono BCL doesn't exposeCollectionsMarshal.AsSpan(List<T>), so a list can't be fed intoSpan<T>/ unsafe paths cleanly. Where the perf justifies it, drop intoSpan<T>/reflocals /Unsafe.As/unsafepointer code to skip bounds checks and copies, and call out the invariants you're relying on under Notes so reviewers can sanity-check them.Testing details
Tick the platforms you actually tested on. Leave the rest unticked — these are informational and do not block merge.
Input / control mode coverage:
Where applicable, confirm these flows still work after your changes:
Notes
Per-frame check (
BasisEventDriver). The adapter's per-frame Blit/rebind runs in a standaloneLateUpdate, matching the established pattern for leaf per-frame components in directly comparable packages —com.basis.integration.audiolink(BasisAudioLinkReactiveLight,BasisAudioLinkReactiveMaterial) and severalcom.basis.mediaplayerruntime scripts all useUpdate/LateUpdate.BasisEventDriveris a core, hard-wired call chain with no registration API; hooking a leaf, optional, inert-until-deps integration into it would mean editing core to reference an assembly that may not compile. Happy to move it onto the driver if you'd prefer that for this class of component — flag it and I'll wire it in.BasisEventDriverbulletproofing — N/A: nothing is added to the driver (see above).Scene discovery. The dependencies are serialized references (
Player,Target) and are the primary path. The singleFindFirstObjectByType<LTCGI_UdonAdapter>()is a one-shot fallback inOnEnable, used only whenTargetis left unassigned in the inspector; the result is cached. No per-frame scanning. Can make the reference mandatory and drop the fallback if you'd rather forbid the scan entirely.GetComponent. The only lookup isGetComponentInParent<BasisMediaPlayer>(), once inOnEnable/Reset, cached on thePlayerfield. No bareGetComponent, nothing on a per-frame path.Allocations / hot path. The
LateUpdatepath (Blit+EnsureBound) is GC-free:Blitconstructs onlyVector2structs (stack, no heap), and the LOD-rebind loop runs solely on a detected reset, not the steady per-frame path.Transform / Addressables / Camera — N/A: the package touches no transforms, loads no assets, and accesses no camera.
Jobification — N/A: the per-frame work is a single
Graphics.Blit(GPU); there is no CPU-side work to move to a Burst job.Testing. Verified on Windows in both desktop and VR. Specifically exercised the desktop↔VR runtime mode switch: LTCGI's globals get cleared on the switch and the adapter's
EnsureBoundself-heal re-asserts the feed within a frame, so the screen keeps lighting the room after the swap. Avatar/server swapping are unrelated to this change. Headset: Quest Pro over Virtual Desktop (PCVR streaming).