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

feat: remove animation canvas entirely | move to animation package #1457

Open
wants to merge 46 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
83f7242
rebase:
shanimal08 Jan 30, 2025
0f18c42
rm gl from licence ignore
shanimal08 Jan 31, 2025
87468b7
ci: update env name
shanimal08 Jan 31, 2025
17e808a
fix format
shanimal08 Jan 31, 2025
4d8e762
rm quotes
shanimal08 Jan 31, 2025
b82ca2d
rm npmrc and ci flows for npm auth
shanimal08 Jan 31, 2025
59f5128
wip: use public package
shanimal08 Jan 31, 2025
dd937de
wip: use latest pkg
shanimal08 Jan 31, 2025
47d5fda
Merge branch 'main' into test/ts_ani
shanimal08 Jan 31, 2025
f3278bf
fix lint now that types work
shanimal08 Jan 31, 2025
0971a29
merge
shanimal08 Feb 3, 2025
eed0549
Merge branch 'main' into test/ts_ani
shanimal08 Feb 4, 2025
062a304
Merge branch 'main' into test/ts_ani
shanimal08 Feb 4, 2025
706ae6c
Merge branch 'main' into test/ts_ani
shanimal08 Feb 4, 2025
f9b4942
merge+bump
shanimal08 Feb 4, 2025
59cf36e
fix props
shanimal08 Feb 4, 2025
3b98ea1
merge:fix conflict
shanimal08 Feb 5, 2025
bbbdbff
merge
shanimal08 Feb 7, 2025
cd60324
use latest
shanimal08 Feb 7, 2025
b04c009
merge
shanimal08 Feb 7, 2025
2327320
fix after merge
shanimal08 Feb 7, 2025
00527fc
remove opacity change on remove
shanimal08 Feb 7, 2025
4aea092
fresh install
shanimal08 Feb 7, 2025
071d12b
merge
shanimal08 Feb 10, 2025
a26c9b9
Merge branch 'main' into test/ts_ani
brianp Feb 10, 2025
0fd5248
review suggestions + npm install for pkg lock
shanimal08 Feb 10, 2025
957ccb2
review changes: toggle improvements
shanimal08 Feb 10, 2025
045a732
Merge branch 'main' into test/ts_ani
shanimal08 Feb 11, 2025
1d9366b
Merge branch 'main' into test/ts_ani
shanimal08 Feb 11, 2025
227040e
Merge branch 'main' into test/ts_ani
shanimal08 Feb 11, 2025
d0f913e
merge: resolve conflicts
shanimal08 Feb 11, 2025
77020ad
fix after merge
shanimal08 Feb 11, 2025
b27d0af
merge: conflicts
shanimal08 Feb 11, 2025
b1240eb
Merge branch 'main' into test/ts_ani
shanimal08 Feb 12, 2025
c2dfff0
fix cargo fmt
shanimal08 Feb 12, 2025
268acc6
Merge branch 'main' into test/ts_ani
shanimal08 Feb 12, 2025
61c6999
use updated tower version + clean up load calls + visual toggle loading
shanimal08 Feb 12, 2025
260dfb6
Merge branch 'main' into test/ts_ani
shanimal08 Feb 13, 2025
7aea559
Merge branch 'main' into test/ts_ani
shanimal08 Feb 13, 2025
ab1c5c9
merge: resolve conflicts
shanimal08 Feb 14, 2025
6c89292
Merge branch 'main' into test/ts_ani
shanimal08 Feb 14, 2025
59cc0ad
Merge branch 'main' into test/ts_ani
shanimal08 Feb 17, 2025
58dce49
Merge branch 'main' into test/ts_ani
shanimal08 Feb 17, 2025
40f5e4a
Merge branch 'main' into test/ts_ani
shanimal08 Feb 18, 2025
9f743b1
Merge branch 'main' into test/ts_ani
shanimal08 Feb 20, 2025
267fbd9
Merge branch 'main' into test/ts_ani
shanimal08 Feb 21, 2025
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
1 change: 0 additions & 1 deletion .license.ignore
Original file line number Diff line number Diff line change
@@ -1 +0,0 @@
./public/assets/glApp.js
71 changes: 3 additions & 68 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
crossorigin
/>

