-
Notifications
You must be signed in to change notification settings - Fork 115
[stale] Improve off-screen waypoint visibility and clarity #416
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
base: next
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -5,19 +5,22 @@ export const WAYPOINT_CONFIG = { | |||||||||||||||||||||||||||||||||||||||||||
// Target size in screen pixels (this controls the final sprite size) | ||||||||||||||||||||||||||||||||||||||||||||
TARGET_SCREEN_PX: 150, | ||||||||||||||||||||||||||||||||||||||||||||
// Canvas size for internal rendering (keep power of 2 for textures) | ||||||||||||||||||||||||||||||||||||||||||||
CANVAS_SIZE: 256, | ||||||||||||||||||||||||||||||||||||||||||||
CANVAS_SIZE: 512, | ||||||||||||||||||||||||||||||||||||||||||||
// Relative positions in canvas (0-1) | ||||||||||||||||||||||||||||||||||||||||||||
LAYOUT: { | ||||||||||||||||||||||||||||||||||||||||||||
DOT_Y: 0.3, | ||||||||||||||||||||||||||||||||||||||||||||
NAME_Y: 0.45, | ||||||||||||||||||||||||||||||||||||||||||||
DISTANCE_Y: 0.55, | ||||||||||||||||||||||||||||||||||||||||||||
DISTANCE_Y: 0.62, | ||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||
// Multiplier for canvas internal resolution to keep text crisp | ||||||||||||||||||||||||||||||||||||||||||||
CANVAS_SCALE: 2, | ||||||||||||||||||||||||||||||||||||||||||||
CANVAS_SCALE: 3, | ||||||||||||||||||||||||||||||||||||||||||||
ARROW: { | ||||||||||||||||||||||||||||||||||||||||||||
enabledDefault: false, | ||||||||||||||||||||||||||||||||||||||||||||
enabledDefault: true, // Changed from false to true | ||||||||||||||||||||||||||||||||||||||||||||
pixelSize: 30, | ||||||||||||||||||||||||||||||||||||||||||||
paddingPx: 50, | ||||||||||||||||||||||||||||||||||||||||||||
// When true, shows full sprite only when in sight and only arrow when offscreen | ||||||||||||||||||||||||||||||||||||||||||||
// When false (default), always shows full sprite with oriented arrow when offscreen | ||||||||||||||||||||||||||||||||||||||||||||
hideOffscreenSprite: false, | ||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
|
@@ -62,6 +65,7 @@ export function createWaypointSprite (options: { | |||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Offscreen arrow (detached by default) | ||||||||||||||||||||||||||||||||||||||||||||
let arrowSprite: THREE.Sprite | undefined | ||||||||||||||||||||||||||||||||||||||||||||
let offscreenSprite: THREE.Sprite | undefined | ||||||||||||||||||||||||||||||||||||||||||||
let arrowParent: THREE.Object3D | null = null | ||||||||||||||||||||||||||||||||||||||||||||
let arrowEnabled = WAYPOINT_CONFIG.ARROW.enabledDefault | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
|
@@ -80,6 +84,15 @@ export function createWaypointSprite (options: { | |||||||||||||||||||||||||||||||||||||||||||
mat.map?.dispose() | ||||||||||||||||||||||||||||||||||||||||||||
mat.map = texture | ||||||||||||||||||||||||||||||||||||||||||||
mat.needsUpdate = true | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Update offscreen sprite if it exists | ||||||||||||||||||||||||||||||||||||||||||||
if (offscreenSprite) { | ||||||||||||||||||||||||||||||||||||||||||||
const offscreenTexture = new THREE.CanvasTexture(canvas.cloneNode(true) as HTMLCanvasElement) | ||||||||||||||||||||||||||||||||||||||||||||
const offscreenMat = offscreenSprite.material | ||||||||||||||||||||||||||||||||||||||||||||
offscreenMat.map?.dispose() | ||||||||||||||||||||||||||||||||||||||||||||
offscreenMat.map = offscreenTexture | ||||||||||||||||||||||||||||||||||||||||||||
offscreenMat.needsUpdate = true | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+88
to
96
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Offscreen texture updates use canvas.cloneNode() — results in blank textures. Use the same drawn canvas and keep filter parity. cloneNode on a canvas doesn’t copy the drawing buffer, so the offscreen sprite will intermittently render empty after updates. Also, new textures created here don’t match the filters set in createCombinedSprite, causing inconsistent appearance. Apply the following diffs in all three update sites (setColor, setLabel, updateDistanceText): @@
- // Update offscreen sprite if it exists
- if (offscreenSprite) {
- const offscreenTexture = new THREE.CanvasTexture(canvas.cloneNode(true) as HTMLCanvasElement)
- const offscreenMat = offscreenSprite.material
- offscreenMat.map?.dispose()
- offscreenMat.map = offscreenTexture
- offscreenMat.needsUpdate = true
- }
+ // Update offscreen sprite if it exists
+ if (offscreenSprite) {
+ const offscreenTexture = new THREE.CanvasTexture(canvas)
+ offscreenTexture.anisotropy = 1
+ offscreenTexture.magFilter = THREE.LinearFilter
+ offscreenTexture.minFilter = THREE.LinearFilter
+ const offscreenMat = offscreenSprite.material as THREE.SpriteMaterial
+ offscreenMat.map?.dispose()
+ offscreenMat.map = offscreenTexture
+ offscreenMat.needsUpdate = true
+ } And ensure main-sprite textures also keep the same filters: @@
- const texture = new THREE.CanvasTexture(canvas)
+ const texture = new THREE.CanvasTexture(canvas)
+ texture.anisotropy = 1
+ texture.magFilter = THREE.LinearFilter
+ texture.minFilter = THREE.LinearFilter
@@
- const texture = new THREE.CanvasTexture(canvas)
+ const texture = new THREE.CanvasTexture(canvas)
+ texture.anisotropy = 1
+ texture.magFilter = THREE.LinearFilter
+ texture.minFilter = THREE.LinearFilter
@@
- const texture = new THREE.CanvasTexture(canvas)
+ const texture = new THREE.CanvasTexture(canvas)
+ texture.anisotropy = 1
+ texture.magFilter = THREE.LinearFilter
+ texture.minFilter = THREE.LinearFilter Also applies to: 107-114, 125-132 🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
function setLabel (newLabel?: string) { | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -90,6 +103,15 @@ export function createWaypointSprite (options: { | |||||||||||||||||||||||||||||||||||||||||||
mat.map?.dispose() | ||||||||||||||||||||||||||||||||||||||||||||
mat.map = texture | ||||||||||||||||||||||||||||||||||||||||||||
mat.needsUpdate = true | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Update offscreen sprite if it exists | ||||||||||||||||||||||||||||||||||||||||||||
if (offscreenSprite) { | ||||||||||||||||||||||||||||||||||||||||||||
const offscreenTexture = new THREE.CanvasTexture(canvas.cloneNode(true) as HTMLCanvasElement) | ||||||||||||||||||||||||||||||||||||||||||||
const offscreenMat = offscreenSprite.material | ||||||||||||||||||||||||||||||||||||||||||||
offscreenMat.map?.dispose() | ||||||||||||||||||||||||||||||||||||||||||||
offscreenMat.map = offscreenTexture | ||||||||||||||||||||||||||||||||||||||||||||
offscreenMat.needsUpdate = true | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
function updateDistanceText (label: string, distanceText: string) { | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -99,6 +121,15 @@ export function createWaypointSprite (options: { | |||||||||||||||||||||||||||||||||||||||||||
mat.map?.dispose() | ||||||||||||||||||||||||||||||||||||||||||||
mat.map = texture | ||||||||||||||||||||||||||||||||||||||||||||
mat.needsUpdate = true | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Update offscreen sprite if it exists | ||||||||||||||||||||||||||||||||||||||||||||
if (offscreenSprite) { | ||||||||||||||||||||||||||||||||||||||||||||
const offscreenTexture = new THREE.CanvasTexture(canvas.cloneNode(true) as HTMLCanvasElement) | ||||||||||||||||||||||||||||||||||||||||||||
const offscreenMat = offscreenSprite.material | ||||||||||||||||||||||||||||||||||||||||||||
offscreenMat.map?.dispose() | ||||||||||||||||||||||||||||||||||||||||||||
offscreenMat.map = offscreenTexture | ||||||||||||||||||||||||||||||||||||||||||||
offscreenMat.needsUpdate = true | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
function setVisible (visible: boolean) { | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -149,15 +180,34 @@ export function createWaypointSprite (options: { | |||||||||||||||||||||||||||||||||||||||||||
if (arrowParent) arrowParent.add(arrowSprite) | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
function ensureOffscreenSprite () { | ||||||||||||||||||||||||||||||||||||||||||||
if (offscreenSprite) return | ||||||||||||||||||||||||||||||||||||||||||||
// Create a copy of the main sprite for offscreen display | ||||||||||||||||||||||||||||||||||||||||||||
const canvas = drawCombinedCanvas(color, currentLabel, '0m') | ||||||||||||||||||||||||||||||||||||||||||||
const texture = new THREE.CanvasTexture(canvas) | ||||||||||||||||||||||||||||||||||||||||||||
const material = new THREE.SpriteMaterial({ | ||||||||||||||||||||||||||||||||||||||||||||
map: texture, | ||||||||||||||||||||||||||||||||||||||||||||
transparent: true, | ||||||||||||||||||||||||||||||||||||||||||||
depthTest: false, | ||||||||||||||||||||||||||||||||||||||||||||
depthWrite: false | ||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||
offscreenSprite = new THREE.Sprite(material) | ||||||||||||||||||||||||||||||||||||||||||||
offscreenSprite.renderOrder = 11 // Between main sprite and arrow | ||||||||||||||||||||||||||||||||||||||||||||
offscreenSprite.visible = false | ||||||||||||||||||||||||||||||||||||||||||||
if (arrowParent) arrowParent.add(offscreenSprite) | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
function enableOffscreenArrow (enabled: boolean) { | ||||||||||||||||||||||||||||||||||||||||||||
arrowEnabled = enabled | ||||||||||||||||||||||||||||||||||||||||||||
if (!enabled && arrowSprite) arrowSprite.visible = false | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
function setArrowParent (parent: THREE.Object3D | null) { | ||||||||||||||||||||||||||||||||||||||||||||
if (arrowSprite?.parent) arrowSprite.parent.remove(arrowSprite) | ||||||||||||||||||||||||||||||||||||||||||||
if (offscreenSprite?.parent) offscreenSprite.parent.remove(offscreenSprite) | ||||||||||||||||||||||||||||||||||||||||||||
arrowParent = parent | ||||||||||||||||||||||||||||||||||||||||||||
if (arrowSprite && parent) parent.add(arrowSprite) | ||||||||||||||||||||||||||||||||||||||||||||
if (offscreenSprite && parent) parent.add(offscreenSprite) | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
function updateOffscreenArrow ( | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -167,7 +217,8 @@ export function createWaypointSprite (options: { | |||||||||||||||||||||||||||||||||||||||||||
): boolean { | ||||||||||||||||||||||||||||||||||||||||||||
if (!arrowEnabled) return true | ||||||||||||||||||||||||||||||||||||||||||||
ensureArrow() | ||||||||||||||||||||||||||||||||||||||||||||
if (!arrowSprite) return true | ||||||||||||||||||||||||||||||||||||||||||||
ensureOffscreenSprite() | ||||||||||||||||||||||||||||||||||||||||||||
if (!arrowSprite || !offscreenSprite) return true | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Build camera basis using camera.up to respect custom orientations | ||||||||||||||||||||||||||||||||||||||||||||
const forward = new THREE.Vector3() | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -192,9 +243,10 @@ export function createWaypointSprite (options: { | |||||||||||||||||||||||||||||||||||||||||||
// Determine if waypoint is inside view frustum using angular checks | ||||||||||||||||||||||||||||||||||||||||||||
const thetaX = Math.atan2(x, z) | ||||||||||||||||||||||||||||||||||||||||||||
const thetaY = Math.atan2(y, z) | ||||||||||||||||||||||||||||||||||||||||||||
const visible = z > 0 && Math.abs(thetaX) <= hFovRad / 2 && Math.abs(thetaY) <= vFovRad / 2 | ||||||||||||||||||||||||||||||||||||||||||||
if (visible) { | ||||||||||||||||||||||||||||||||||||||||||||
const inSight = z > 0 && Math.abs(thetaX) <= hFovRad / 2 && Math.abs(thetaY) <= vFovRad / 2 | ||||||||||||||||||||||||||||||||||||||||||||
if (inSight) { | ||||||||||||||||||||||||||||||||||||||||||||
arrowSprite.visible = false | ||||||||||||||||||||||||||||||||||||||||||||
offscreenSprite.visible = false | ||||||||||||||||||||||||||||||||||||||||||||
return true | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
|
@@ -215,39 +267,60 @@ export function createWaypointSprite (options: { | |||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Place on the rectangle border [-1,1]x[-1,1] | ||||||||||||||||||||||||||||||||||||||||||||
const s = Math.max(Math.abs(rx), Math.abs(ry)) || 1 | ||||||||||||||||||||||||||||||||||||||||||||
let ndcX = rx / s | ||||||||||||||||||||||||||||||||||||||||||||
let ndcY = ry / s | ||||||||||||||||||||||||||||||||||||||||||||
const ndcX = rx / s | ||||||||||||||||||||||||||||||||||||||||||||
const ndcY = ry / s | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Apply padding in pixel space by clamping | ||||||||||||||||||||||||||||||||||||||||||||
const padding = WAYPOINT_CONFIG.ARROW.paddingPx | ||||||||||||||||||||||||||||||||||||||||||||
const pxX = ((ndcX + 1) * 0.5) * viewportWidthPx | ||||||||||||||||||||||||||||||||||||||||||||
const pxY = ((1 - ndcY) * 0.5) * viewportHeightPx | ||||||||||||||||||||||||||||||||||||||||||||
const clampedPxX = Math.min(Math.max(pxX, padding), viewportWidthPx - padding) | ||||||||||||||||||||||||||||||||||||||||||||
const clampedPxY = Math.min(Math.max(pxY, padding), viewportHeightPx - padding) | ||||||||||||||||||||||||||||||||||||||||||||
ndcX = (clampedPxX / viewportWidthPx) * 2 - 1 | ||||||||||||||||||||||||||||||||||||||||||||
ndcY = -(clampedPxY / viewportHeightPx) * 2 + 1 | ||||||||||||||||||||||||||||||||||||||||||||
const finalNdcX = (clampedPxX / viewportWidthPx) * 2 - 1 | ||||||||||||||||||||||||||||||||||||||||||||
const finalNdcY = -(clampedPxY / viewportHeightPx) * 2 + 1 | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Compute world position at a fixed distance in front of the camera using camera basis | ||||||||||||||||||||||||||||||||||||||||||||
const placeDist = Math.max(2, camera.near * 4) | ||||||||||||||||||||||||||||||||||||||||||||
const halfPlaneHeight = Math.tan(vFovRad / 2) * placeDist | ||||||||||||||||||||||||||||||||||||||||||||
const halfPlaneWidth = halfPlaneHeight * aspect | ||||||||||||||||||||||||||||||||||||||||||||
const pos = camPos.clone() | ||||||||||||||||||||||||||||||||||||||||||||
const basePos = camPos.clone() | ||||||||||||||||||||||||||||||||||||||||||||
.add(forward.clone().multiplyScalar(placeDist)) | ||||||||||||||||||||||||||||||||||||||||||||
.add(right.clone().multiplyScalar(ndcX * halfPlaneWidth)) | ||||||||||||||||||||||||||||||||||||||||||||
.add(upCam.clone().multiplyScalar(ndcY * halfPlaneHeight)) | ||||||||||||||||||||||||||||||||||||||||||||
.add(right.clone().multiplyScalar(finalNdcX * halfPlaneWidth)) | ||||||||||||||||||||||||||||||||||||||||||||
.add(upCam.clone().multiplyScalar(finalNdcY * halfPlaneHeight)) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Update arrow sprite | ||||||||||||||||||||||||||||||||||||||||||||
arrowSprite.visible = true | ||||||||||||||||||||||||||||||||||||||||||||
arrowSprite.position.copy(pos) | ||||||||||||||||||||||||||||||||||||||||||||
arrowSprite.position.copy(basePos) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Angle for rotation relative to screen right/up (derived from camera up vector) | ||||||||||||||||||||||||||||||||||||||||||||
const angle = Math.atan2(ry, rx) | ||||||||||||||||||||||||||||||||||||||||||||
arrowSprite.material.rotation = angle - Math.PI / 2 | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Constant pixel size for arrow (use fixed placement distance) | ||||||||||||||||||||||||||||||||||||||||||||
// Use configured arrow pixel size for proper scale | ||||||||||||||||||||||||||||||||||||||||||||
const worldUnitsPerScreenHeightAtDist = Math.tan(vFovRad / 2) * 2 * placeDist | ||||||||||||||||||||||||||||||||||||||||||||
const sPx = worldUnitsPerScreenHeightAtDist * (WAYPOINT_CONFIG.ARROW.pixelSize / viewportHeightPx) | ||||||||||||||||||||||||||||||||||||||||||||
arrowSprite.scale.set(sPx, sPx, 1) | ||||||||||||||||||||||||||||||||||||||||||||
const arrowPx = worldUnitsPerScreenHeightAtDist * (WAYPOINT_CONFIG.ARROW.pixelSize / viewportHeightPx) | ||||||||||||||||||||||||||||||||||||||||||||
arrowSprite.scale.set(arrowPx, arrowPx, 1) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Position offscreen sprite offset from arrow toward center to avoid overlap | ||||||||||||||||||||||||||||||||||||||||||||
const offsetDistance = WAYPOINT_CONFIG.ARROW.pixelSize + 30 // Offset distance in pixels | ||||||||||||||||||||||||||||||||||||||||||||
const offsetWorldUnits = worldUnitsPerScreenHeightAtDist * (offsetDistance / viewportHeightPx) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Calculate offset direction (opposite to arrow direction, toward screen center) | ||||||||||||||||||||||||||||||||||||||||||||
const offsetX = -finalNdcX * offsetWorldUnits * 0.5 // Move toward center | ||||||||||||||||||||||||||||||||||||||||||||
const offsetY = -finalNdcY * offsetWorldUnits * 0.5 | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
const spritePos = basePos.clone() | ||||||||||||||||||||||||||||||||||||||||||||
.add(right.clone().multiplyScalar(offsetX)) | ||||||||||||||||||||||||||||||||||||||||||||
.add(upCam.clone().multiplyScalar(offsetY)) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
offscreenSprite.visible = true | ||||||||||||||||||||||||||||||||||||||||||||
offscreenSprite.position.copy(spritePos) | ||||||||||||||||||||||||||||||||||||||||||||
offscreenSprite.material.rotation = 0 // No rotation for the sprite | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Scale offscreen sprite to match the main sprite size | ||||||||||||||||||||||||||||||||||||||||||||
const spritePx = worldUnitsPerScreenHeightAtDist * (WAYPOINT_CONFIG.TARGET_SCREEN_PX / viewportHeightPx) | ||||||||||||||||||||||||||||||||||||||||||||
offscreenSprite.scale.set(spritePx, spritePx, 1) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+316
to
+323
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Respect ARROW.hideOffscreenSprite: don’t show the offscreen full sprite when configured to “arrow only.” Currently, offscreenSprite is forced visible offscreen regardless of the flag. Gate it: - offscreenSprite.visible = true
+ offscreenSprite.visible = !WAYPOINT_CONFIG.ARROW.hideOffscreenSprite 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||
return false | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
|
@@ -268,7 +341,16 @@ export function createWaypointSprite (options: { | |||||||||||||||||||||||||||||||||||||||||||
updateDistanceText(currentLabel, `${Math.round(distance)}m`) | ||||||||||||||||||||||||||||||||||||||||||||
// Update arrow and visibility | ||||||||||||||||||||||||||||||||||||||||||||
const onScreen = updateOffscreenArrow(camera, viewportWidthPx, viewportHeightPx) | ||||||||||||||||||||||||||||||||||||||||||||
setVisible(onScreen) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Handle sprite visibility based on config | ||||||||||||||||||||||||||||||||||||||||||||
if (WAYPOINT_CONFIG.ARROW.hideOffscreenSprite) { | ||||||||||||||||||||||||||||||||||||||||||||
// When hideOffscreenSprite is true: show sprite only when in sight | ||||||||||||||||||||||||||||||||||||||||||||
setVisible(onScreen) | ||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||
// When hideOffscreenSprite is false: always show sprite (default behavior) | ||||||||||||||||||||||||||||||||||||||||||||
setVisible(true) | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
return onScreen | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
|
@@ -281,6 +363,11 @@ export function createWaypointSprite (options: { | |||||||||||||||||||||||||||||||||||||||||||
am.map?.dispose() | ||||||||||||||||||||||||||||||||||||||||||||
am.dispose() | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
if (offscreenSprite) { | ||||||||||||||||||||||||||||||||||||||||||||
const om = offscreenSprite.material | ||||||||||||||||||||||||||||||||||||||||||||
om.map?.dispose() | ||||||||||||||||||||||||||||||||||||||||||||
om.dispose() | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -299,60 +386,60 @@ export function createWaypointSprite (options: { | |||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Internal helpers | ||||||||||||||||||||||||||||||||||||||||||||
function drawCombinedCanvas (color: number, id: string, distance: string): HTMLCanvasElement { | ||||||||||||||||||||||||||||||||||||||||||||
function drawCombinedCanvas (color: number, id: string, distance: string, ctx?: CanvasRenderingContext2D, size?: number): HTMLCanvasElement { | ||||||||||||||||||||||||||||||||||||||||||||
const scale = WAYPOINT_CONFIG.CANVAS_SCALE * (globalThis.devicePixelRatio || 1) | ||||||||||||||||||||||||||||||||||||||||||||
const size = WAYPOINT_CONFIG.CANVAS_SIZE * scale | ||||||||||||||||||||||||||||||||||||||||||||
const sizeCanvas = size || WAYPOINT_CONFIG.CANVAS_SIZE * scale | ||||||||||||||||||||||||||||||||||||||||||||
const canvas = document.createElement('canvas') | ||||||||||||||||||||||||||||||||||||||||||||
canvas.width = size | ||||||||||||||||||||||||||||||||||||||||||||
canvas.height = size | ||||||||||||||||||||||||||||||||||||||||||||
const ctx = canvas.getContext('2d')! | ||||||||||||||||||||||||||||||||||||||||||||
canvas.width = sizeCanvas | ||||||||||||||||||||||||||||||||||||||||||||
canvas.height = sizeCanvas | ||||||||||||||||||||||||||||||||||||||||||||
const ctxCanvas = ctx || canvas.getContext('2d')! | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+389
to
+395
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainTexture size can exceed common GPU limits (DPR-multiplied 512×3 → 4608 on DPR=3). Clamp and cap DPR to prevent perf/alloc spikes. Multiplying CANVAS_SIZE (512) by CANVAS_SCALE (3) and devicePixelRatio produces >4K canvases on high-DPR displays, risking stalls and OOM on some GPUs. Cap DPR and clamp to a safe max size: -function drawCombinedCanvas (color: number, id: string, distance: string, ctx?: CanvasRenderingContext2D, size?: number): HTMLCanvasElement {
- const scale = WAYPOINT_CONFIG.CANVAS_SCALE * (globalThis.devicePixelRatio || 1)
- const sizeCanvas = size || WAYPOINT_CONFIG.CANVAS_SIZE * scale
+function drawCombinedCanvas (color: number, id: string, distance: string, ctx?: CanvasRenderingContext2D, size?: number): HTMLCanvasElement {
+ const deviceScale = Math.min(globalThis.devicePixelRatio || 1, 2) // cap DPR impact
+ const scale = WAYPOINT_CONFIG.CANVAS_SCALE * deviceScale
+ const baseSize = WAYPOINT_CONFIG.CANVAS_SIZE * scale
+ const MAX_TEX = 2048
+ const sizeCanvas = size || Math.min(baseSize, MAX_TEX) Optionally consider reducing CANVAS_SCALE to 2 for mobile profiles. Also applies to: 8-8, 16-16, 21-23 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Searching repository for drawCombinedCanvas, WAYPOINT_CONFIG, CANVAS_SIZE, CANVAS_SCALE, devicePixelRatio..."
if command -v rg >/dev/null 2>&1; then
rg -n --hidden --no-ignore-vcs -S "drawCombinedCanvas|WAYPOINT_CONFIG|CANVAS_SIZE|CANVAS_SCALE|devicePixelRatio" || true
else
echo "rg (ripgrep) not found; using find+grep fallback"
find . -type f -name '*.ts' -o -name '*.js' -o -name '*.tsx' -o -name '*.jsx' | xargs grep -nH -E "drawCombinedCanvas|WAYPOINT_CONFIG|CANVAS_SIZE|CANVAS_SCALE|devicePixelRatio" || true
fi
# Show the waypointSprite file if present
WSP=$(rg -n --hidden --no-ignore-vcs -S "waypointSprite.ts" -l || true)
if [ -n "$WSP" ]; then
echo "Found waypointSprite file(s):"
echo "$WSP"
for f in $WSP; do
echo "---- $f (first 400 lines) ----"
sed -n '1,400p' "$f" | nl -ba
done
else
echo "No waypointSprite.ts file found by name; trying to locate files containing drawCombinedCanvas..."
FILES=$(rg -n --hidden --no-ignore-vcs -S "drawCombinedCanvas" -l || true)
if [ -n "$FILES" ]; then
echo "Files with drawCombinedCanvas:"
echo "$FILES"
for f in $FILES; do
echo "---- $f (first 400 lines) ----"
sed -n '1,400p' "$f" | nl -ba
done
else
echo "No files contain drawCombinedCanvas."
fi
fi
echo "Searching for definitions/values of CANVAS_SIZE / CANVAS_SCALE / WAYPOINT_CONFIG..."
CFGFILES=$(rg -n --hidden --no-ignore-vcs -S "CANVAS_SIZE|CANVAS_SCALE|WAYPOINT_CONFIG" -l || true)
if [ -n "$CFGFILES" ]; then
for f in $CFGFILES; do
echo "---- $f ----"
rg -n --hidden --no-ignore-vcs -S "CANVAS_SIZE|CANVAS_SCALE|WAYPOINT_CONFIG" "$f" -n -C3 || true
echo "File head:"
sed -n '1,200p' "$f" | nl -ba
done
else
echo "No files found defining CANVAS_SIZE/CANVAS_SCALE/WAYPOINT_CONFIG."
fi
echo "Searching for raw uses of canvas.width/height and getContext..."
rg -n --hidden --no-ignore-vcs -S "canvas.width|canvas.height|getContext\\(" || true
echo "Done." Length of output: 3105 Clamp DPR and cap canvas size to avoid >4K textures WAYPOINT_CONFIG.CANVAS_SIZE = 512 and CANVAS_SCALE = 3 are multiplied by devicePixelRatio in drawCombinedCanvas — DPR=3 → 51233 = 4608px, which can cause GPU stalls/OOM. Cap DPR and clamp the computed canvas size. Location: renderer/viewer/three/waypointSprite.ts (drawCombinedCanvas at ~lines 389–395; WAYPOINT_CONFIG at top ~lines 4–16) -function drawCombinedCanvas (color: number, id: string, distance: string, ctx?: CanvasRenderingContext2D, size?: number): HTMLCanvasElement {
- const scale = WAYPOINT_CONFIG.CANVAS_SCALE * (globalThis.devicePixelRatio || 1)
- const sizeCanvas = size || WAYPOINT_CONFIG.CANVAS_SIZE * scale
+function drawCombinedCanvas (color: number, id: string, distance: string, ctx?: CanvasRenderingContext2D, size?: number): HTMLCanvasElement {
+ const deviceScale = Math.min(globalThis.devicePixelRatio || 1, 2) // cap DPR impact
+ const scale = WAYPOINT_CONFIG.CANVAS_SCALE * deviceScale
+ const baseSize = WAYPOINT_CONFIG.CANVAS_SIZE * scale
+ const MAX_TEX = 2048
+ const sizeCanvas = size || Math.min(baseSize, MAX_TEX) Also review other drawCombinedCanvas call sites in this file to ensure they don’t force oversized textures. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Clear canvas | ||||||||||||||||||||||||||||||||||||||||||||
ctx.clearRect(0, 0, size, size) | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.clearRect(0, 0, sizeCanvas, sizeCanvas) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Draw dot | ||||||||||||||||||||||||||||||||||||||||||||
const centerX = size / 2 | ||||||||||||||||||||||||||||||||||||||||||||
const dotY = Math.round(size * WAYPOINT_CONFIG.LAYOUT.DOT_Y) | ||||||||||||||||||||||||||||||||||||||||||||
const radius = Math.round(size * 0.05) // Dot takes up ~12% of canvas height | ||||||||||||||||||||||||||||||||||||||||||||
const centerX = sizeCanvas / 2 | ||||||||||||||||||||||||||||||||||||||||||||
const dotY = Math.round(sizeCanvas * WAYPOINT_CONFIG.LAYOUT.DOT_Y) | ||||||||||||||||||||||||||||||||||||||||||||
const radius = Math.round(sizeCanvas * 0.05) // Dot takes up ~12% of canvas height | ||||||||||||||||||||||||||||||||||||||||||||
const borderWidth = Math.max(2, Math.round(4 * scale)) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Outer border (black) | ||||||||||||||||||||||||||||||||||||||||||||
ctx.beginPath() | ||||||||||||||||||||||||||||||||||||||||||||
ctx.arc(centerX, dotY, radius + borderWidth, 0, Math.PI * 2) | ||||||||||||||||||||||||||||||||||||||||||||
ctx.fillStyle = 'black' | ||||||||||||||||||||||||||||||||||||||||||||
ctx.fill() | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.beginPath() | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.arc(centerX, dotY, radius + borderWidth, 0, Math.PI * 2) | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.fillStyle = 'black' | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.fill() | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Inner circle (colored) | ||||||||||||||||||||||||||||||||||||||||||||
ctx.beginPath() | ||||||||||||||||||||||||||||||||||||||||||||
ctx.arc(centerX, dotY, radius, 0, Math.PI * 2) | ||||||||||||||||||||||||||||||||||||||||||||
ctx.fillStyle = `#${color.toString(16).padStart(6, '0')}` | ||||||||||||||||||||||||||||||||||||||||||||
ctx.fill() | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.beginPath() | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.arc(centerX, dotY, radius, 0, Math.PI * 2) | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.fillStyle = `#${color.toString(16).padStart(6, '0')}` | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.fill() | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Text properties | ||||||||||||||||||||||||||||||||||||||||||||
ctx.textAlign = 'center' | ||||||||||||||||||||||||||||||||||||||||||||
ctx.textBaseline = 'middle' | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.textAlign = 'center' | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.textBaseline = 'middle' | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Title | ||||||||||||||||||||||||||||||||||||||||||||
const nameFontPx = Math.round(size * 0.08) // ~8% of canvas height | ||||||||||||||||||||||||||||||||||||||||||||
const distanceFontPx = Math.round(size * 0.06) // ~6% of canvas height | ||||||||||||||||||||||||||||||||||||||||||||
ctx.font = `bold ${nameFontPx}px mojangles` | ||||||||||||||||||||||||||||||||||||||||||||
ctx.lineWidth = Math.max(2, Math.round(3 * scale)) | ||||||||||||||||||||||||||||||||||||||||||||
const nameY = Math.round(size * WAYPOINT_CONFIG.LAYOUT.NAME_Y) | ||||||||||||||||||||||||||||||||||||||||||||
const nameFontPx = Math.round(sizeCanvas * 0.16) | ||||||||||||||||||||||||||||||||||||||||||||
const distanceFontPx = Math.round(sizeCanvas * 0.11) | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.font = `bold ${nameFontPx}px mojangles` | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.lineWidth = Math.max(3, Math.round(4 * scale)) | ||||||||||||||||||||||||||||||||||||||||||||
const nameY = Math.round(sizeCanvas * WAYPOINT_CONFIG.LAYOUT.NAME_Y) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
ctx.strokeStyle = 'black' | ||||||||||||||||||||||||||||||||||||||||||||
ctx.strokeText(id, centerX, nameY) | ||||||||||||||||||||||||||||||||||||||||||||
ctx.fillStyle = 'white' | ||||||||||||||||||||||||||||||||||||||||||||
ctx.fillText(id, centerX, nameY) | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.strokeStyle = 'black' | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.strokeText(id, centerX, nameY) | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.fillStyle = 'white' | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.fillText(id, centerX, nameY) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
// Distance | ||||||||||||||||||||||||||||||||||||||||||||
ctx.font = `bold ${distanceFontPx}px mojangles` | ||||||||||||||||||||||||||||||||||||||||||||
ctx.lineWidth = Math.max(2, Math.round(2 * scale)) | ||||||||||||||||||||||||||||||||||||||||||||
const distanceY = Math.round(size * WAYPOINT_CONFIG.LAYOUT.DISTANCE_Y) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
ctx.strokeStyle = 'black' | ||||||||||||||||||||||||||||||||||||||||||||
ctx.strokeText(distance, centerX, distanceY) | ||||||||||||||||||||||||||||||||||||||||||||
ctx.fillStyle = '#CCCCCC' | ||||||||||||||||||||||||||||||||||||||||||||
ctx.fillText(distance, centerX, distanceY) | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.font = `bold ${distanceFontPx}px mojangles` | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.lineWidth = Math.max(3, Math.round(3 * scale)) | ||||||||||||||||||||||||||||||||||||||||||||
const distanceY = Math.round(sizeCanvas * WAYPOINT_CONFIG.LAYOUT.DISTANCE_Y) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.strokeStyle = 'black' | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.strokeText(distance, centerX, distanceY) | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.fillStyle = '#CCCCCC' | ||||||||||||||||||||||||||||||||||||||||||||
ctxCanvas.fillText(distance, centerX, distanceY) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
return canvas | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Canvas Cloning Fails for Offscreen Textures
Using
canvas.cloneNode(true)
to create textures for offscreen sprites is incorrect.cloneNode
only copies the DOM element structure, not its drawn content, causing offscreen sprites to display blank textures instead of the waypoint graphics when updated insetColor
,setLabel
, andupdateDistanceText
.Additional Locations (2)
renderer/viewer/three/waypointSprite.ts#L108-L109
renderer/viewer/three/waypointSprite.ts#L126-L127