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

fix(react-components): improve asset mapping cache mechanism and refactoring to stabilize rule threshold styling and switching #4652

Merged
merged 33 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
7ea39ca
refactoring to use caching and use treeindex instead of numeric range…
danpriori Jun 14, 2024
246970f
use the asset mappings per model cache to generate the cache per asse…
danpriori Jun 20, 2024
61d5aa4
add cache for node 3d when loading reveal 3d resources to speed up th…
danpriori Jun 24, 2024
71a3dd7
some wip refactoring on cache functions
danpriori Jun 25, 2024
4b848b3
minor refactoring
danpriori Jun 25, 2024
8eb42fa
refactoring and adding cache for assets for no mappings to skip reque…
danpriori Jun 26, 2024
f46889c
cleanup
danpriori Jun 26, 2024
5232873
separate caches for asset and node ids
danpriori Jun 26, 2024
f7baf3b
initial test for triggering callback to switch on off rule base styli…
danpriori Jun 27, 2024
5466801
Merge remote-tracking branch 'origin/master' into danpriori/rule-base…
danpriori Jun 28, 2024
2c1857c
add loading spinner and some ui changes
danpriori Jun 28, 2024
67a8b9d
split into different useEffects hooks to let render the spinner sooner
danpriori Jun 28, 2024
06309dd
dont use spinner on the reset button
danpriori Jun 28, 2024
c48d6f6
cleanup
danpriori Jul 1, 2024
78673b9
lint and remove unused function
danpriori Jul 1, 2024
9a5413e
missed call to show up outputs panel
danpriori Jul 2, 2024
01d9e41
changes from cr
danpriori Jul 3, 2024
fa47fce
changes from cr - splitting out selector component into smaller pieces
danpriori Jul 4, 2024
b04a961
move extract asset id from mapped to the general hooks folder
danpriori Jul 4, 2024
6820e25
move _amountOfAssetIdsChunks to a private readonly parameter
danpriori Jul 4, 2024
511c495
use the correct map key for asset ids
danpriori Jul 4, 2024
06c1f4b
refactoring from cr and using the resource context provider to link s…
danpriori Jul 8, 2024
f0282f2
refactoring and splitting asset mapping caches into smaller classes
danpriori Jul 9, 2024
f4f9d23
lint
danpriori Jul 9, 2024
a123bf9
turn splitChunkInCacheNode3D to private
danpriori Jul 10, 2024
c46f3cb
more refactoring - cr
danpriori Jul 10, 2024
818296a
lint
danpriori Jul 10, 2024
306ca38
removing not used rule based callback
danpriori Jul 10, 2024
2e9ec23
populate the both caches
danpriori Jul 10, 2024
58a592e
move hooks into the hooks sub folder
danpriori Jul 10, 2024
fa90456
Merge remote-tracking branch 'origin/master' into danpriori/rule-base…
danpriori Jul 11, 2024
44c7891
update import for Reveal3DResourcesInfoContext
danpriori Jul 11, 2024
63bf233
minor refactoring to force spinner more stable - still not fully but …
danpriori Jul 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
330 changes: 295 additions & 35 deletions react-components/src/components/CacheProvider/AssetMappingCache.ts
danpriori marked this conversation as resolved.
Show resolved Hide resolved

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,53 @@ const useAssetMappingCache = (): AssetMappingCache => {
return content.cache;
};

export const useGenerateNode3DCache = async (
cadModelOptions: CadModelOptions[],
assetMappings: ModelWithAssetMappings[] | undefined
): Promise<void> => {
const assetMappingCache = useAssetMappingCache();

useMemo(() => {
haakonflatval-cognite marked this conversation as resolved.
Show resolved Hide resolved
cadModelOptions.forEach(async ({ modelId, revisionId }) => {
const assetMapping = assetMappings?.filter(
(item) => item.model.modelId === modelId && item.model.revisionId === revisionId
);
const nodeIdsFromAssetMappings = assetMapping?.flatMap((item) =>
item.assetMappings.map((mapping) => mapping.nodeId)
);

if (nodeIdsFromAssetMappings === undefined || nodeIdsFromAssetMappings.length === 0) return;

await assetMappingCache.generateNode3DCachePerItem(
modelId,
revisionId,
nodeIdsFromAssetMappings
);
});
}, [cadModelOptions, assetMappings]);
};

export const useGenerateAssetMappingCachePerItemFromModelCache = async (
cadModelOptions: CadModelOptions[],
assetMappings: ModelWithAssetMappings[] | undefined
): Promise<void> => {
const assetMappingCache = useAssetMappingCache();
useMemo(() => {
haakonflatval-cognite marked this conversation as resolved.
Show resolved Hide resolved
cadModelOptions.forEach(async ({ modelId, revisionId }) => {
const assetMapping = assetMappings?.filter(
(item) => item.model.modelId === modelId && item.model.revisionId === revisionId
);
if (assetMapping !== undefined && assetMapping.length > 0) {
await assetMappingCache.generateAssetMappingsCachePerItemFromModelCache(
modelId,
revisionId,
assetMapping
);
}
});
}, [cadModelOptions, assetMappings]);
};

export const useAssetMappedNodesForRevisions = (
cadModels: CadModelOptions[]
): UseQueryResult<ModelWithAssetMappings[]> => {
Expand Down
13 changes: 12 additions & 1 deletion react-components/src/components/CacheProvider/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import {
type AnnotationModel,
type AnnotationsBoundingVolume,
type Node3D,
type AnnotationsCogniteAnnotationTypesImagesAssetLink
type AnnotationsCogniteAnnotationTypesImagesAssetLink,
type AssetMapping3D
} from '@cognite/sdk';
import { type DmsUniqueIdentifier, type Source, type EdgeItem } from '../../utilities/FdmSDK';
import { type InModel3dEdgeProperties } from '../../utilities/globalDataModels';
Expand Down Expand Up @@ -64,3 +65,13 @@ export type Image360AnnotationAssetInfo = {
};

export type AnnotationId = number;

export type ChunkInCacheTypes = {
chunkInCache: Array<Required<AssetMapping3D>>;
chunkNotInCache: number[];
};

export type ChunkInCacheTypesNode3D = {
chunkInCache: Node3D[];
chunkNotInCache: number[];
};
haakonflatval-cognite marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,23 @@ import {
import { type ImageCollectionModelStyling } from '../Image360CollectionContainer/useApply360AnnotationStyling';
import { is360ImageAddOptions } from './typeGuards';
import { useRemoveNonReferencedModels } from './useRemoveNonReferencedModels';
import {
useAssetMappedNodesForRevisions,
useGenerateAssetMappingCachePerItemFromModelCache,
useGenerateNode3DCache
} from '../CacheProvider/AssetMappingCacheProvider';

export const Reveal3DResources = ({
resources,
defaultResourceStyling,
instanceStyling,
onResourcesAdded,
onResourceLoadError,
onCallback,
image360Settings
}: Reveal3DResourcesProps): ReactElement => {
const viewer = useReveal();

const [reveal3DModels, setReveal3DModels] = useState<TypedReveal3DModel[]>([]);

const numModelsLoaded = useRef(0);
Expand All @@ -62,6 +69,11 @@ export const Reveal3DResources = ({
[reveal3DModels]
);

const { data: assetMappings } = useAssetMappedNodesForRevisions(cadModelOptions);

void useGenerateAssetMappingCachePerItemFromModelCache(cadModelOptions, assetMappings);
void useGenerateNode3DCache(cadModelOptions, assetMappings);
danpriori marked this conversation as resolved.
Show resolved Hide resolved

const pointCloudModelOptions = useMemo(
() =>
reveal3DModels.filter(
Expand All @@ -70,7 +82,7 @@ export const Reveal3DResources = ({
[reveal3DModels]
);

const styledCadModelOptions = useCalculateCadStyling(
const { styledModels: styledCadModelOptions, modelMappingsIsFetched } = useCalculateCadStyling(
cadModelOptions,
instanceStyling?.filter(isCadAssetMappingStylingGroup) ?? EMPTY_ARRAY,
defaultResourceStyling
Expand Down Expand Up @@ -112,6 +124,12 @@ export const Reveal3DResources = ({
}
};

useEffect(() => {
if (modelMappingsIsFetched && onCallback !== undefined) {
onCallback(modelMappingsIsFetched);
}
}, [modelMappingsIsFetched, onCallback]);

return (
<>
{styledCadModelOptions.map(({ styleGroups, model }, index) => {
Expand Down
1 change: 1 addition & 0 deletions react-components/src/components/Reveal3DResources/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,5 @@ export type CommonResourceContainerProps = {
image360Settings?: CommonImage360Settings;
onResourcesAdded?: () => void;
onResourceLoadError?: (failedResource: AddResourceOptions, error: any) => void;
onCallback?: (data?: any) => void;
haakonflatval-cognite marked this conversation as resolved.
Show resolved Hide resolved
};
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,24 @@ import { RuleBasedSelectionItem } from '../RuleBasedOutputs/components/RuleBased
type RuleBasedOutputsButtonProps = {
onRuleSetStylingChanged?: (stylings: AssetStylingGroup[] | undefined) => void;
onRuleSetSelectedChanged?: (ruleSet: RuleAndEnabled | undefined) => void;
onRuleSetStylingLoaded?: (callback: (isLoaded: boolean) => void) => void;
};
export const RuleBasedOutputsButton = ({
onRuleSetStylingChanged,
onRuleSetSelectedChanged
onRuleSetSelectedChanged,
onRuleSetStylingLoaded
}: RuleBasedOutputsButtonProps): ReactElement => {
const [currentRuleSetEnabled, setCurrentRuleSetEnabled] = useState<RuleAndEnabled>();
const [emptyRuleSelected, setEmptyRuleSelected] = useState<EmptyRuleForSelection>();
const [ruleInstances, setRuleInstances] = useState<RuleAndEnabled[] | undefined>();
const { t } = useTranslation();
const models = use3dModels();
const cadModels = models.filter((model) => model.type === 'cad') as CadModelOptions[];
const { isLoading } = useAssetMappedNodesForRevisions(cadModels);

const { isLoading: isAssetMappingsLoading } = useAssetMappedNodesForRevisions(cadModels);
const [isRuleLoading, setIsRuleLoading] = useState(false);

const [newRuleSetEnabled, setNewRuleSetEnabled] = useState<RuleAndEnabled>();

const ruleInstancesResult = useFetchRuleInstances();

Expand All @@ -43,6 +49,10 @@ export const RuleBasedOutputsButton = ({
setRuleInstances(ruleInstancesResult.data);
}, [ruleInstancesResult]);

useEffect(() => {
setCurrentRuleSetEnabled(newRuleSetEnabled);
}, [newRuleSetEnabled]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this? Could we just use setCurrentRuleSetEnabled instead of setNewRuleSetEnabled below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice observation. But this is to let the spinner start render one event cycle before triggering the process of setting the current rule set enabled on the next cycles.
If we set the flag IsRuleLoading on the same event cycle of setting the CurrentRuleSetEnabled, then the spinner will only shows up a bit later in the middle of the rule processing/when the event flow is free to trigger a new render.
Do you have any other suggestion to do that?

Copy link
Contributor

@haakonflatval-cognite haakonflatval-cognite Jul 4, 2024

Choose a reason for hiding this comment

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

We can have a comment here 👍


const onChange = useCallback(
(data: string | undefined): void => {
ruleInstances?.forEach((item) => {
haakonflatval-cognite marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -72,10 +82,10 @@ export const RuleBasedOutputsButton = ({
if (onRuleSetStylingChanged !== undefined) onRuleSetStylingChanged(undefined);
}

if (onRuleSetSelectedChanged !== undefined) onRuleSetSelectedChanged(selectedRule);
setIsRuleLoading(true);

setEmptyRuleSelected(emptySelection);
setCurrentRuleSetEnabled(selectedRule);
setNewRuleSetEnabled(selectedRule);
},
[ruleInstances, onRuleSetStylingChanged, onRuleSetSelectedChanged]
);
Expand All @@ -87,6 +97,12 @@ export const RuleBasedOutputsButton = ({
if (onRuleSetStylingChanged !== undefined) onRuleSetStylingChanged(assetStylingGroups);
};

const callbackWhenIsLoaded = (isLoaded: boolean): void => {
setIsRuleLoading(!isLoaded);
};

if (onRuleSetStylingLoaded !== undefined) onRuleSetStylingLoaded(callbackWhenIsLoaded);

if (ruleInstances === undefined || ruleInstances.length === 0) {
return <></>;
}
Expand All @@ -99,7 +115,7 @@ export const RuleBasedOutputsButton = ({
appendTo={document.body}>
<Dropdown
placement="right-start"
disabled={isLoading}
disabled={isAssetMappingsLoading}
content={
<Menu
style={{
Expand All @@ -114,6 +130,8 @@ export const RuleBasedOutputsButton = ({
label={t('RULESET_NO_SELECTION', 'No RuleSet selected')}
checked={currentRuleSetEnabled === undefined || emptyRuleSelected?.isEnabled}
onChange={onChange}
isLoading={isRuleLoading}
isEmptyRuleItem={true}
/>
{ruleInstances?.map((item) => (
<RuleBasedSelectionItem
Expand All @@ -122,11 +140,18 @@ export const RuleBasedOutputsButton = ({
label={item?.rule?.properties.name}
checked={item?.isEnabled}
onChange={onChange}
isLoading={isRuleLoading}
isEmptyRuleItem={false}
/>
))}
</Menu>
}>
<Button icon="ColorPalette" aria-label="Select RuleSet" type="ghost" />
<Button
disabled={isAssetMappingsLoading}
icon="ColorPalette"
aria-label="Select RuleSet"
type="ghost"
/>
</Dropdown>
</CogsTooltip>
{ruleInstances !== undefined && ruleInstances?.length > 0 && (
Expand Down
Loading
Loading