<script type="module" crossorigin src="/assets/glApp.js"></script>
<title>Tari Universe | Testnet</title>
<style>
/* Poppins Font Faces */
Expand Down Expand Up @@ -182,78 +181,14 @@
padding: 0;
margin: 0;
}

#canvas {
#tower-canvas {
position: absolute;
pointer-events: auto;
}
Comment on lines +184 to 187
Copy link

@coderabbitai coderabbitai bot Feb 11, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Ensure proper cleanup of tower-canvas element.

Given the reported memory leak issues when toggling visual mode (memory usage increasing from 800MB to 2GB), ensure that the canvas element is properly disposed of when visual mode is disabled. Consider implementing the following:

  1. Remove all WebGL contexts and resources
  2. Remove event listeners
  3. Clear the canvas element from DOM

To help diagnose the memory leak, consider running this script to analyze canvas cleanup:


🏁 Script executed:

#!/bin/bash
# Description: Search for canvas cleanup code in the codebase

# Look for canvas cleanup patterns
echo "Searching for canvas cleanup patterns..."
rg -A 5 "removeChild|remove\(|dispose|destroy|cleanup" --type ts

# Look for event listener cleanup
echo "Searching for event listener cleanup..."
rg -A 5 "removeEventListener" --type ts

# Look for WebGL context cleanup
echo "Searching for WebGL context cleanup..."
rg -A 5 "getContext\(['\"]webgl['\"]|loseContext|delete" --type ts

Length of output: 3407


Potential Memory Leak Issue with the tower-canvas Element

The search results show no explicit cleanup for the tower-canvas element, particularly for its WebGL context. To avoid memory leaks when toggling visual mode, please ensure that:

  • WebGL Context Disposal: Invoke appropriate methods (e.g., context loss or explicit disposal functions) to properly free WebGL resources.
  • Event Listener Removal: Detach any event listeners attached to the canvas.
  • DOM Cleanup: Remove the canvas element from the DOM when visual mode is disabled.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

DOM Cleanup: Remove the canvas element from the DOM when visual mode is disabled.

@coderabbitai that's the whole point of this PR 😭 - the removal of the canvas is handled in the @tari-project/tari-tower package

Copy link

Choose a reason for hiding this comment

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

@shanimal08 I apologize for the redundant suggestion. You're absolutely right - the canvas cleanup is already being handled by the @tari-project/tari-tower package, which is precisely what this PR is about. Thank you for the clarification.


✏️ Learnings added
Learnt from: shanimal08
PR: tari-project/universe#1457
File: index.html:184-187
Timestamp: 2025-02-12T07:12:44.217Z
Learning: The canvas cleanup and DOM manipulation for the tower animation is handled by the `@tari-project/tari-tower` package, which completely removes the canvas element when visual mode is disabled.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

</style>
</head>
<body>
<main id="main">
<div id="root"></div>
<canvas id="canvas"></canvas>
<script type="module" src="src/main.tsx"></script>
<script>
let time;
let lastRender;
let targetFPS = 40;
let frameInterval = 1 / targetFPS;

function preload() {
glApp.preload(
{
canvas: document.getElementById('canvas'),
orbitTarget: document.getElementById('canvas'),
ASSETS_PATH: '/assets/',
},
() => {
init();
}
);
}

function init() {
glApp.init();
time = performance.now() / 1000;
lastRender = time;
window.addEventListener('resize', onResize);

onResize();
animate();
}

function animate() {
let newTime = performance.now() / 1000;
let dt = newTime - time;
if (newTime - lastRender >= frameInterval) {
lastRender = newTime;
update(dt);
time = newTime;
}

requestAnimationFrame(animate);
}

function update(dt) {
glApp.render(dt);
}

function onResize() {
const sidebarOffset = 348 + 20; // sidebar + padding
glApp.properties.cameraOffsetX = sidebarOffset / window.innerWidth;
glApp.setSize(window.innerWidth, window.innerHeight);
}

