Skip to content

Conversation

@willeastcott
Copy link
Contributor

@willeastcott willeastcott commented Nov 29, 2025

Description

playcanvas-engine-annotations.mp4

Checklist

  • I have read the contributing guidelines
  • My code follows the project's coding standards
  • This PR focuses on a single change

@willeastcott willeastcott self-assigned this Nov 29, 2025
@willeastcott willeastcott added enhancement Request for a new feature area: examples labels Nov 29, 2025
Copilot finished reviewing on behalf of willeastcott November 29, 2025 15:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new ESM script for creating interactive 3D annotations in PlayCanvas scenes, along with a comprehensive example showcasing the feature on a 3D jet fighter model. The annotation system creates clickable hotspots with tooltips that maintain constant screen-space size and remain visible even when occluded by geometry.

Key Changes

  • New annotation.mjs script providing the Annotation class with hotspot rendering, tooltip display, and event handling
  • Example implementation demonstrating 7 annotations on a jet fighter model with HDR environment and shadow catching
  • Build script enhancement for improved cross-platform spawn compatibility

Reviewed changes

Copilot reviewed 4 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
scripts/esm/annotation.mjs Core annotation script with 3D hotspot rendering, DOM integration, and interactive tooltip system
examples/src/examples/misc/annotations.example.mjs Example showcasing annotations on a 3D jet fighter model with camera controls
examples/thumbnails/misc_annotations_small.webp Small thumbnail image for the annotations example
examples/thumbnails/misc_annotations_large.webp Large thumbnail image for the annotations example
examples/scripts/build-thumbnails.mjs Added shell: true option to spawn for cross-platform compatibility
examples/assets/models/jet-fighter.txt Attribution file for the jet fighter 3D model (CC BY 4.0)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Comment on lines +413 to +438
this.hotspotDom.addEventListener('click', (e) => {
e.stopPropagation();
this.showTooltip();
});

const leave = () => {
if (Annotation.hoverAnnotation === this) {
Annotation.hoverAnnotation = null;
this.setHover(false);
}
};

const enter = () => {
if (Annotation.hoverAnnotation !== null) {
Annotation.hoverAnnotation.setHover(false);
}
Annotation.hoverAnnotation = this;
this.setHover(true);
};

this.hotspotDom.addEventListener('pointerenter', enter);
this.hotspotDom.addEventListener('pointerleave', leave);

document.addEventListener('click', () => {
this.hideTooltip();
});
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

Event listeners added to document and this.hotspotDom are not removed during cleanup, causing potential memory leaks. The listeners on lines 413, 433, 434, and 436 should be stored as references and removed in the 'destroy' handler.

Consider storing the handler functions and removing them:

this.clickHandler = (e) => {
    e.stopPropagation();
    this.showTooltip();
};
this.hotspotDom.addEventListener('click', this.clickHandler);

this.documentClickHandler = () => {
    this.hideTooltip();
};
document.addEventListener('click', this.documentClickHandler);

// In destroy handler:
this.hotspotDom.removeEventListener('click', this.clickHandler);
this.hotspotDom.removeEventListener('pointerenter', enter);
this.hotspotDom.removeEventListener('pointerleave', leave);
document.removeEventListener('click', this.documentClickHandler);

Copilot uses AI. Check for mistakes.
willeastcott and others added 3 commits November 29, 2025 15:57
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@willeastcott willeastcott merged commit d27a89a into main Nov 29, 2025
7 checks passed
@willeastcott willeastcott deleted the annotations branch November 29, 2025 18:44
@Maksims
Copy link
Collaborator

Maksims commented Dec 1, 2025

There are a few simplifications and improvements can be done to these hotspots:

  1. No need for canvas 2D, as circle with a number can be drawn using CSS as you already have an element. Use border-radius for a circle.
  2. top/left positioning via CSS, leads to re-layouting, while transform 2D does not. And top/left does not work well with floating pixels, while transform does, so it will feel smoother positioned.

It might sound insignificant, but browsers can be notoriously bad at re-layouting, so even a single extra re-layout of a simple page, can lead to some ms wasted reducing framerate from nicely steady one. Especially with multiple elements updated during requestAnimationFrame.

@slimbuck
Copy link
Member

slimbuck commented Dec 1, 2025

Hey @Maksims,

  1. we use canvas2d to generate a texture for rendering the circle into the scene using occlusion (so circle is semi-occluded by gaussians and other solid objects), not possible with css.
  2. cool, what is re-layouting?!

@Maksims
Copy link
Collaborator

Maksims commented Dec 1, 2025

  1. we use canvas2d to generate a texture for rendering the circle into the scene using occlusion (so circle is semi-occluded by gaussians and other solid objects), not possible with css.

Got it, I thought that circle was drawn using CSS in 2D.

2. cool, what is re-layouting?!

This will then apply to the popup of an info box. Re-layouting/reflow is a process when browser has to re-calculate a lot of things with DOM due to potentially structure affecting changes in drawing state (top/left - is affecting operations). This means browser will go through all elements, re-calculate it's new bounding boxes, tree, check how they overlap between each other, and re-apply all styles. If this is triggered on every frame - this will is CPU intensive operation, especially on mobile. And will lead to framerate drops.

To preserve a good performance, it is essential to avoid re-layouting/reflow. Here is a comprehensive list of things that can trigger it: https://gist.github.com/paulirish/5d52fb081b3570c81e3a

This is the reason why we created 2D UI in the first place, as DOM UI - will eventually kill the performance of real-time apps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: examples enhancement Request for a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants