Skip to content

Commit d41fc91

Browse files
committed
fix(soba): clean up a lot of afterNextRender / autoEffect
- Remove a lot of afterNextRender and autoEffect usages. This is due to misunderstanding of when inputs are resolved. The initial train of thought was to use afterNextRender to ensure Inputs are resolved in effects. However, this is not necessarily true because it depends on when the signals (signals, and computed) are read. effect() is mostly a safe place to start reacting to inputs; the template itself is an effect so as long as the result of CIFs are used on the template; we are safe with the timing of inputs - code is made more consistent with register deps up front and return early if possible.
1 parent 0e27857 commit d41fc91

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+821
-987
lines changed

libs/soba/abstractions/src/lib/edges.ts

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import { afterNextRender, ChangeDetectionStrategy, Component, computed, input, viewChild } from '@angular/core';
1+
import { ChangeDetectionStrategy, Component, computed, effect, input, viewChild } from '@angular/core';
22
import { checkNeedsUpdate, getLocalState, NgtMesh, omit } from 'angular-three';
3-
import { injectAutoEffect } from 'ngxtension/auto-effect';
43
import { mergeInputs } from 'ngxtension/inject-inputs';
54
import { BufferAttribute, BufferGeometry, ColorRepresentation, EdgesGeometry, Mesh } from 'three';
65
import { LineMaterialParameters } from 'three-stdlib';
@@ -45,35 +44,32 @@ export class NgtsEdges {
4544
private memoizedThreshold?: number;
4645

4746
constructor() {
48-
const autoEffect = injectAutoEffect();
49-
afterNextRender(() => {
50-
autoEffect(() => {
51-
const line = this.line().lineRef()?.nativeElement;
52-
if (!line) return;
47+
effect(() => {
48+
const line = this.line().lineRef()?.nativeElement;
49+
if (!line) return;
5350

54-
const lS = getLocalState(line);
55-
if (!lS) return;
51+
const lS = getLocalState(line);
52+
if (!lS) return;
5653

57-
const parent = lS.parent() as Mesh;
58-
if (!parent) return;
54+
const parent = lS.parent() as Mesh;
55+
if (!parent) return;
5956

60-
const { geometry: explicitGeometry, threshold } = this.options();
61-
const geometry = explicitGeometry ?? parent.geometry;
62-
if (!geometry) return;
57+
const { geometry: explicitGeometry, threshold } = this.options();
58+
const geometry = explicitGeometry ?? parent.geometry;
59+
if (!geometry) return;
6360

64-
const cached = this.memoizedGeometry === geometry && this.memoizedThreshold === threshold;
65-
if (cached) return;
61+
const cached = this.memoizedGeometry === geometry && this.memoizedThreshold === threshold;
62+
if (cached) return;
6663

67-
this.memoizedGeometry = geometry;
68-
this.memoizedThreshold = threshold;
64+
this.memoizedGeometry = geometry;
65+
this.memoizedThreshold = threshold;
6966

70-
const points = (new EdgesGeometry(geometry, threshold).attributes['position'] as BufferAttribute)
71-
.array as Float32Array;
72-
line.geometry.setPositions(points);
73-
checkNeedsUpdate(line.geometry.attributes['instanceStart']);
74-
checkNeedsUpdate(line.geometry.attributes['instanceEnd']);
75-
line.computeLineDistances();
76-
});
67+
const points = (new EdgesGeometry(geometry, threshold).attributes['position'] as BufferAttribute)
68+
.array as Float32Array;
69+
line.geometry.setPositions(points);
70+
checkNeedsUpdate(line.geometry.attributes['instanceStart']);
71+
checkNeedsUpdate(line.geometry.attributes['instanceEnd']);
72+
line.computeLineDistances();
7773
});
7874
}
7975
}

libs/soba/abstractions/src/lib/helper.ts

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ import {
77
ElementRef,
88
Injector,
99
input,
10-
signal,
11-
untracked,
1210
viewChild,
1311
} from '@angular/core';
1412
import { extend, getLocalState, injectBeforeRender, injectStore, resolveRef } from 'angular-three';
@@ -32,30 +30,28 @@ export function injectHelper<
3230
const store = injectStore();
3331
const scene = store.select('scene');
3432

35-
const helper = signal<THelperInstance | null>(null);
36-
37-
effect((onCleanup) => {
38-
let currentHelper: THelperInstance | undefined = undefined;
33+
const helper = computed(() => {
3934
const maybeObject3D = object();
40-
if (!maybeObject3D) return;
35+
if (!maybeObject3D) return null;
4136

4237
const object3D = resolveRef(maybeObject3D);
43-
if (!object3D) return;
38+
if (!object3D) return null;
39+
40+
const currentHelper: THelperInstance = new (helperConstructor() as any)(object3D, ...args());
41+
currentHelper.traverse((child) => (child.raycast = () => null));
42+
return currentHelper;
43+
});
4444

45-
currentHelper = new (helperConstructor() as any)(object3D, ...args());
45+
effect((onCleanup) => {
46+
const currentHelper = helper();
4647
if (!currentHelper) return;
4748

48-
untracked(() => {
49-
helper.set(currentHelper);
50-
});
49+
const _scene = scene();
5150

52-
// Prevent the helpers from blocking rays
53-
currentHelper.traverse((child) => (child.raycast = () => null));
54-
scene().add(currentHelper);
51+
_scene.add(currentHelper);
5552

5653
onCleanup(() => {
57-
helper.set(null);
58-
scene().remove(currentHelper);
54+
_scene.remove(currentHelper);
5955
currentHelper.dispose?.();
6056
});
6157
});
@@ -65,7 +61,7 @@ export function injectHelper<
6561
if (currentHelper) currentHelper.update();
6662
});
6763

68-
return helper.asReadonly();
64+
return helper;
6965
});
7066
}
7167

