Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: angular-threejs/angular-three
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: main
Choose a base ref
...
head repository: angular-threejs/angular-three
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: feat/core-optimizations
Choose a head ref
Checking mergeability… Don’t worry, you can still create the pull request.
  • 4 commits
  • 4 files changed
  • 1 contributor

Commits on May 29, 2025

  1. feat(core): Implement various performance optimizations in core libra… 

    …ries

This commit introduces several optimizations across the core libraries (`events.ts`, `instance.ts`, `loader.ts`) based on a detailed analysis of potential performance improvements.

The following changes have been made:

1.  **`libs/core/src/lib/events.ts` (`intersect` function):**
    *   Optimized the `raycastResults.sort()` method by pre-calculating sort keys (priority, distance, state existence) before sorting. This reduces redundant calls to `getInstanceState` within the sort comparator, improving performance when many raycast results are present.

2.  **`libs/core/src/lib/instance.ts`:**
    *   **`notifyAncestors` function:**
        *   Refactored `notificationCache` from `Map<string, ...>` to `WeakMap<NgtInstanceNode, ...>`, using the instance object itself as the key.
        *   Removed the `queueMicrotask`-based global cache clearing.
        *   This change improves memory safety by preventing potential leaks associated with string-keyed maps and makes cache entry lifecycle management automatic via the `WeakMap` behavior.
    *   **`prepare` function:**
        *   Defined `setPointerEvent`, `addInteraction`, and `removeInteraction` methods directly on the `instance.__ngt__` object literal. This is a minor change for code consistency with other method definitions in the same object.

3.  **`libs/core/src/lib/loader.ts` (`_injectLoader` function):**
    *   Optimized the `effect` hook to reduce redundant processing.
    *   The effect now compares the current normalized URLs with the previously processed ones. If the normalized URLs haven't changed (even if the input signal reference has), further processing (like re-evaluating promises and updating the output signal) is skipped. This avoids unnecessary work when input signals change identity but not effective content.

Unit tests have been added for all these optimizations to ensure correctness and prevent regressions. These tests cover:
*   Correct sorting logic in `events.ts`.
*   Method consistency and notification throttling in `instance.ts`.
*   Conditional execution of loader logic in `loader.ts`.
    @google-labs-jules
    google-labs-jules[bot] committed May 29, 2025
    Configuration menu
    Copy the full SHA
    188f3a1 View commit details
    Browse the repository at this point in the history

  2. refactor(core): Revise optimizations and remove specs 

    This commit incorporates feedback on previous optimizations:

1.  **`libs/core/src/lib/instance.ts` (`prepare` function):**
    *   Reverted the definition of `setPointerEvent`, `addInteraction`, and `removeInteraction` methods to use `Object.defineProperty` and `Object.defineProperties` respectively. This restores their original definition style, ensuring properties like non-enumerability are maintained as intended.

2.  **`libs/core/src/lib/events.ts` (`intersect` function):**
    *   The sorting mechanism for `raycastResults` has been updated to use a `WeakMap`.
    *   Before sorting, a `WeakMap` is populated where keys are the `THREE.Intersection` objects and values are their calculated `priority` and `stateExists` status.
    *   The sort comparator then uses this `WeakMap` to access these pre-calculated values. This approach avoids intermediate array mapping, potentially improving memory efficiency while still preventing redundant `getInstanceState` calls within the sort.

3.  **Spec Files Removed:**
    *   Deleted `libs/core/src/lib/events.spec.ts`.
    *   Deleted `libs/core/src/lib/instance.spec.ts`.
    *   Deleted `libs/core/src/lib/loader.spec.ts`.
    Unit tests will be handled separately as per feedback.

The optimization for `libs/core/src/lib/loader.ts` (`_injectLoader` effect) from the previous set of changes remains in place.
    @google-labs-jules
    google-labs-jules[bot] committed May 29, 2025
    Configuration menu
    Copy the full SHA
    734d4ac View commit details
    Browse the repository at this point in the history

  3. fix(core): Revert WeakMap in notifyAncestors to Map 

    Reverts the caching mechanism in the `notifyAncestors` function
(libs/core/src/lib/instance.ts) back to using `Map<string, NotificationCacheState>`
keyed by instance IDs, instead of a `WeakMap` keyed by instance objects.

This change also reinstates the `queueMicrotask`-based global cache clearing.

The previous `WeakMap` implementation, while aimed at improving memory
management, was found to cause regressions in scenarios where object
identity for instances might not be stable, leading to incorrect
notification throttling. This reversion addresses those regressions.
    @google-labs-jules
    google-labs-jules[bot] committed May 29, 2025
    Configuration menu
    Copy the full SHA
    035e282 View commit details
    Browse the repository at this point in the history

  4. perf(core): Optimize store propagation in renderer utils 

    Optimizes the `propagateStoreRecursively` function in
`libs/core/src/lib/renderer/utils.ts` to reduce redundant calls
to `getInstanceState`.

The function's signature has been updated to accept an optional
`parentInstanceState`. When making recursive calls, the instance state
of the current node (which is the parent in the next call) is now
passed along. The initial call site in `attachThreeNodes` has also
been updated to provide the already fetched parent's instance state.

This change avoids repeatedly looking up the parent's instance state
at each level of the recursion, which can lead to a performance
improvement during scene graph manipulations, especially with deeper
component trees.
    @google-labs-jules
    google-labs-jules[bot] committed May 29, 2025
    Configuration menu
    Copy the full SHA
    eaa6bc3 View commit details
    Browse the repository at this point in the history
Loading