fix: improve focal points draggable style/perf (#1371)

* fix: improve focal points draggable style/perf

* remove unnecessary global

* fix all the things

* fix comment

package.json

@@ -46,7 +46,6 @@
     "@babel/core": "^7.5.0",
     "@gamestdio/websocket": "^0.3.2",
     "@webcomponents/custom-elements": "^1.2.4",
-    "@wessberg/pointer-events": "^1.0.9",
     "babel-loader": "^8.0.6",
     "babel-plugin-transform-react-remove-prop-types": "^0.4.24",
     "cheerio": "^1.0.0-rc.2",
@@ -150,7 +149,8 @@
       "CSS",
       "customElements",
       "AbortController",
-      "matchMedia"
+      "matchMedia",
+      "MessageChannel"
     ],
     "ignore": [
       "dist",

src/routes/_components/Draggable.html

@@ -1,13 +1,13 @@
-<div class="draggable-area {draggableClass}"
+<div class="draggable-area {draggableClassAfterRaf}"
-     on:pointermove="onPointerMove(event)"
+     on:pointerMove="onPointerMove(event)"
-     on:pointerleave="onPointerLeave(event)"
+     on:pointerLeave="onPointerLeave(event)"
+     on:pointerUp="onPointerUp(event)"
      on:click="onClick(event)"
      ref:area
 >
-  <div class="draggable-indicator {indicatorClass}"
+  <div class="draggable-indicator {indicatorClassAfterRaf}"
-       style={indicatorStyle}
+       style={indicatorStyleAfterRaf}
-       on:pointerdown="onPointerDown(event)"
+       on:pointerDown="onPointerDown(event)"
-       on:pointerup="onPointerUp(event)"
        ref:indicator
   >
     <div class="draggable-indicator-inner">
@@ -30,19 +30,60 @@
   }
 </style>
 <script>
-  import { throttleRaf } from '../_utils/throttleRaf'
+  import { observe } from 'svelte-extras'
+  import {
+    throttleRequestAnimationFrame,
+    throttleRequestPostAnimationFrame
+  } from '../_utils/throttleTimers'
+  import { pointerUp, pointerDown, pointerLeave, pointerMove } from '../_utils/pointerEvents'
+
+  // ensure DOM writes only happen once after a rAF
+  const updateIndicatorStyle = throttleRequestAnimationFrame()
+  const updateIndicatorClass = throttleRequestAnimationFrame()
+  const updateDraggableClass = throttleRequestAnimationFrame()
+
+  // ensure DOM reads only happen once after a rPAF
+  const calculateGBCR = throttleRequestPostAnimationFrame()
 
   const clamp = x => Math.max(0, Math.min(1, x))