document.addEventListener('DOMContentLoaded', () => {
if (!window.WebGL2RenderingContext && !window.WebGLRenderingContext) {
console.error('WebGL not supported!');
return;
}

preload();
});
</script>
</main>
<div id="root"></div>
<script type="module" src="src/main.tsx"></script>
</body>
</html>
207 changes: 116 additions & 91 deletions package-lock.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"@floating-ui/react": "^0.27.4",
"@lottiefiles/dotlottie-react": "^0.13.0",
"@sentry/react": "^8.48.0",
"@tari-project/tari-tower": "^0.0.14",
"@tauri-apps/api": "^2.2.0",
"@tauri-apps/plugin-clipboard-manager": "^2.2.0",
"@tauri-apps/plugin-os": "^2.2.0",
Expand Down
4,732 changes: 0 additions & 4,732 deletions public/assets/glApp.js

This file was deleted.

Binary file removed public/assets/models/BASE.buf
Binary file not shown.
Binary file removed public/assets/models/BOX.buf
Binary file not shown.
Binary file removed public/assets/models/COIN.buf
Binary file not shown.
Binary file removed public/assets/models/COIN_PLACEMENT.buf
Binary file not shown.
Binary file removed public/assets/models/lose_animation.buf
Binary file not shown.
Binary file removed public/assets/textures/LDR_RGB1_0.png
Binary file not shown.
8 changes: 0 additions & 8 deletions public/assets/textures/LICENSES.txt

This file was deleted.

