Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor PostProcessingComponent into reactive registry of effects #10148

Draft
wants to merge 70 commits into
base: dev
Choose a base branch
from

Conversation

MbfloydIR
Copy link
Contributor

Summary

Added a Highlight system to add the highlight component to selected items.
Based off of the branch michael/IR-594-postprocessing since it also effects the post processing changes

Subtasks Checklist

Breaking Changes

References

closes #insert number here

QA Steps

MbfloydIR and others added 30 commits April 18, 2024 11:25
* dev: (42 commits)
  fix(magic-link): Fix guest identity provider removal upon login token validation and retain only the latest generated token (#10078)
  Support GLTF Authoring (#10088)
  fix getNestedChildren not snubing children (#10097)
  Ir 507 Refactor InteractableComponent and expose API (#10064)
  Remove AssetDatabaseType from Asset schema (#10095)
  Added Asset Database Type in schema (#10094)
  Updated argument of install-projects.js script
  Fix distance model deserialization having wrong type check (#10091)
  Add GLTF Scene Support (#10045)
  FileThumbnailJobState now unescapes file names and replaces whitespace characters with hyphens to avoid issues with escaped characters in thumbnail URLs not loading.
  Made changes to input and select components (#10081)
  Fixed a couple bugs in cronjob updating. (#10087)
  tailwind error view and other tailwind fixes (#10076)
  Improve mac os onboarding, disable minio, and update mediasoup (#10071)
  Break Up and Type GLTF Loader (#10044)
  update default scene ground plane to be a nicer colour
  Added check for aws rolearn rollback.
  Added `helm repo update` before `helm upgrade` commands (#10084)
  Added batch invalidation fields to builder helm ugprade command (#10083)
  Modal fixes & UI improvements (#10077)
  ...

# Conflicts:
#	packages/spatial/src/renderer/effects/PostProcessing.ts
#	packages/spatial/src/renderer/functions/configureEffectComposer.ts
* dev:
  fix admin projects and build status modals and statuses (#10116)
  Add unique constraint to project name column (#10125)
  9928 optimize asset builds (#9965)
  Physics API Abstraction (#10124)
  rework scene loaded state usage (#10127)
  Camera and Spectation are Event Sourced (#10130)
  Location GLTF State Cleanup (#10129)
  Add locationType menu (#10128)
  IR 465 ECS Materials (#10109)
  Fix scenestate references in playmode tool (#10126)
  Remove old ColliderComponent (#10122)
  GLTF ECS Loader (#10102)
  Systems without any pre/post/sub systems will no longer start out expanded (#10120)
  Updated the avatar radius and camera rotation (#10118)
  Fixed setup helm in dev-deploy (#10092)
  Fix interactable cursor style reactor (#10117)
  IR-1790 making the addComponent search box gain focus automatically when opening the addComponent popup (#10106)
  Update AddEditLocationModal.tsx (#10110)
  Update README.md (#10112)
  MountPoint/Grabbable/Link Components now add Interactables to the authoring layer, optimized useEffects to Systems/Components/Editors (#10105)
…tate of this reactor, so it stops creating a new composer and render pass every time the renderer reactor re-runs
removal of the highlight component, is now down by returning a cleanup function in the use effect
@HexaField HexaField marked this pull request as draft May 20, 2024 23:02
@HexaField HexaField changed the title Michael/ir 1983 highlight Refactor PostProcessingComponent into reactive registry of effects May 20, 2024
…IR-1983-Highlight

* michael/IR-198-Unit-Tests-PostProcessing:
  removed dev logs
  corrected effect schema issue
  updated the post processesing component to not have a use effect for each effect
  removed obsolete commented out code
  updated highlight state and play mode connection to rely on the data of the selection state
  removed the query in the body of a reactor removal of the highlight component, is now down by returning a cleanup function in the use effect
  Updated the composure ref to use a factory function for the initial state of this reactor, so it stops creating a new composer and render pass every time the renderer reactor re-runs
  added unit test for the effect composer
  Move postprocessing
  fix linkedin icon on user table (#10164)
  Adds editor and reviewer project permission types and removes user (#10171)
  Remove unnecessary code for client setting seed (#10168)
  remove unused file
  Add unmount to remove component if extension is deleted (#10167)
  updated the unity test to loop through the effects of the post processing and set its various properties using a master data obj
  [IR-1828] fix: Reparenting Transform applied based on the new Parent Transform (#10161)
  initial attempt at unity test
@MbfloydIR MbfloydIR requested a review from HexaField May 23, 2024 20:59
* dev: (63 commits)
  hotfix vrm0 data not being where we expect it (#10236)
  Update tween.js version (#10241)
  Added ProjectPermissionDatabaseType
  physics bug fix (#10221)
  updated the background to use the color with when in wireframe render mode (#10231)
  vrm expressionmanager was being thrown away it is actually needed for viseme support to work (#10230)
  IR-1887-Asset-Preview-breaking-if-you-click-gltf-that-is-in-the-scene (#10208)
  query function fix (#10229)
  IR-2102 Material/Plugin Parameters (#10180)
  Fixed webcam light not turning off when camera paused. (#10224)
  Made client's server.js not bound to a specific host (#10223)
  Update FeathersHooks.tsx (#10228)
  Updated app name
  feat(setup): update logo and components props (#10204)
  Changes for feature flag schema to be string enum (#10225)
  Cleaned location hooks (#10216)
  Ir 1652 interactable input refactor (#10219)
  IR-2018 Refactor the Select component to disable search functionality for the dropdown only (#10206)
  refactor: Update Primus initialization to include pathname in server URL (#10205)
  Move physics enter/exit back to reactors (#10193)
  ...

# Conflicts:
#	packages/editor/src/components/element/ElementList.tsx
#	packages/editor/src/components/properties/PostProcessingSettingsEditor.tsx
#	packages/engine/src/scene/SceneModule.ts
#	packages/spatial/src/renderer/components/PostProcessingComponent.tsx
#	packages/spatial/src/renderer/functions/configureEffectComposer.ts
* dev:
  URL resolution to "__$project__" on scene save
  IR-1652 input sink fixes (#10233)
@DanielBelmes DanielBelmes marked this pull request as ready for review May 24, 2024 21:41
}
}, [schema.isActive])

useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

For the next changes we can probably just have schema.isActive as the dep and both create and destroy in one useEffect. I don't wanna break functionality if this works for now 😅

const smaaPreset = getState(RenderSettingsState).smaaPreset
const smaaEffect = new SMAAEffect({
preset: smaaPreset,
edgeDetectionMode: EdgeDetectionMode.COLOR
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like we forgot to add this to the effect composer. Gonna do it now

Copy link
Contributor

Choose a reason for hiding this comment

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

Forgot the reason for this. A lot of the new effects are convolution effects that can't mix with SMAA. Gonna just expose in editor and allow people to decide at scene authoring I think. Though we gotta consider error messaging or handling for this library. It's a headache.

Copy link
Member

@HexaField HexaField left a comment

Choose a reason for hiding this comment

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

We spoke on discord to again change the implementation to a full blown registry, making the component extensible and well scoped.

/** Spatial Package */
type EffectRegistryEntry = {
  reactor: (props: { parameters; isActive }) => void
  defaultValues: any // specifies the default values for the effect
  schema: any // specifies a schema for the editor to generate UI for each effect
}

export const PostProcessingEffectState = defineState({
  name: 'PostProcessingEffectState',
  initial: {} as Record<string, EffectRegistryEntry>
})

// PostProcessingComponent
const reactor = () => {
  const entity = useEntityContext()
  const postProcessingComponent = useComponent(entity, PostProcessingComponent)
  const rendererEntity = useScene(entity) // get renderer entity
  const effects = useComponent(entity, PostProcessingEffectState)

  useEffect(() => {
    // create composer etc
  }, [])

  // for each effect specified in our postProcessingComponent, we mount a sub-reactor based on the effect registry for that effect ID
  return (
    <>
      {Object.entries(postProcessingComponent.effects.value).map(([key, value]) => {
        const effect = effects[key] // get effect registry entry
        return (
          <effect.reactor
            key={key}
            parameters={postProcessingComponent.effects[key]}
            isActive={value.isActive}
            rendererEntity={rendererEntity}
          />
        )
      })}
    </>
  )
}

/** Engine package */

// for effects using assets, the effect needs to exist in the engine package
const ChromaticAberrationEffectProcessReactor = (props: { parameters; isActive; rendererEntity }) => {
  const postProcessgComponent = useComponent(entity, PostProcessingComponent)
  const isLutPassActive = postProcessgComponent.effects[lutpass].isActive // if we want an effect to be active only if another effect is active
  const effectComposer = useComponent(props.rendererEntity, RendererComponent).effectComposer
  const mytexture = useAsset()

  useEffect(() => {
    if (!props.isActive || isLutPassActive) return
    const { schema, effects } = props
    const eff = new ChromaticAberrationEffect(schema)
    effects[Effects.ChromaticAberrationEffect].set(eff)
    effectComposer.addPass(eff)
    return () => {
      //cleanup
    }
  }, [props.isActive, isLutPassActive])
}

// registers the effect
getMutableState(PostProcessingEffectState)[Effects.ChromaticAberrationEffect].set({
  reactor: ChromaticAberrationEffectProcessReactor,
  defaultValues: {
    //...
  },
  schema: {
    hue: { propertyType: PropertyTypes.Number, name: 'Hue', min: -1, max: 1, step: 0.01 },
    saturation: { propertyType: PropertyTypes.Number, name: 'Saturation', min: -1, max: 1, step: 0.01 }
  }
})





/**
 * 1. create registry and move effect definitions to use registry 
 *    (removing Effects, EffectMap, defaultPostProcessingSchema etc in PostProcessing.ts)
 * 
 * 2. register each effect into the registry
 * 
 * 3. refactor PostProcessingComponent to use the registry definitions and mount sub-reactors for each effect
 * 
 * 4. refactor studio editor to use the registry definition's schemas for generating UI
 */

@HexaField HexaField marked this pull request as draft May 29, 2024 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants