TODO-FIXES.md

@@ -315,3 +315,56 @@ held the Playwright profile lock), so the following still needs a manual pass:
    console shows `Rendering n%` (frame-accurate path) and not
    `Falling back to the real-time encoder`
 4. Play the result: check audio is in sync and the video layer is not smeared
+
+---
+
+## Upstream open issues (alyssaxuu/motionity)
+
+The 13 bug reports still open upstream were replayed in Chromium against this
+fork. Nine were already fixed by the audit above — mp4 export (#16), blank
+render (#25), image download (#10), audio on download (#30), filters not
+retained (#24), `EyeDropper is not defined` (#15, #18), the endless
+*Loading video…* (#28) and text selection while resizing the timeline (#5).
+GIF export (#21) works through the same ffmpeg path. #8 (video from the search
+tab) needs a Pixabay API key, so it stays untested; #29 and #4 carry no
+reproducer. The remaining three are fixed here:
+
+- [x] **#23 — Border radius behaved like a percentage** — `src/js/functions.js`,
+  `src/js/ui.js`, `src/js/events.js`, `src/js/database.js`
+  fabric applies `rx`/`ry` before the object's scale, and shapes are resized by
+  scaling, so a radius typed as 20 drew at 60 px on a rect scaled 3x while the
+  panel — which read `rx` back raw — still said 20. A new `cornerRadius` property
+  stores the pixel value the user asked for; `rx`/`ry` are derived from it
+  (`setCornerRadius`) and re-derived on `object:scaling` / `object:modified`
+  (`syncCornerRadius`), so the same number means the same pixels at any size.
+  `cornerRadius` was added to the four serialised property lists, and
+  `getCornerRadius` falls back to `rx * scaleX` for projects saved before it
+  existed. Known limit, unchanged from before: if `scaleX` is *keyframed*, the
+  drawn radius still varies over the animation.
+  Verified: typed 20 px draws a 20 px corner at scale 1 and at scale 3, the
+  panel keeps reading 20, and the value survives save, JSON round-trip and a
+  page reload.
+
+- [x] **#27 — Cropping a rotated image** — `src/js/functions.js`
+  `crop()` compared canvas-space edges and covered only three of the four
+  quadrants, so any rotated image cropped the wrong region — and nothing at all
+  when the crop window was centred on it, since all three branches need a
+  strictly positive offset. The guards added in the audit stopped the crash but
+  left the geometry wrong. `crop()` now works in the image's own frame
+  (`rotateVector`), clamps the region to the bitmap, and re-centres the object on
+  the region it kept; the crop window is created with the image's `angle`, and
+  the "expand back to the full bitmap" shift in `cropImage` is rotated the same
+  way. The four-branch soup is gone.
+  Verified: at 0°, 30° and 45°, the pixels under the crop window are byte
+  identical before and after the crop, everything outside it is dropped, and
+  crop mode exits cleanly.
+
+- [x] **#1 — A modal did not block the timeline** — `src/js/functions.js`
+  Modals are painted over the timeline but never took its pointer events, so the
+  resize handle, the seekbar, keyframes and layer bars all still reacted to a
+  drag behind the dialog. The onboarding modal from the report does not exist in
+  the open-source build; the export, import/export and credits modals all had
+  the bug. `dragTimeline`, `dragSeekBar`, `dragKeyframe` and `dragObjectProps`
+  now bail while `.modal-open` is present.
+  Verified: with the export modal open a real mouse drag on the handle leaves
+  the timeline height untouched, and dragging works again once it closes.

src/js/database.js

@@ -50,6 +50,7 @@ function checkDB() {
           'strokeUniform',
           'rx',
           'ry',
+          'cornerRadius',
           'selectable',
           'hasControls',
           'subTargetCheck',
@@ -226,6 +227,7 @@ async function autoSave() {
       'strokeUniform',
       'rx',
       'ry',
+      'cornerRadius',
       'selectable',
       'hasControls',
       'subTargetCheck',

src/js/events.js

@@ -100,6 +100,8 @@ $(document).ready(function () {
   canvas.on('object:scaling', function (e) {
     e.target.hasControls = false;
     centerLines(e);
+    // Keep the corner radius at its pixel value while the handle is dragged
+    syncCornerRadius(e.target);
     if (cropping) {
       updateCropBounds();
       crop(canvas.getItemById('cropped'));
@@ -135,12 +137,16 @@ $(document).ready(function () {
       canvas.renderAll();
       if (e.target.type == 'activeSelection') {
         const tempselection = canvas.getActiveObject();
+        // Discarding first bakes the group transform into the children, so
+        // their scale is final by the time the radius is re-derived
         canvas.discardActiveObject();
         e.target._objects.forEach(function (object) {
+          syncCornerRadius(object);
           autoKeyframe(object, e, true);
         });
         reselect(tempselection);
       } else {
+        syncCornerRadius(e.target);
         autoKeyframe(e.target, e, false);
       }
       updatePanelValues();

src/js/functions.js

@@ -495,6 +495,7 @@ async function updateRecordCanvas() {
     'strokeUniform',
     'rx',
     'ry',
+    'cornerRadius',
     'selectable',
     'hasControls',
     'subTargetCheck',
@@ -684,6 +685,7 @@ function save() {
     'strokeUniform',
     'rx',
     'ry',
+    'cornerRadius',
     'selectable',
     'hasControls',
     'subTargetCheck',
@@ -1050,13 +1052,57 @@ $(document).on('click', '#redo', function () {
   }
 });
 
+// Corner radius, in the canvas pixels the panel advertises.
+//
+// fabric applies rx/ry in the object's own coordinate space, before its scale,
+// and shapes are resized by scaling rather than by changing width/height. The
+// raw input was written straight to rx, so a radius typed as 20 drew at 60 on a
+// rect scaled 3x: the value behaved like a proportion of the shape instead of
+// pixels. `cornerRadius` stores what the user asked for and rx/ry are derived
+// from it, so the same number means the same number of pixels at any size.
+function getCornerRadius(object) {
+  if (!object) {
+    return 0;
+  }
+  if (typeof object.cornerRadius == 'number') {
+    return object.cornerRadius;
+  }
+  // Projects saved before cornerRadius existed only carry the raw rx
+  return (
+    (object.get('rx') || 0) * Math.abs(object.get('scaleX') || 1)
+  );
+}
+
+function setCornerRadius(object, radius) {
+  if (!object) {
+    return;
+  }
+  var px = radius > 0 ? radius : 0;
+  object.cornerRadius = px;
+  object.set({
+    rx: px / Math.abs(object.get('scaleX') || 1),
+    ry: px / Math.abs(object.get('scaleY') || 1),
+  });
+}
+
+// Re-derive rx/ry after a resize so the drawn radius keeps the typed value
+function syncCornerRadius(object) {
+  if (!object || object.get('type') != 'rect') {
+    return;
+  }
+  if (typeof object.cornerRadius != 'number') {
+    return;
+  }
+  setCornerRadius(object, object.cornerRadius);
+}
+
 // Generate keyframes
 function keyframeChanges(object, type, id, selection) {
   if (object.get('type') == 'rect') {
-    object.set({
-      rx: parseFloat($('#object-corners input').val()),
-      ry: parseFloat($('#object-corners input').val()),
-    });
+    setCornerRadius(
+      object,
+      parseFloat($('#object-corners input').val())
+    );
   } else if (object.get('type') == 'textbox') {
     object.set({
       charSpacing: parseFloat($('#text-h input').val()) * 10,
@@ -3512,7 +3558,23 @@ function checkCrop(obj) {
   crop(canvas.getItemById('cropped'));
 }
 
+// Rotate a vector by `radians` around the origin
+function rotateVector(x, y, radians) {
+  if (!radians) {
+    return { x: x, y: y };
+  }
+  var cos = Math.cos(radians);
+  var sin = Math.sin(radians);
+  return { x: x * cos - y * sin, y: x * sin + y * cos };
+}
+
 // Perform a crop
+//
+// Everything is computed in the image's own frame rather than along the canvas
+// axes, so a rotated image crops the region the crop window actually covers.
+// The previous version compared canvas-space edges and only handled three of
+// the four quadrants, which cropped the wrong area of any rotated image (and
+// nothing at all when the crop window was centred on it).
 function crop(obj) {
   var cropUI = canvas.getItemById('crop');
   if (!obj || !cropUI || !cropobj) {
@@ -3520,58 +3582,54 @@ function crop(obj) {
   }
   cropobj.setCoords();
   cropUI.setCoords();
-  var cleft =
-    cropUI.get('left') - (cropUI.get('width') * cropUI.get('scaleX')) / 2;
-  var ctop =
-    cropUI.get('top') - (cropUI.get('height') * cropUI.get('scaleY')) / 2;
-  var height =
-    (cropUI.get('height') / cropobj.get('scaleY')) * cropUI.get('scaleY');
-  var width =
-    (cropUI.get('width') / cropobj.get('scaleX')) * cropUI.get('scaleX');
-  var img_height = cropobj.get('height') * cropobj.get('scaleY');
-  var img_width = cropobj.get('width') * cropobj.get('scaleX');
-  var left =
-    cleft -
-    (cropobj.get('left') -
-      (cropobj.get('width') * cropobj.get('scaleX')) / 2);
-  var top =
-    ctop -
-    (cropobj.get('top') -
-      (cropobj.get('height') * cropobj.get('scaleY')) / 2);
-  if (left < 0 && top > 0) {
-    obj
-      .set({ cropY: top / cropobj.get('scaleY'), height: height })
-      .setCoords();
-    canvas.renderAll();
-    obj.set({
-      top: ctop + (obj.get('height') * obj.get('scaleY')) / 2,
-    });
-    canvas.renderAll();
-  } else if (top < 0 && left > 0) {
-    obj
-      .set({ cropX: left / cropobj.get('scaleX'), width: width })
-      .setCoords();
-    canvas.renderAll();
-    obj.set({
-      left: cleft + (obj.get('width') * obj.get('scaleX')) / 2,
-    });
-    canvas.renderAll();
-  } else if (top > 0 && left > 0) {
-    obj
-      .set({
-        cropX: left / cropobj.get('scaleX'),
-        cropY: top / cropobj.get('scaleY'),
-        height: height,
-        width: width,
-      })
-      .setCoords();
-    canvas.renderAll();
-    obj.set({
-      left: cleft + (obj.get('width') * obj.get('scaleX')) / 2,
-      top: ctop + (obj.get('height') * obj.get('scaleY')) / 2,
-    });
-    canvas.renderAll();
-  }
+  // Read everything before writing: the final call passes cropobj as `obj`
+  var scaleX = cropobj.get('scaleX') || 1;
+  var scaleY = cropobj.get('scaleY') || 1;
+  var ogWidth = cropobj.get('width');
+  var ogHeight = cropobj.get('height');
+  var centerX = cropobj.get('left');
+  var centerY = cropobj.get('top');
+  var radians = fabric.util.degreesToRadians(
+    cropobj.get('angle') || 0
+  );
+
+  // The crop window carries the image's angle, so its own width and height are
+  // already axis-aligned with the image
+  var width = (cropUI.get('width') * cropUI.get('scaleX')) / scaleX;
+  var height = (cropUI.get('height') * cropUI.get('scaleY')) / scaleY;
+
+  // Where the crop window sits relative to the image centre, un-rotated
+  var offset = rotateVector(
+    cropUI.get('left') - centerX,
+    cropUI.get('top') - centerY,
+    -radians
+  );
+  var cropX = offset.x / scaleX + ogWidth / 2 - width / 2;
+  var cropY = offset.y / scaleY + ogHeight / 2 - height / 2;
+
+  // Never leave the source bitmap: fabric draws nothing for a negative crop
+  width = Math.max(1, Math.min(width, ogWidth));
+  height = Math.max(1, Math.min(height, ogHeight));
+  cropX = Math.max(0, Math.min(cropX, ogWidth - width));
+  cropY = Math.max(0, Math.min(cropY, ogHeight - height));
+
+  // Put the cropped object back under the (clamped) region it represents
+  var center = rotateVector(
+    (cropX + width / 2 - ogWidth / 2) * scaleX,
+    (cropY + height / 2 - ogHeight / 2) * scaleY,
+    radians
+  );
+  obj
+    .set({
+      cropX: cropX,
+      cropY: cropY,
+      width: width,
+      height: height,
+      left: centerX + center.x,
+      top: centerY + center.y,
+    })
+    .setCoords();
+  canvas.renderAll();
   if (obj.get('id') != 'cropped') {
     canvas.remove(cropUI);
     canvas.remove(canvas.getItemById('crop-overlay'));
@@ -3626,20 +3684,23 @@ function cropImage(object) {
     cropobj = object;
     canvas.uniformScaling = false;
     cropobj.setCoords();
-    var left =
-      cropobj.get('left') -
-      (cropobj.get('width') * cropobj.get('scaleX')) / 2;
-    var top =
-      cropobj.get('top') -
-      (cropobj.get('height') * cropobj.get('scaleY')) / 2;
+    // The visible region, in the source bitmap's own pixels, before the image
+    // is expanded back to its full size below
     var cropx = cropobj.get('cropX');
     var cropy = cropobj.get('cropY');
+    var shownWidth = cropobj.get('width');
+    var shownHeight = cropobj.get('height');
+    var shownLeft = cropobj.get('left');
+    var shownTop = cropobj.get('top');
     overlay();
     var cropUI = new fabric.Rect({
       left: object.get('left'),
       top: object.get('top'),
       width: object.get('width') * object.get('scaleX') - 5,
       height: object.get('height') * object.get('scaleY') - 5,
+      // The crop window has to turn with the image, otherwise the region it
+      // covers cannot be expressed as a crop at all
+      angle: object.get('angle') || 0,
       originX: 'center',
       originY: 'center',
       id: 'crop',
@@ -3674,15 +3735,19 @@ function cropImage(object) {
       })
       .setCoords();
     canvas.renderAll();
+    // Growing back to the full bitmap moves the centre, so shift the image so
+    // the region that was on screen stays exactly where it was. The offset is
+    // in the image's own frame, hence the rotation.
+    var shift = rotateVector(
+      (cropx + shownWidth / 2 - cropobj.get('width') / 2) *
+        cropobj.get('scaleX'),
+      (cropy + shownHeight / 2 - cropobj.get('height') / 2) *
+        cropobj.get('scaleY'),
+      fabric.util.degreesToRadians(cropobj.get('angle') || 0)
+    );
     cropobj.set({
-      left:
-        left +
-        (cropobj.get('width') * cropobj.get('scaleX')) / 2 -
-        cropx * cropobj.get('scaleX'),
-      top:
-        top +
-        (cropobj.get('height') * cropobj.get('scaleY')) / 2 -
-        cropy * cropobj.get('scaleY'),
+      left: shownLeft - shift.x,
+      top: shownTop - shift.y,
     });
     cropUI.setControlsVisibility({
       mt: false,
@@ -4058,6 +4123,7 @@ function newRectangle(color) {
     backgroundColor: 'rgba(255,255,255,0)',
     rx: 0,
     ry: 0,
+    cornerRadius: 0,
     fill: color,
     cursorWidth: 1,
     cursorDuration: 1,
@@ -4862,9 +4928,17 @@ function bindPointerDrag(e, el, onMove, onEnd) {
   window.addEventListener('blur', end);
 }
 
+// A modal covers the timeline but does not cover pointer events, so every
+// timeline drag has to opt out while one is open - otherwise the seekbar,
+// keyframes, layer bars and the resize handle all still react to a drag
+// happening "behind" the dialog.
+function modalOpen() {
+  return $('.modal-open').length > 0;
+}
+
 // Dragging a keyframe
 function dragKeyframe(e) {
-  if (e.which == 3) {
+  if (e.which == 3 || modalOpen()) {
     return false;
   }
   e.stopPropagation();
@@ -5014,7 +5088,7 @@ function updateTime(drag, check) {
 
 // Dragging the seekbar
 function dragSeekBar(e) {
-  if (e.which == 3) {
+  if (e.which == 3 || modalOpen()) {
     return false;
   }
   // Stop the browser from turning the press into a native drag-and-drop.
@@ -5085,7 +5159,7 @@ $(document).on(
 
 // Dragging layer horizontally
 function dragObjectProps(e) {
-  if (e.which == 3) {
+  if (e.which == 3 || modalOpen()) {
     return false;
   }
   var drag = $(this).parent();
@@ -5299,7 +5373,7 @@ function resetHeight() {
 
 // Dragging timeline vertically
 function dragTimeline(e) {
-  if (e.which == 3) {
+  if (e.which == 3 || modalOpen()) {
     return false;
   }
   // Suppress text selection for the duration of the drag only - leaving these

src/js/ui.js

@@ -570,7 +570,7 @@ function updatePanelValues() {
     o_slider.setValue(object.get('opacity') * 100);
     if (object.get('type') == 'rect') {
       $('#object-corners input').val(
-        parseFloat(object.get('rx').toFixed(2))
+        parseFloat(getCornerRadius(object).toFixed(2))
       );
       colormode = 'fill';
       o_fill.setColor(object.get('fill'));