-  const throttledRaf = throttleRaf()
 
   export default {
+    oncreate () {
+      this.observe('dragging', dragging => {
+        if (dragging) {
+          this.fire('dragStart')
+        } else {
+          this.fire('dragEnd')
+        }
+      }, { init: false })
+      this.observe('indicatorStyle', indicatorStyle => {
+        console.log('Draggable indicatorStyle', indicatorStyle)
+        updateIndicatorStyle(() => {
+          this.set({ indicatorStyleAfterRaf: indicatorStyle })
+        })
+      })
+      this.observe('indicatorClass', indicatorClass => {
+        updateIndicatorClass(() => {
+          this.set(({ indicatorClassAfterRaf: indicatorClass }))
+        })
+      })
+      this.observe('draggableClass', draggableClass => {
+        updateDraggableClass(() => {
+          this.set({ draggableClassAfterRaf: draggableClass })
+        })
+      })
+    },
     data: () => ({
+      dragging: false,
       draggableClass: '',
+      draggableClassAfterRaf: '',
       indicatorClass: '',
+      indicatorClassAfterRaf: '',
       x: 0,
       y: 0,
       indicatorWidth: 0,
-      indicatorHeight: 0
+      indicatorHeight: 0,
+      indicatorStyleAfterRaf: ''
     }),
     computed: {
       indicatorStyle: ({ x, y, indicatorWidth, indicatorHeight }) => (
@@ -50,10 +91,12 @@
       )
     },
     methods: {
+      observe,
       onPointerDown (e) {
-        e.preventDefault()
+        console.log('Draggable: onPointerDown')
-        e.stopPropagation()
         const rect = this.refs.indicator.getBoundingClientRect()
+        console.log('Draggable: e.clientX', e.clientX)
+        console.log('Draggable: e.clientY', e.clientY)
         this.set({
           dragging: true,
           dragOffsetX: e.clientX - rect.left,
@@ -61,11 +104,11 @@
         })
       },
       onPointerMove (e) {
-        if (this.get().dragging) {
+        console.log('Draggable: onPointerMove')
-          e.preventDefault()
+        const { dragging, indicatorWidth, indicatorHeight, dragOffsetX, dragOffsetY } = this.get()
-          e.stopPropagation()
+        if (dragging) {
-          const { indicatorWidth, indicatorHeight, dragOffsetX, dragOffsetY } = this.get()
+          console.log('Draggable: dragging')
-          throttledRaf(() => {
+          calculateGBCR(() => {
             const rect = this.refs.area.getBoundingClientRect()
             const offsetX = dragOffsetX - (indicatorWidth / 2)
             const offsetY = dragOffsetY - (indicatorHeight / 2)
@@ -77,19 +120,19 @@
         }
       },
       onPointerUp (e) {
-        e.preventDefault()
+        console.log('Draggable: onPointerUp')
-        e.stopPropagation()
         this.set({ dragging: false })
       },
       onPointerLeave (e) {
-        e.preventDefault()
+        console.log('Draggable: onPointerLeave')
-        e.stopPropagation()
         this.set({ dragging: false })
       },
       onClick (e) {
+        console.log('Draggable: onClick')
+        console.log('Draggable: target classList', e.target.classList)
+        console.log('Draggable: currentTarget classList', e.currentTarget.classList)
         if (!e.target.classList.contains('draggable-indicator')) {
-          e.preventDefault()
+          console.log('Draggable: onClick handled')
-          e.stopPropagation()
           const rect = this.refs.area.getBoundingClientRect()
           const x = clamp((e.clientX - rect.left) / rect.width)
           const y = clamp((e.clientY - rect.top) / rect.height)
@@ -97,6 +140,12 @@
           this.fire('change', { x, y })
         }
       }
+    },
+    events: {
+      pointerUp,
+      pointerDown,
+      pointerLeave,
+      pointerMove
     }
   }
 </script>

src/routes/_components/dialog/components/MediaFocalPointDialog.html

@@ -25,12 +25,14 @@
         <!-- 52px == 32px icon width + 10px padding -->
         <Draggable
           draggableClass="media-draggable-area-inner"
-          indicatorClass="media-focal-point-indicator {imageLoaded ? '': 'hidden'}"
+          indicatorClass="media-focal-point-indicator {imageLoaded ? '': 'hidden'} {dragging ? 'dragging' : ''}"
           indicatorWidth={52}
           indicatorHeight={52}
           x={indicatorX}
           y={indicatorY}
           on:change="onDraggableChange(event)"
+          on:dragStart="set({dragging: true})"
+          on:dragEnd="set({dragging: false})"
         >
           <SvgIcon
             className="media-focal-point-indicator-svg"
@@ -142,6 +144,14 @@
     display: flex;
   }
 
+  :global(.media-focal-point-indicator:hover) {
+    background: var(--focal-bg-hover);
+  }
+
+  :global(.media-focal-point-indicator.dragging) {
+    background: var(--focal-bg-drag);
+  }
+
   :global(.media-draggable-area-inner) {
     width: 100%;
     height: 100%;
@@ -177,13 +187,17 @@
   import { store } from '../../../_store/store'
   import { get } from '../../../_utils/lodash-lite'
   import { observe } from 'svelte-extras'
-  import debounce from 'lodash-es/debounce'
   import { scheduleIdleTask } from '../../../_utils/scheduleIdleTask'
   import { coordsToPercent, percentToCoords } from '../../../_utils/coordsToPercent'
   import SvgIcon from '../../SvgIcon.html'
   import { intrinsicScale } from '../../../_thirdparty/intrinsic-scale/intrinsicScale'
   import { resize } from '../../../_utils/events'
   import Draggable from '../../Draggable.html'
+  import { throttleScheduleIdleTask } from '../../../_utils/throttleTimers'
+
+  // Updating the focal points in the store causes a lot of computations (extra JS work),
+  // so we really don't want to do it for every drag event.
+  const updateFocalPointsInStore = throttleScheduleIdleTask()
 
   const parseAndValidateFloat = rawText => {
     let float = parseFloat(rawText)
@@ -208,6 +222,7 @@
       Draggable
     },
     data: () => ({
+      dragging: false,
       rawFocusX: '0',
       rawFocusY: '0',
       containerWidth: 0,
@@ -276,8 +291,6 @@
         })
       },
       setupSyncToStore () {
-        const saveStore = debounce(() => scheduleIdleTask(() => this.store.save()), 1000)
-
         const observeAndSync = (rawKey, key) => {
           this.observe(rawKey, rawFocus => {
             const { realm, index, media } = this.get()
@@ -285,7 +298,7 @@
             if (media[index][key] !== rawFocusDecimal) {
               media[index][key] = rawFocusDecimal
               this.store.setComposeData(realm, { media })
-              saveStore()
+              scheduleIdleTask(() => this.store.save())
             }
           }, { init: false })
         }
@@ -294,16 +307,16 @@
         observeAndSync('rawFocusY', 'focusY')
       },
       onDraggableChange ({ x, y }) {
-        const saveStore = debounce(() => scheduleIdleTask(() => this.store.save()), 1000)
+        updateFocalPointsInStore(() => {
-
+          const focusX = parseAndValidateFloat(percentToCoords(x * 100))
-        scheduleIdleTask(() => {
+          const focusY = parseAndValidateFloat(percentToCoords(100 - (y * 100)))
-          const focusX = percentToCoords(x * 100)
-          const focusY = percentToCoords(100 - (y * 100))
           const { realm, index, media } = this.get()
-          media[index].focusX = parseAndValidateFloat(focusX)
+          if (media[index].focusX !== focusX || media[index].focusY !== focusY) {
-          media[index].focusY = parseAndValidateFloat(focusY)
+            media[index].focusX = focusX
+            media[index].focusY = focusY
             this.store.setComposeData(realm, { media })
-          saveStore()
+            scheduleIdleTask(() => this.store.save())
+          }
         })
       },
       measure () {

src/routes/_utils/asyncPolyfills.js

@@ -13,7 +13,3 @@ export const importIndexedDBGetAllShim = () => import(
 export const importCustomElementsPolyfill = () => import(
   /* webpackChunkName: '$polyfill$-@webcomponents/custom-elements' */ '@webcomponents/custom-elements'
   )
-
-export const importPointerEventsPolyfill = () => import(
-  /* webpackChunkName: '$polyfill$-@wessberg/pointer-events' */ '@wessberg/pointer-events'
-  )

src/routes/_utils/loadPolyfills.js

@@ -2,8 +2,7 @@ import {
   importCustomElementsPolyfill,
   importIndexedDBGetAllShim,
   importIntersectionObserver,
-  importRequestIdleCallback,
+  importRequestIdleCallback
-  importPointerEventsPolyfill
 } from './asyncPolyfills'
 
 export function loadPolyfills () {
@@ -11,7 +10,6 @@ export function loadPolyfills () {
     typeof IntersectionObserver === 'undefined' && importIntersectionObserver(),
     typeof requestIdleCallback === 'undefined' && importRequestIdleCallback(),
     !IDBObjectStore.prototype.getAll && importIndexedDBGetAllShim(),
-    typeof customElements === 'undefined' && importCustomElementsPolyfill(),
+    typeof customElements === 'undefined' && importCustomElementsPolyfill()
-    typeof PointerEvent === 'undefined' && importPointerEventsPolyfill()
   ])
 }

src/routes/_utils/pointerEvents.js

@@ -0,0 +1,53 @@
+import { get } from './lodash-lite'
+
+const hasPointerEvents = process.browser && typeof PointerEvent === 'function'
+
+// Epiphany browser reports that it's a touch device even though it's not
+const isTouchDevice = process.browser && 'ontouchstart' in document && !/Epiphany/.test(navigator.userAgent)
+
+let pointerDown
+let pointerUp
+let pointerLeave
+let pointerMove
+
+function createEventListener (event) {
+  return (node, callback) => {
+    const listener = e => {
+      // lightweight polyfill for clientX/clientY in pointer events,
+      // which is slightly different in touch events
+      if (typeof e.clientX !== 'number') {
+        e.clientX = get(e, ['touches', 0, 'clientX'])
+      }
+      if (typeof e.clientY !== 'number') {
+        e.clientY = get(e, ['touches', 0, 'clientY'])
+      }
+      callback(e)
+    }
+
+    node.addEventListener(event, listener)
+    return {
+      destroy () {
+        node.removeEventListener(event, listener)
+      }
+    }
+  }
+}
+
+if (hasPointerEvents) {
+  pointerDown = createEventListener('pointerdown')
+  pointerUp = createEventListener('pointerup')
+  pointerLeave = createEventListener('pointerleave')
+  pointerMove = createEventListener('pointermove')
+} else if (isTouchDevice) {
+  pointerDown = createEventListener('touchstart')
+  pointerUp = createEventListener('touchend')
+  pointerLeave = createEventListener('touchend')
+  pointerMove = createEventListener('touchmove')
+} else {
+  pointerDown = createEventListener('mousedown')
+  pointerUp = createEventListener('mouseup')
+  pointerLeave = createEventListener('mouseleave')
+  pointerMove = createEventListener('mousemove')
+}
+
+export { pointerDown, pointerUp, pointerLeave, pointerMove }

src/routes/_utils/requestPostAnimationFrame.js

@@ -0,0 +1,9 @@
+// modeled after https://github.com/andrewiggins/afterframe
+// see also https://github.com/WICG/requestPostAnimationFrame
+export const requestPostAnimationFrame = cb => {
+  requestAnimationFrame(() => {
+    const channel = new MessageChannel()
+    channel.port1.onmessage = cb
+    channel.port2.postMessage(undefined)
+  })
+}

src/routes/_utils/throttleRaf.js

@@ -1,18 +0,0 @@
-// ensure callback is only executed once per raf
-export const throttleRaf = () => {
-  let rafCallback
-  let rafQueued
-
-  return function throttledRaf (callback) {
-    rafCallback = callback
-    if (!rafQueued) {
-      rafQueued = true
-      requestAnimationFrame(() => {
-        const cb = rafCallback
-        rafCallback = null
-        rafQueued = false
-        cb()
-      })
-    }
-  }
-}

src/routes/_utils/throttleTimers.js

@@ -0,0 +1,28 @@
+// Sometimes we want to queue multiple requestAnimationFrames but only run the last one.
+// It's tedious to do this using cancelAnimationFrame, so this is a utility to throttle
+// a timer such that it only runs the last callback when it fires.
+
+import { requestPostAnimationFrame } from './requestPostAnimationFrame'
+import { scheduleIdleTask } from './scheduleIdleTask'
+
+const throttle = (timer) => {
+  return () => {
+    let queuedCallback
+
+    return function throttledRaf (callback) {
+      const alreadyQueued = !!queuedCallback
+      queuedCallback = callback
+      if (!alreadyQueued) {
+        timer(() => {
+          const cb = queuedCallback
+          queuedCallback = null
+          cb()
+        })
+      }
+    }
+  }
+}
+
+export const throttleRequestAnimationFrame = throttle(requestAnimationFrame)
+export const throttleRequestPostAnimationFrame = throttle(requestPostAnimationFrame)
+export const throttleScheduleIdleTask = throttle(scheduleIdleTask)

src/scss/themes/_base.scss

@@ -124,5 +124,7 @@
   --focal-img-backdrop-filter: #{rgba($main-bg-color, 0.5)};
   --focal-img-bg: #{rgba($main-bg-color, 0.3)};
   --focal-bg: #{rgba($toast-bg, 0.8)};
+  --focal-bg-drag: #{rgba($toast-bg, 0.9)};
+  --focal-bg-hover: #{lighten(rgba($toast-bg, 0.8), 5%)};
   --focal-color: #{$secondary-text-color};
 }