libs/soba/abstractions/src/lib/line.ts

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
import {
2-
afterNextRender,
32
ChangeDetectionStrategy,
43
Component,
54
computed,
65
CUSTOM_ELEMENTS_SCHEMA,
6+
effect,
77
ElementRef,
88
input,
99
viewChild,
1010
} from '@angular/core';
1111
import { checkNeedsUpdate, injectStore, NgtAfterAttach, NgtArgs, NgtObject3DNode, omit, pick } from 'angular-three';
12-
import { injectAutoEffect } from 'ngxtension/auto-effect';
1312
import { mergeInputs } from 'ngxtension/inject-inputs';
1413
import { Color, ColorRepresentation, Vector2, Vector3, Vector4 } from 'three';
1514
import {
@@ -55,7 +54,7 @@ const defaultOptions: NgtsLineOptions = {
5554
[color]="color()"
5655
[vertexColors]="vertex()"
5756
[resolution]="resolution()"
58-
[linewidth]="linewidth() ?? lineWidth() ?? 1"
57+
[linewidth]="actualLineWidth()"
5958
[dashed]="dashed()"
6059
[transparent]="itemSize() === 4"
6160
[parameters]="parameters()"
@@ -80,17 +79,19 @@ export class NgtsLine {
8079
private segments = pick(this.options, 'segments');
8180
private vertexColors = pick(this.options, 'vertexColors');
8281

83-
dashed = pick(this.options, 'dashed');
84-
color = pick(this.options, 'color');
85-
lineWidth = pick(this.options, 'lineWidth');
86-
linewidth = pick(this.options, 'linewidth');
87-
vertex = computed(() => Boolean(this.vertexColors()));
88-
resolution = computed(() => [this.size().width, this.size().height]);
82+
protected dashed = pick(this.options, 'dashed');
83+
protected color = pick(this.options, 'color');
84+
protected vertex = computed(() => Boolean(this.vertexColors()));
85+
protected resolution = computed(() => [this.size().width, this.size().height]);
86+
87+
private lineWidth = pick(this.options, 'lineWidth');
88+
private linewidth = pick(this.options, 'linewidth');
8989

9090
line2 = computed(() => (this.segments() ? new LineSegments2() : new Line2()));
9191
lineMaterial = new LineMaterial();
9292

93-
itemSize = computed(() => ((this.vertexColors()?.[0] as number[] | undefined)?.length === 4 ? 4 : 3));
93+
protected actualLineWidth = computed(() => this.linewidth() ?? this.lineWidth() ?? 1);
94+
protected itemSize = computed(() => ((this.vertexColors()?.[0] as number[] | undefined)?.length === 4 ? 4 : 3));
9495

9596
lineGeometry = computed(() => {
9697
const geom = this.segments() ? new LineSegmentsGeometry() : new LineGeometry();
@@ -126,25 +127,21 @@ export class NgtsLine {
126127
}
127128

128129
constructor() {
129-
const autoEffect = injectAutoEffect();
130-
131-
afterNextRender(() => {
132-
autoEffect(() => {
133-
const [lineMaterial, dashed] = [this.lineMaterial, this.dashed()];
134-
if (dashed) {
135-
lineMaterial.defines['USE_DASH'] = '';
136-
} else {
137-
delete lineMaterial.defines['USE_DASH'];
138-
}
139-
checkNeedsUpdate(lineMaterial);
140-
});
130+
effect(() => {
131+
const [lineMaterial, dashed] = [this.lineMaterial, this.dashed()];
132+
if (dashed) {
133+
lineMaterial.defines['USE_DASH'] = '';
134+
} else {
135+
delete lineMaterial.defines['USE_DASH'];
136+
}
137+
checkNeedsUpdate(lineMaterial);
138+
});
141139

142-
autoEffect(() => {
143-
const [lineGeometry, lineMaterial] = [this.lineGeometry(), this.lineMaterial];
144-
return () => {
145-
lineGeometry.dispose();
146-
lineMaterial.dispose();
147-
};
140+
effect((onCleanup) => {
141+
const [lineGeometry, lineMaterial] = [this.lineGeometry(), this.lineMaterial];
142+
onCleanup(() => {
143+
lineGeometry.dispose();
144+
lineMaterial.dispose();
148145
});
149146
});
150147
}

libs/soba/abstractions/src/lib/rounded-box.ts

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
import {
2-
afterNextRender,
32
ChangeDetectionStrategy,
43
Component,
54
computed,
65
CUSTOM_ELEMENTS_SCHEMA,
6+
effect,
77
ElementRef,
88
input,
99
untracked,
1010
viewChild,
1111
} from '@angular/core';
1212
import { extend, NgtArgs, NgtMesh, omit, pick } from 'angular-three';
13-
import { injectAutoEffect } from 'ngxtension/auto-effect';
1413
import { mergeInputs } from 'ngxtension/inject-inputs';
1514
import { ExtrudeGeometry, Mesh, Shape } from 'three';
1615
import { toCreasedNormals } from 'three-stdlib';
@@ -87,11 +86,11 @@ export class NgtsRoundedBox {
8786
meshRef = viewChild.required<ElementRef<Mesh>>('mesh');
8887
geometryRef = viewChild<ElementRef<ExtrudeGeometry>>('geometry');
8988

90-
shape = computed(() => {
89+
protected shape = computed(() => {
9190
const [width, height, radius] = [this.width(), this.height(), this.radius()];
9291
return createShape(width, height, radius);
9392
});
94-
params = computed(() => {
93+
protected params = computed(() => {
9594
const [depth, radius, smoothness, bevelSegments, steps] = [
9695
this.depth(),
9796
this.radius(),
@@ -114,18 +113,12 @@ export class NgtsRoundedBox {
114113
constructor() {
115114
extend({ ExtrudeGeometry, Mesh });
116115

117-
const autoEffect = injectAutoEffect();
118-
afterNextRender(() => {
119-
autoEffect(() => {
120-
const geometry = this.geometryRef()?.nativeElement;
121-
if (!geometry) return;
116+
effect(() => {
117+
const geometry = this.geometryRef()?.nativeElement;
118+
if (!geometry) return;
122119

123-
this.shape();
124-
this.params();
125-
126-
geometry.center();
127-
toCreasedNormals(geometry, untracked(this.creaseAngle));
128-
});
120+
geometry.center();
121+
toCreasedNormals(geometry, untracked(this.creaseAngle));
129122
});
130123
}
131124
}

libs/soba/abstractions/src/lib/text-3d.ts

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
import {
2-
afterNextRender,
32
ChangeDetectionStrategy,
43
Component,
54
computed,
65
CUSTOM_ELEMENTS_SCHEMA,
6+
effect,
77
ElementRef,
88
input,
99
viewChild,
1010
} from '@angular/core';
1111
import { extend, NgtArgs, NgtMesh, omit, pick } from 'angular-three';
1212
import { injectFont, NgtsFontInput } from 'angular-three-soba/loaders';
13-
import { injectAutoEffect } from 'ngxtension/auto-effect';
1413
import { mergeInputs } from 'ngxtension/inject-inputs';
1514
import { Mesh } from 'three';
1615
import { mergeVertices, TextGeometry, TextGeometryParameters } from 'three-stdlib';
@@ -50,7 +49,7 @@ export class NgtsText3D {
5049
font = input.required<NgtsFontInput>();
5150
text = input.required<string>();
5251
options = input(defaultOptions, { transform: mergeInputs(defaultOptions) });
53-
parameters = omit(this.options, [
52+
protected parameters = omit(this.options, [
5453
'letterSpacing',
5554
'lineHeight',
5655
'size',
@@ -90,17 +89,15 @@ export class NgtsText3D {
9089
constructor() {
9190
extend({ Mesh, TextGeometry });
9291

93-
const autoEffect = injectAutoEffect();
92+
effect(() => {
93+
const [mesh, textArgs] = [this.meshRef()?.nativeElement, this.textArgs()];
94+
if (!textArgs || !mesh) return;
9495

95-
afterNextRender(() => {
96-
autoEffect(() => {
97-
const [mesh, smooth, textArgs] = [this.meshRef()?.nativeElement, this.smooth(), this.textArgs()];
98-
if (!textArgs || !mesh) return;
99-
if (smooth) {
100-
mesh.geometry = mergeVertices(mesh.geometry, smooth);
101-
mesh.geometry.computeVertexNormals();
102-
}
103-
});
96+
const smooth = this.smooth();
97+
if (smooth) {
98+
mesh.geometry = mergeVertices(mesh.geometry, smooth);
99+
mesh.geometry.computeVertexNormals();
100+
}
104101
});
105102
}
106103
}

libs/soba/abstractions/src/lib/text.ts

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,12 @@ import {
44
ChangeDetectionStrategy,
55
Component,
66
DestroyRef,
7-
afterNextRender,
7+
effect,
88
inject,
99
input,
1010
output,
1111
} from '@angular/core';
1212
import { NgtArgs, NgtMesh, injectStore, omit, pick } from 'angular-three';
13-
import { injectAutoEffect } from 'ngxtension/auto-effect';
1413
import { mergeInputs } from 'ngxtension/inject-inputs';
1514
import { ColorRepresentation } from 'three';
1615
// @ts-expect-error - no type def
@@ -84,7 +83,6 @@ export class NgtsText {
8483

8584
synced = output<Text>();
8685

87-
private autoEffect = injectAutoEffect();
8886
private store = injectStore();
8987
private invalidate = this.store.select('invalidate');
9088

@@ -103,21 +101,18 @@ export class NgtsText {
103101
this.troikaMesh.dispose();
104102
});
105103

106-
// NOTE: this could be just effect but autoEffect is used for consistency
107-
this.autoEffect(() => {
104+
effect(() => {
108105
const [font, characters, invalidate] = [this.font(), this.characters(), this.invalidate()];
109106
if (font) {
110107
preloadFont({ font, characters }, () => invalidate());
111108
}
112109
});
113110

114-
afterNextRender(() => {
115-
this.autoEffect(() => {
116-
const [invalidate] = [this.invalidate(), this.text(), this.options()];
117-
this.troikaMesh.sync(() => {
118-
invalidate();
119-
this.synced.emit(this.troikaMesh);
120-
});
111+
effect(() => {
112+
const [invalidate] = [this.invalidate(), this.text(), this.options()];
113+
this.troikaMesh.sync(() => {
114+
invalidate();
115+
this.synced.emit(this.troikaMesh);
121116
});
122117
});
123118
}

0 commit comments

Comments
 (0)