Binary file removed public/assets/textures/gobo.jpg
Binary file not shown.
Binary file removed public/assets/textures/matcap_gold.jpg
Binary file not shown.
13 changes: 2 additions & 11 deletions src/App/App.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as Sentry from '@sentry/react';
import { useEffect } from 'react';
import { AppContentContainer } from '@app/App/App.styles';
import { useShuttingDown } from '@app/hooks';
Expand Down Expand Up @@ -28,19 +27,11 @@ export default function App() {

useEffect(() => {
if (!window.WebGL2RenderingContext && !window.WebGLRenderingContext) {
Sentry.captureMessage('WebGL not supported by the browser', { extra: { userAgent: navigator.userAgent } });
console.error('WebGL not supported by the browser.', 'userAgent:', navigator.userAgent);
setIsWebglNotSupported(true);
setError(t('webgl-not-supported'));
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

useEffect(() => {
const canvasElement = document.getElementById('canvas');
if (canvasElement) {
canvasElement.style.opacity = isShuttingDown || isSettingUp ? '0' : '1';
}
}, [isShuttingDown, isSettingUp]);
}, [setError, setIsWebglNotSupported, t]);

const showSetup = isSettingUp && !isShuttingDown && isAppReady;
const showMainView = !isSettingUp && !isShuttingDown && isAppReady;
Expand Down
49 changes: 39 additions & 10 deletions src/containers/main/Dashboard/components/VisualMode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import { useCallback } from 'react';
import { useTranslation } from 'react-i18next';
import styled from 'styled-components';
import { ToggleSwitch } from '@app/components/elements/ToggleSwitch.tsx';
import { useAppConfigStore } from '@app/store/useAppConfigStore';
import { useUIStore } from '@app/store/useUIStore';
import { setVisualMode, useAppConfigStore } from '@app/store/useAppConfigStore';
import { sidebarTowerOffset, TOWER_CANVAS_ID, useUIStore } from '@app/store/useUIStore';
import { Typography } from '@app/components/elements/Typography';
import {
SettingsGroup,
Expand All @@ -13,23 +13,49 @@ import {
SettingsGroupWrapper,
} from '@app/containers/floating/Settings/components/SettingsGroup.styles';

import { loadTowerAnimation, removeTowerAnimation, setAnimationState } from '@tari-project/tari-tower';

export const ErrorTypography = styled(Typography)(({ theme }) => ({
color: theme.palette.error.main,
}));

function VisualMode() {
const visualMode = useAppConfigStore((s) => s.visual_mode);
const setVisualMode = useAppConfigStore((s) => s.setVisualMode);
const visualModeToggleLoading = useAppConfigStore((s) => s.visualModeToggleLoading);
const isWebglNotSupported = useUIStore((s) => s.isWebglNotSupported);
const { t } = useTranslation('settings', { useSuspense: false });

const handleDisable = useCallback(() => {
setVisualMode(false);
removeTowerAnimation({ canvasId: TOWER_CANVAS_ID })
.then(() => {
// Force garbage collection to clean up WebGL context
if (window.gc) window.gc();
})
.catch((e) => {
console.error('Could not disable visual mode. Error at loadTowerAnimation:', e);
setVisualMode(true);
});
}, []);
const handleEnable = useCallback(() => {
loadTowerAnimation({ canvasId: TOWER_CANVAS_ID, offset: sidebarTowerOffset })
.then(() => {
setVisualMode(true);
setAnimationState('showVisual');
})
.catch((e) => {
console.error('Could not enable visual mode. Error at loadTowerAnimation:', e);
setVisualMode(false);
});
}, []);
Comment on lines +42 to +52
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Monitor memory usage during animation loading.

Based on the PR comments about memory leaks, add memory usage monitoring during animation loading.

 const handleEnable = useCallback(() => {
+    const initialMemory = window.performance.memory?.usedJSHeapSize;
+    
     loadTowerAnimation({ canvasId: TOWER_CANVAS_ID, offset: sidebarTowerOffset })
         .then(() => {
             setVisualMode(true);
             setAnimationState('showVisual');
+            
+            // Monitor memory usage
+            const currentMemory = window.performance.memory?.usedJSHeapSize;
+            if (currentMemory && initialMemory && currentMemory - initialMemory > 500 * 1024 * 1024) {
+                console.warn('High memory usage detected after enabling visual mode');
+            }
         })
         .catch((e) => {
             console.error('Could not enable visual mode. Error at loadTowerAnimation:', e);
             setVisualMode(false);
         });
 }, []);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleEnable = useCallback(() => {
loadTowerAnimation({ canvasId: TOWER_CANVAS_ID, offset: sidebarTowerOffset })
.then(() => {
setVisualMode(true);
setAnimationState('showVisual');
})
.catch((e) => {
console.error('Could not enable visual mode. Error at loadTowerAnimation:', e);
setVisualMode(false);
});
}, []);
const handleEnable = useCallback(() => {
const initialMemory = window.performance.memory?.usedJSHeapSize;
loadTowerAnimation({ canvasId: TOWER_CANVAS_ID, offset: sidebarTowerOffset })
.then(() => {
setVisualMode(true);
setAnimationState('showVisual');
// Monitor memory usage
const currentMemory = window.performance.memory?.usedJSHeapSize;
if (currentMemory && initialMemory && currentMemory - initialMemory > 500 * 1024 * 1024) {
console.warn('High memory usage detected after enabling visual mode');
}
})
.catch((e) => {
console.error('Could not enable visual mode. Error at loadTowerAnimation:', e);
setVisualMode(false);
});
}, []);


const handleSwitch = useCallback(() => {
const canvasElement = document.getElementById('canvas');
if (canvasElement) {
canvasElement.style.display = visualMode ? 'none' : 'block';
if (visualMode) {
handleDisable();
} else {
handleEnable();
}
setVisualMode(!visualMode);
}, [setVisualMode, visualMode]);
}, [handleDisable, handleEnable, visualMode]);
Comment on lines 54 to +60
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add debounce to prevent rapid toggling.

Based on the PR comments about memory issues, rapid toggling should be prevented.

+import { debounce } from 'lodash';
+
-const handleSwitch = useCallback(() => {
+const handleSwitch = useCallback(
+    debounce(() => {
         if (visualMode) {
             handleDisable();
         } else {
             handleEnable();
         }
-    }, [handleDisable, handleEnable, visualMode]);
+    }, 1000),
+    [handleDisable, handleEnable, visualMode],
+);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleSwitch = useCallback(() => {
const canvasElement = document.getElementById('canvas');
if (canvasElement) {
canvasElement.style.display = visualMode ? 'none' : 'block';
if (visualMode) {
handleDisable();
} else {
handleEnable();
}
setVisualMode(!visualMode);
}, [setVisualMode, visualMode]);
}, [handleDisable, handleEnable, visualMode]);
import { debounce } from 'lodash';
const handleSwitch = useCallback(
debounce(() => {
if (visualMode) {
handleDisable();
} else {
handleEnable();
}
}, 1000),
[handleDisable, handleEnable, visualMode],
);


return (
<SettingsGroupWrapper>
Expand All @@ -41,11 +67,14 @@ function VisualMode() {
{isWebglNotSupported && <ErrorTypography color="error">{t('webgl-not-supported')}</ErrorTypography>}
</SettingsGroupContent>
<SettingsGroupAction style={{ alignItems: 'center' }}>
<ToggleSwitch disabled={isWebglNotSupported} checked={visualMode} onChange={handleSwitch} />
<ToggleSwitch
disabled={visualModeToggleLoading || isWebglNotSupported}
checked={visualMode}
onChange={handleSwitch}
/>
</SettingsGroupAction>
</SettingsGroup>
</SettingsGroupWrapper>
);
}

export default VisualMode;
72 changes: 0 additions & 72 deletions src/glApp.d.ts

This file was deleted.

10 changes: 0 additions & 10 deletions src/global.d.ts

This file was deleted.

42 changes: 33 additions & 9 deletions src/hooks/mining/useMiningUiStateMachine.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,57 @@
import { useAppStateStore } from '@app/store/appStateStore';
import { useMiningStore } from '@app/store/useMiningStore';
import { setAnimationState } from '@app/visuals';
import { useEffect } from 'react';
import { setAnimationState, animationStatus } from '@tari-project/tari-tower';
import { useMiningMetricsStore } from '@app/store/useMiningMetricsStore.ts';
import { useAppConfigStore } from '@app/store/useAppConfigStore.ts';

export const useUiMiningStateMachine = () => {
const isMiningInitiated = useMiningStore((s) => s.miningInitiated);
const isChangingMode = useMiningStore((s) => s.isChangingMode);
const cpuIsMining = useMiningMetricsStore((s) => s.cpu_mining_status.is_mining);
const gpuIsMining = useMiningMetricsStore((s) => s.gpu_mining_status.is_mining);
const setupComplete = useAppStateStore((s) => s.setupComplete);
const visualMode = useAppConfigStore((s) => s.visual_mode);
const visualModeToggleLoading = useAppConfigStore((s) => s.visualModeToggleLoading);
const isMining = cpuIsMining || gpuIsMining;

const statusIndex = window?.glApp?.stateManager?.statusIndex;
const indexTrigger = animationStatus;

useEffect(() => {
const status = window?.glApp?.stateManager?.status;
const notStarted = !status || status == 'not-started' || status == 'stop';
let isLatestEffect = true;
if (!visualMode || visualModeToggleLoading) return;
const notStarted = !animationStatus || animationStatus == 'not-started';
if (isMining && notStarted) {
setAnimationState('start');
// Debounce animation state changes
const timer = setTimeout(() => {
if (isLatestEffect) {
setAnimationState('start');
}
}, 300);
return () => {
clearTimeout(timer);
isLatestEffect = false;
};
}
}, [statusIndex, isMining]);
}, [indexTrigger, isMining, visualMode, visualModeToggleLoading]);

useEffect(() => {
const notStopped = window?.glApp?.stateManager?.status !== 'not-started';
let isLatestEffect = true;
if (!visualMode || visualModeToggleLoading) return;
const notStopped = animationStatus !== 'not-started';
const preventStop = !setupComplete || isMiningInitiated || isChangingMode;
const shouldStop = !isMining && notStopped && !preventStop;
if (shouldStop) {
setAnimationState('stop');
// Debounce animation state changes
const timer = setTimeout(() => {
if (isLatestEffect) {
setAnimationState('stop');
}
}, 300);
return () => {
clearTimeout(timer);
isLatestEffect = false;
};
}
}, [statusIndex, setupComplete, isMiningInitiated, isMining, isChangingMode]);
}, [indexTrigger, setupComplete, isMiningInitiated, isMining, isChangingMode, visualMode, visualModeToggleLoading]);
};
7 changes: 6 additions & 1 deletion src/main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@ import { StrictMode } from 'react';
import { createRoot } from 'react-dom/client';
import AppWrapper from './App/AppWrapper.tsx';

const root = createRoot(document.getElementById('root') as HTMLElement);
document.addEventListener('DOMContentLoaded', () => {
if (!window.WebGL2RenderingContext && !window.WebGLRenderingContext) {
console.error('WebGL not supported!');
}
});

const root = createRoot(document.getElementById('root') as HTMLElement);
root.render(
<StrictMode>
<AppWrapper />
Expand Down
Loading