Skip to content

Conversation

@Taktar
Copy link
Collaborator

@Taktar Taktar commented Nov 10, 2025

  1. What Was Changed

Added text label annotations for the Bony Pelvis slide (slide02_bony_pelvis.json).

Implemented logic in annotationOverlay.js and dropdowns.js to only display annotations when the selected boneset is “Bony Pelvis.”

Ensured annotations disappear when switching to other bonesets.

Adjusted the EMU-to-pixel mapping and positioning logic to align the labels with the correct anatomical areas.

Updated styling to improve readability and layering of annotations on top of bone images.

  1. Why It Was Changed

Previously, the Bony Pelvis slides didn’t have visible annotations, making it difficult for users to identify key features like the Ilium, Pubis, and Ischium.
This update helps users clearly see anatomical relationships and interact with the slide more effectively.

  1. How It Was Changed

Added a new annotation JSON file (slide02_bony_pelvis.json) with label and line coordinates.

Integrated a crop template (template_bony_pelvis.json) to align slide coordinates with displayed images.

Updated JS logic to fetch, parse, and render labels dynamically when “Bony Pelvis” is selected.

Fine-tuned coordinate mapping to ensure annotations track correctly with image positioning.

  1. Known Issue / Next Steps

The annotations are now working, but there’s a slight positional offset on some screens due to scaling differences between PowerPoint EMU coordinates and displayed image ratios.

Currently testing small adjustments to the crop normalization to achieve a perfect alignment.

Will request additional review or suggestions for fine-tuning overlay scaling.

  1. Screenshots

Before: No annotations shown for Bony Pelvis slides.
After: Text label annotations appear correctly for Bony Pelvis; hidden for other selections.

Screenshot 2025-11-09 at 11 10 20 PM

@Taktar Taktar requested a review from UcheWendy as a code owner November 10, 2025 05:10
@Taktar Taktar self-assigned this Nov 10, 2025
@Taktar
Copy link
Collaborator Author

Taktar commented Nov 10, 2025

@UcheWendy I didn't change 111 files, it shows that because I had to add the data folder to get the annotations working. These are the files I modified:

Files Changed for Issue #165

  1. templates/js/annotationOverlay.js
    Purpose: Core logic for rendering PowerPoint-style text annotations and pointer lines.
    Changes Made:
  • Implemented dynamic annotation rendering for Bony Pelvis images.
  • Added coordinate mapping logic to convert EMU → pixel positions using template_bony_pelvis.json crop geometry.
  • Added support for both .annotations and .text_annotations arrays for flexible JSON formats.
  • Updated functions ensureStage() and clearAnnotations() for consistent overlay creation/removal.
  • Integrated conditional crop logic that only applies for Bony Pelvis slides.
  1. templates/js/dropdowns.js (or your main event-handling script, depending on your file structure)
    Purpose: Controls dropdown selection behavior and annotation display triggers.
    Changes Made:
  • Added logic to load annotations only when “Bony Pelvis” is selected from the boneset dropdown.
  • Ensured annotations clear and disappear when switching to other bonesets.
  • Linked bone ID selection to loadBoneImages() with annotation options passed dynamically.
  1. data/DataPelvis/annotations/text_label_annotations/slide02_bony_pelvis.json
    Purpose: Added new JSON file with labeled coordinates for Bony Pelvis (Ilium, Pubis, Ischium, Acetabulum, Obturator Foramen).
    Changes Made:
  • Created structured text annotations including text_box, pointer_lines, and target_regions.
  • Normalized EMU coordinates consistent with PowerPoint exports.
  1. data/DataPelvis/annotations/rotations annotations/template_bony_pelvis.json
    Purpose: Template crop reference for aligning slide coordinates with rendered image boundaries.
    Changes Made:
  • Added normalized geometry fields (left, right) to define bounding boxes for slide-to-image mapping.
  1. static/css/style.css
    Purpose: Enhanced annotation overlay visuals and positioning.
    Changes Made:
  • Added .annotation-stage, .annotation-line, .annotation-label, and .annotation-labels selectors.
  • Adjusted z-index and pointer-events for correct layering above bone images.
  • Ensured .with-annotations class disables image tilt transforms when active.
  • Improved text box legibility (contrast, padding, and shadow).

Copy link
Collaborator

@UcheWendy UcheWendy left a comment

Choose a reason for hiding this comment

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

the code you wrote for the feature itself is very high-quality.

Excellent Architecture: Creating a new, self-contained js/annotationOverlay.js module is the perfect choice. It’s small, has a clear purpose, and doesn't overcomplicate other files.

Correctly Solved the Hardest Problem: You correctly identified that this is a coordinate math problem. Your solution is excellent:

You defined the PowerPoint EMU constants (PPT_EMU).

Your emuRectToPx and emuPointToPx helper functions are the correct way to handle the scaling.

It creates an SVG and a separate div for labels (.annotation-stage).

Great CSS: The new styles in style.css are perfect. You've correctly used position: absolute and z-index to create the overlay.

Clean Integration: You've correctly modified imageDisplay.js and dropdowns.js to call loadAndDrawAnnotations and clearAnnotations at the right times.

🚨 Critical Issues: PR Scope & Architecture
This PR cannot be merged in its current state. The logical changes are good, but they are bundled with thousands of unrelated files that break the project's architecture.

Massive Scope Creep:

The Problem: This PR adds over 2,000 files that are unrelated to the issue. It includes new GitHub templates, a LICENSE, CODEOWNERS, and the entire data structure for the whole project (images, descriptions, colored regions, etc.) in a new templates/data directory.

The Fix: A pull request must only contain the code needed to solve its specific issue. All of these extra files must be removed.

Major Architectural Violation (New Data Source):

The Problem: You added a new templates/data directory. The established architecture of this project is that the backend (boneset-api) fetches all data from the GitHub data branch. The frontend (templates) should never contain its own data folder. This PR creates a second, conflicting source of truth.

The Fix: This entire templates/data directory and all its contents must be deleted from this PR.

Architectural Violation (Hardcoded URL):

The Problem: Your dropdowns.js file has a hardcoded path to a local file: ./templates/data/DataPelvis/annotations/text_label_annotations/slide02_bony_pelvis.json. This breaks the architecture and will not work on a deployed server. The frontend must not read from its own filesystem.

The Fix: The frontend must ask the backend for this data. You need to create a new API endpoint in server.js (e.g., /api/annotations/:boneId). The frontend will call this endpoint, and the backend will be responsible for fetching the correct JSON file from the GitHub data branch and returning it.

💡 How to Fix This PR (Required Action)
This is a very common issue, and the fix is straightforward:

Do not modify this PR.

Create a new, clean branch from main.

Copy only the 5 files that are part of this feature into your new branch:

templates/boneset.html (just the 1-line script addition)

templates/js/annotationOverlay.js (your new file)

templates/js/dropdowns.js (your changes)

templates/js/imageDisplay.js (your changes)

templates/style.css (the new annotation styles)

Fix the hardcoded URL: In your new branch, modify dropdowns.js to call a (new) backend API endpoint instead of a local file.

Submit a new, clean PR with just these 5 files.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants