Skip to content

Capture Object.width/height for GIF uploads (#690 phase 1)#947

Open
vjpixel wants to merge 1 commit into
developfrom
claude/fix-690-phase1-gif-dimensions
Open

Capture Object.width/height for GIF uploads (#690 phase 1)#947
vjpixel wants to merge 1 commit into
developfrom
claude/fix-690-phase1-gif-dimensions

Conversation

@vjpixel
Copy link
Copy Markdown
Member

@vjpixel vjpixel commented Apr 25, 2026

Summary

Phase 1 of #690 — capture native pixel dimensions for raster uploads on Object. Lays the groundwork for thumbnails and AR/MR rendering to use real dimensions (refs #473, #319) instead of the hardcoded 50×50 in Object.as_html_thumbnail.

Scope is intentionally minimal: only GIF, only via Pillow (already in deps). MP4/WebM (ffprobe) and GLB (pygltflib/trimesh) are deferred — see my comment on the issue for the phased breakdown.

Changes

  • Model: Object.width and Object.height (PositiveIntegerField(null=True, blank=True)).
  • Migration: core.0028 adds the fields. objectevent (pghistory tracking model) gets the same fields automatically.
  • Form: UploadObjectForm.save() populates width/height when file_extension == "gif". Other extensions leave the fields null until phase 2/3.
  • Management command: backfill_object_dimensions for legacy rows. Iterates GIF rows where width__isnull=True, probes via Pillow, supports --dry-run, logs failures and skips unreadable files.

Verified locally

>>> form = UploadObjectForm(files={"source": <320×240 GIF>}, data={...})
>>> form.is_valid(); obj = form.save(commit=False); obj.owner = profile; obj.save()
saved: width=320, height=240, ext=gif

Risk

  • Backward compat: fields are nullable; legacy rows have null until backfill runs.
  • Phase 2/3: depend on ffmpeg + ffprobe (system deps) and a GLB parser. Out of scope here.
  • No template change: nothing reads width/height yet. The follow-up to use these in the AR/MR rendering will arrive once the data is actually populated.

Test plan

  • make test passes
  • make migrate applies cleanly
  • Upload a GIF → row has width/height set
  • Upload an mp4 → row has width/height null (expected, phase 2)
  • python src/manage.py backfill_object_dimensions --dry-run reports counts without saving
  • python src/manage.py backfill_object_dimensions populates dimensions for existing GIFs

Refs #690 (phase 1)

https://claude.ai/code/session_01XC1THLWgnGXGf5wgRhdyvB


Generated by Claude Code

Adds nullable PositiveIntegerField width/height on Object, populated
on upload for GIF files via Pillow (already in deps). Sets up the
infrastructure for templates and AR/MR rendering to use real
dimensions instead of hardcoded 50×50 thumbnails (#473, #319).

Phase 2/3 (mp4/webm via ffprobe and glb via pygltflib/trimesh) are
deferred — see the comment thread on the issue for the breakdown.

Includes:
- core.0028 migration adding the fields (objectevent gets the same
  fields automatically via pghistory).
- UploadObjectForm.save now populates width/height when extension
  is gif. Other extensions leave the fields null until phase 2/3.
- New management command `backfill_object_dimensions` for legacy
  rows: probes GIFs, supports --dry-run, logs failures and skips
  unreadable files.

Verified locally: uploading a 320x240 GIF saves the row with
width=320, height=240.

Refs #690

https://claude.ai/code/session_01XC1THLWgnGXGf5wgRhdyvB
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't have access to run management commands on our servers. This should be implemented as a migration with python operations to populate the missing fields for older records, we have a few examples of this.

This ensures that after the new migrations run, fields are created, older records have the new fields populated and the new code should ensure that new records always populate the field through tests

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