Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CustomDragLayer slow performance #592

Closed
jr69 opened this issue Dec 7, 2016 · 47 comments
Closed

CustomDragLayer slow performance #592

jr69 opened this issue Dec 7, 2016 · 47 comments

Comments

@jr69
Copy link

jr69 commented Dec 7, 2016

Is there any reason why the CustomDragLayer for me is giving me bad performance? Its like theres low fps, or rather, the arbitrary object that I am dragging around is lagging and not looking smooth

@choffmeister
Copy link

I also experienced this. The problem here is, that even if you render pure components that don't change in the drag layer, the offset change triggers a at least a react reconciliation on every single mouse move. This is very inefficient if one has the common case, where the drag layer should just display a non-changing custom preview element.

To work around that, I duplicated the DragLayer.js the does the offset rendering for me and this in a high performant manner, meaning changing the style of the container that moves around the preview directly.

// PerformantDragLayer.js
/* eslint-disable */
import React, { Component, PropTypes } from 'react';
import shallowEqual from 'react-dnd/lib/utils/shallowEqual';
import shallowEqualScalar from 'react-dnd/lib/utils/shallowEqualScalar';
import isPlainObject from 'lodash/isPlainObject';
import invariant from 'invariant';
import checkDecoratorArguments from 'react-dnd/lib/utils/checkDecoratorArguments';
import hoistStatics from 'hoist-non-react-statics';

function layerStyles(isDragging) {
  return {
    position: 'fixed',
    pointerEvents: 'none',
    zIndex: 1,
    left: 0,
    top: 0,
    width: isDragging ? '100%' : 0,
    height: isDragging ? '100%' : 0,
    opacity: isDragging ? 1 : 0
  };
}

export default function DragLayer(collect, options = {}) {
  checkDecoratorArguments('DragLayer', 'collect[, options]', ...arguments);
  invariant(
    typeof collect === 'function',
    'Expected "collect" provided as the first argument to DragLayer ' +
    'to be a function that collects props to inject into the component. ',
    'Instead, received %s. ' +
    'Read more: http://gaearon.github.io/react-dnd/docs-drag-layer.html',
    collect
  );
  invariant(
    isPlainObject(options),
    'Expected "options" provided as the second argument to DragLayer to be ' +
    'a plain object when specified. ' +
    'Instead, received %s. ' +
    'Read more: http://gaearon.github.io/react-dnd/docs-drag-layer.html',
    options
  );

  return function decorateLayer(DecoratedComponent) {
    const { arePropsEqual = shallowEqualScalar } = options;
    const displayName =
      DecoratedComponent.displayName ||
      DecoratedComponent.name ||
      'Component';

    class DragLayerContainer extends Component {
      static DecoratedComponent = DecoratedComponent;

      static displayName = `DragLayer(${displayName})`;

      static contextTypes = {
        dragDropManager: PropTypes.object.isRequired
      }

      getDecoratedComponentInstance() {
        return this.refs.child;
      }

      shouldComponentUpdate(nextProps, nextState) {
        return !arePropsEqual(nextProps, this.props) ||
          !shallowEqual(nextState, this.state);
      }

      constructor(props, context) {
        super(props);
        this.handleOffsetChange = this.handleOffsetChange.bind(this);
        this.handleStateChange = this.handleStateChange.bind(this);

        this.manager = context.dragDropManager;
        invariant(
          typeof this.manager === 'object',
          'Could not find the drag and drop manager in the context of %s. ' +
          'Make sure to wrap the top-level component of your app with DragDropContext. ' +
          'Read more: http://gaearon.github.io/react-dnd/docs-troubleshooting.html#could-not-find-the-drag-and-drop-manager-in-the-context',
          displayName,
          displayName
        );

        this.state = this.getCurrentState();
      }

      componentDidMount() {
        this.isCurrentlyMounted = true;

        const monitor = this.manager.getMonitor();
        this.unsubscribeFromOffsetChange = monitor.subscribeToOffsetChange(
          this.handleOffsetChange
        );
        this.unsubscribeFromStateChange = monitor.subscribeToStateChange(
          this.handleStateChange
        );

        this.handleStateChange();
      }

      componentWillUnmount() {
        this.isCurrentlyMounted = false;

        this.unsubscribeFromOffsetChange();
        this.unsubscribeFromStateChange();
      }

      handleOffsetChange() {
        if (!this.isCurrentlyMounted) {
          return;
        }

        const monitor = this.manager.getMonitor();
        const offset = monitor.getSourceClientOffset();
        const offsetDiv = this.refs.offset;
        if (offset && offsetDiv) {
          offsetDiv.style.transform = `translate(${offset.x}px, ${offset.y}px)`;
        }
      }

      handleStateChange() {
        if (!this.isCurrentlyMounted) {
          return;
        }

        const nextState = this.getCurrentState();
        if (!shallowEqual(nextState, this.state)) {
          this.setState(nextState);
        }
      }

      getCurrentState() {
        const monitor = this.manager.getMonitor();
        return {
          collected: collect(monitor),
          isDragging: monitor.isDragging()
        };
      }

      render() {
        return (
          <div style={layerStyles(this.state.isDragging)}>
            <div ref='offset'>
              {this.state.isDragging && <DecoratedComponent {...this.props} {...this.state.collected} ref='child' />}
            </div>
          </div>
        );
      }
    }

    return hoistStatics(DragLayerContainer, DecoratedComponent);
  };
}

This can then be used the following way:

// MyCustomDragLayer.js
import PerformantDragLayer from './PerformantDragLayer'
import React, {PropTypes} from 'react'

class CustomDragLayerRaw extends React.Component {
  static propTypes = {
    item: PropTypes.any,
    itemType: PropTypes.string
  }

  render () {
    const {item, itemType} = this.props
    return <div>{itemType}</div>
  }
}

export default PerformantDragLayer((monitor) => ({
  item: monitor.getItem(),
  itemType: monitor.getItemType()
})(CustomDragLayerRaw)

The performance improvment is very noticable:

drag-layer-default

drag-layer-performant

Of course the default DragLayer is still more flexible. My more performant implementation is just faster, because it handles a special case. But I guess this special case is very common.

@gaearon Is such a specialized but way more performant implementation interesting for you to be integrated as alternative into react-dnd?

@jr69
Copy link
Author

jr69 commented Dec 9, 2016

@choffmeister Hi. Thanks for your post. I had a brief look at it a day ago but only looked at it in depth just then. The code you posted seems very specialised within your implementation, could you please perhaps point out the important aspects needed for this? And how that CustomDragLayerRaw is used would be very benefitial too.

I am not sure if all of those plugins, libraries and other code you have is part of the solution or not.

Thanks

@jr69
Copy link
Author

jr69 commented Dec 10, 2016

I have experimented around and managed to make it work. It seems that the content you want in your "drag ghost thing" should be in the 'offset' div. I don't know the optimal way to do this, but in my test, I just put a similar div to the dragged content. I've yet to make the drag layer item the same as the dragged item, but passing in some data and changing the drag layer item's state isn't too hard to do.

The performance is very good, though it only works when dragging outside of the container. If you drag in circles around the item, the performance is also as flawless as the previous case (i.e. you have a list of 4 items, dragging and keeping the mouse in the area of the 4th item). However, if you draw circles with your mouse, dragging, across all the 4 elements, it still lags, like before, but with a slight fps improvement (nonetheless its still laggy). But this implementation is a good start. At least for most common cases when you want to drag from one container to another, then this will definitely keep the UX good.

@choffmeister
Copy link

choffmeister commented Dec 10, 2016

What exactly do you mean by laggy:

  1. Is the FPS to low so that it looks stuttering, or
  2. is the element moving smoothly, but while moving it is always some distance behind the mouse movement?

@jr69
Copy link
Author

jr69 commented Dec 10, 2016

I'd say both

@jr69
Copy link
Author

jr69 commented Dec 11, 2016

Upon further usage, I realise that the solution doesn't really eliminate lag altogether. I have a card div that has quiet a bit of features in it. Your solution did initially provide a huge fps boost compared to the example in the docs, however, as I make my div more complicated, it starts to lag like I didn't have this.

I have no idea if its semantic ui causing this or react dnd. If there is any other more optimal way to have the drag layer very performant, I would really like to know.

Also if anyone would like to try and see if it also lags for them, I had used a Semantic UI card, with lots of elements inside of it. My placeholder card had 2 titles, 5 semantic "label"s, 3 semantic "icon"s, and an avatar image.

@jr69
Copy link
Author

jr69 commented Dec 12, 2016

After further testing with this different example at: http://react-trello-board.web-pal.com/ (its implementation is very similar to what choffmeister had posted earlier), I still obtain lag when trying to drag my complicated div. I'm glad that this is the conclusion I have come to, as I don't need to do some more experimenting with code; the problem lies with my div, which can be easily fixed.

@jr69 jr69 closed this as completed Dec 12, 2016
@framerate
Copy link

I'm still experiencing this pretty badly using VERY similar code to the example in the docs. It must be the transform calculation having every tiny mouse move?

Has anyone else found a solution to this?

@laurenkt
Copy link

I've had a performance problem, even with PureComponents. Using the 'Highlight Updates' feature in the Chrome React Dev plug-in to diagnose seems like all my components were being updated when the currentOffset prop was getting changed. To mitigate I made sure that the DragLayer that passes on this prop only contains the drag preview itself, even though there was already a DragLayer at the top-level of the app to track what item is being dragged.

I haven't tried adapting @choffmeister's solution but it looks like it would also solve the issue. I think perhaps something like that should be considered for the core library because I can only imagine that this is a common problem for anyone implementing drag previews.

@framerate
Copy link

I made sure that the DragLayer that passes on this prop only contains the drag preview itself, even though there was already a DragLayer at the top-level of the app

I'm not sure I follow! I only have the one custom drag layer in my app, I believe!

@laurenkt
Copy link

It's a problem specific to my use case – a drag layer was already being used before a custom drag preview was needed, in order to track what item was being dragged. I had initially thought it made sense to use the same drag layer to do the custom drag preview when I added it but, since this was at the top level of my application, changing the props for this on every slight movement was causing a lot of extra component updates.

@framerate
Copy link

Gotcha, thanks.

I simplified my drag layer down to a single dumb component, so as far as I can tell it's the constant x/y calculation that's causing the lag.

I don't quite follow what @choffmeister is doing above yet but it looks like I'm going to have to take a closer look.

@gaearon This seems to be a pretty decent problem for the lib, any suggestions on what the proper fix might be?

@dobesv
Copy link

dobesv commented Mar 3, 2018

It seems like the react-dnd-html5-backend has terrible performance with a custom DragLayer whereas the react-dnd-touch-backend has OK performance.

@dusanjovanov
Copy link

Kinda obvious, but it helps:

let updates = 0;
@DragLayer(monitor => {
  if (updates++ % 2 === 0) {
    return {
      currentOffset: monitor.getSourceClientOffset()
    }
  }
  else {
    return {}
  }
})

@aks427
Copy link

aks427 commented Apr 10, 2018

Another workaround is to use the react-throttle-render package to throttle the render method of the DragLayer.

https://www.npmjs.com/package/react-throttle-render

@framerate
Copy link

Thanks @aks427 I'll try that and see if it improves further (using a custom drag layer like above too which helped a lot in MOST cases but not all)

@macoto-liu
Copy link

@dobesv change to react-dnd-touch-backend solved the problem. I also tried react-throttle-render, but still seems not good.

@dobesv
Copy link

dobesv commented Jun 5, 2018

Yeah, I know the touch backend works, but it doesn't support drag and drop file uploads, which I want to use.

@macoto-liu
Copy link

macoto-liu commented Jun 5, 2018

@dobesv we use https://react-dropzone.js.org/ for drag and drop files :) maybe it's good for you as well.

@dobesv ignore my comments. react-dropzone only supports drop files.

but for more common senario, if the file is needs to be uploaded, the file should be out of the browser. we only need drop files should be ok.

@framerate
Copy link

framerate commented Jun 5, 2018 via email

@stellarhoof
Copy link

@framerate Even using PureComponents it's an issue because the reconciliation algo is expensive to run on every mouse move. Just profile ur app with Chrome DevTools and CPU throttling to 4X, which is a realistic slowdown for mid-end mobile devices.

@stellarhoof
Copy link

stellarhoof commented Jun 22, 2018

For posterity and anyone else who is struggling with the drag performance and you don't want to pull lots of code as in @choffmeister solution:

let subscribedToOffsetChange = false;

let dragPreviewRef = null;

const onOffsetChange = monitor => () => {
  if (!dragPreviewRef) return;

  const offset = monitor.getSourceClientOffset();
  if (!offset) return;

  const transform = `translate(${offset.x}px, ${offset.y}px)`;
  dragPreviewRef.style["transform"] = transform;
  dragPreviewRef.style["-webkit-transform"] = transform;
};

export default DragLayer(monitor => {
  if (!subscribedToOffsetChange) {
    monitor.subscribeToOffsetChange(onOffsetChange(monitor));
    subscribedToOffsetChange = true;
  }

  return {
    itemBeingDragged: monitor.getItem(),
    componentType: monitor.getItemType(),
    beingDragged: monitor.isDragging()
  };
})(
  class CustomDragLayer extends React.PureComponent {
    componentDidUpdate(prevProps) {
      dragPreviewRef = this.rootNode;
    }

    render() {
      if (!this.props.beingDragged) return null;
      return (
        <div
          role="presentation"
          ref={el => (this.rootNode = el)}
          style={{
            position: "fixed",
            pointerEvents: "none",
            top: 0,
            left: 0
          }}
        >
          {renderComponent(
            this.props.componentType,
            this.props.itemBeingDragged
          )}
        </div>
      );
    }
  }
);

@Vacummus
Copy link

Vacummus commented Aug 31, 2018

@stellarhoof Thank you for the great answer! Unfortunately the solution does not work on IE11 for me. subscribeToOffsetChange does not seem to call the callback we pass into it. Fortunately, I was able to fix it by not using subscribeToOffsetChange, but instead just setting the translations inside of the collector like so:

let dragLayerRef: HTMLElement = null;

const layerStyles: React.CSSProperties = {
  position: 'fixed',
  pointerEvents: 'none',
  zIndex: 100,
  left: 0,
  top: 0,
  width: '100%',
  height: '100%',
};

const dragLayerCollector = (monitor: DragLayerMonitor) => {
  if (dragLayerRef) {
    const offset = monitor.getSourceClientOffset() || monitor.getInitialClientOffset();

    if (offset) {
      dragLayerRef.style["transform"] = `translate(${offset.x}px, ${offset.y}px)`;
    } else {
      dragLayerRef.style["display"] = `none`;
    }
  }

  return {
    item: monitor.getItem(),
    isDragging: monitor.isDragging(),
  };
};

export default DragLayer(dragLayerCollector)(
  (props): JSX.Element => {
    if (!props.isDragging) {
      return null;
    }
		
    return (
      <div style={layerStyles}>
        <div ref={ (ref) => dragLayerRef = ref }>test</div>
      </div>
    );
  }
);

@framerate
Copy link

@stellarhoof I notice renderComponent isn't defined. Was this part of a larger file? (React isn't imported as well)

@framerate
Copy link

@choffmeister Have you updated for later versions of dnd? It would appear changes to context have broken your implementation and I wanted to try it and compare to what @stellarhoof was doing

@orlandovallejos
Copy link

@stellarhoof Thank you for the great answer! Unfortunately the solution does not work on IE11 for me. subscribeToOffsetChange does not seem to call the callback we pass into it. Fortunately, I was able to fix it by not using subscribeToOffsetChange, but instead just setting the translations inside of the collector like so:

let dragLayerRef: HTMLElement = null;

const layerStyles: React.CSSProperties = {
  position: 'fixed',
  pointerEvents: 'none',
  zIndex: 100,
  left: 0,
  top: 0,
  width: '100%',
  height: '100%',
};

const dragLayerCollector = (monitor: DragLayerMonitor) => {
  if (dragLayerRef) {
    const offset = monitor.getSourceClientOffset() || monitor.getInitialClientOffset();

    if (offset) {
      dragLayerRef.style["transform"] = `translate(${offset.x}px, ${offset.y}px)`;
    } else {
      dragLayerRef.style["display"] = `none`;
    }
  }

  return {
    item: monitor.getItem(),
    isDragging: monitor.isDragging(),
  };
};

export default DragLayer(dragLayerCollector)(
  (props): JSX.Element => {
    if (!props.isDragging) {
      return null;
    }
		
    return (
      <div style={layerStyles}>
        <div ref={ (ref) => dragLayerRef = ref }>test</div>
      </div>
    );
  }
);

This worked for me!
I'm using this version: 9.4.0

The others solutions seems to work only for older versions.

@njerschow
Copy link

Hey Guys,

Don't try to make a css animated component draggable, or at least remove the transition property before proceeding.

After removing the transition property onStart and adding it back onEnd, everything worked properly

@fromi
Copy link

fromi commented May 13, 2020

For people that uses hooks (useDragLayer hook) and lands here: I opened a ticket specifically about the hook implementation, with a workaround proposal here : #2414

@d07RiV
Copy link

d07RiV commented Nov 28, 2020

Agreed, hooks implementation is a little optimistic in regards to performance. Mostly because of missing dependencies in useEffect, but even fixing those it seems to lag a bit (odd, doing mouse movement via props on very simple components usually works okay). This seems to work like a charm for me.

export default function DragLayer() {
  const dragDropManager = useDragDropManager();
  const monitor = dragDropManager.getMonitor();
  const [item, setItem] = React.useState();

  const updateState = React.useCallback(() => setItem(monitor.getItem()), [monitor]);
  React.useEffect(() => monitor.subscribeToStateChange(updateState), [monitor, updateState]);

  const previewRef = React.useRef();
  const updateOffset = React.useCallback(() => {
    if (previewRef.current) {
      const offset = monitor.getClientOffset();
      if (offset) {
        previewRef.current.style.left = `${offset.x}px`;
        previewRef.current.style.top = `${offset.y}px`;
        previewRef.current.style.visibility = "";
      } else {
        previewRef.current.style.visibility = "hidden";
      }
    }
  }, [monitor]);
  React.useEffect(() => monitor.subscribeToOffsetChange(updateOffset), [monitor, updateOffset]);

  if (!item) return null;

  // implement render logic here
  if (item.type === "IMAGE") {
    return (
      <div className="position-fixed-parent">
        <img src={item.src} ref={previewRef}/>
      </div>
    );
  }

  return null;
}

@hejtmii
Copy link

hejtmii commented Mar 4, 2021

Hi guys. I stumbled upon the same problem but didn't like the hackish solutions with the direct access / event handling above.

The rule of thumb for efficient hooks to prevent re-renders and reconciliation is: Do not output more than necessary from a hook returning state

This is what the original solution everyone copies doesn't honor, creating a new object with very volatile properties each time, that is your problem (as many properly pointed out).

What I did was rather simple ... I output only the transform part as a string (not a new object). In addition to that, I simply round the transform coordinates in it to reduce the number of rerenders that are actually needed.

interface IDragPreviewProps {
  readonly renderPreview: () => React.ReactNode;
  readonly sourceId: string;
  readonly width?: number;
}

const DragPreview: React.FC<IDragPreviewProps> = ({
  renderPreview,
  sourceId,
  width,
}) => {
  const getPreviewTransform = useCallback(
    (monitor: DragLayerMonitor): string | null => {
      const item = monitor.getItem();
      const initialOffset = monitor.getInitialSourceClientOffset();
      const currentOffset = monitor.getSourceClientOffset();
      const isDragging = monitor.isDragging();

      if (!isDragging || (item.sourceId !== sourceId) || !initialOffset || !currentOffset) {
        return null;
      }

      const { x, y } = currentOffset;

      return `translate(${Math.round(x)}px, ${Math.round(y)}px)`;
    },
    [sourceId],
  );
  const transform = useDragLayer(getPreviewTransform);

  if (!transform) {
    return null;
  }

  return (
    <div
      className="drag-layer"
      style={width ? { width: `${width}px` } : undefined}
    >
      <div
        style={{
          transform,
          WebkitTransform: transform,
        }}
      >
        {renderPreview()}
      </div>
    </div>
  );
};
.drag-layer {
	position: fixed;
	top: 0;
	left: 0;
	z-index: 100;
	width: 100%;
	height: 100%;
	pointer-events: none;
	opacity: 0.7;
}

No hacks with direct element access, no additional events, works like a charm.

For further optimization, I could imagine further debouncing or rounding to more rough values accompanied

Note that I place render this preview component directly to the component being dragged, so I match the sourceId from my item and sync width of the preview based on the source width (using useResizeObserver)

@jamesopti
Copy link

jamesopti commented Mar 4, 2021

@kenticomartinh Thanks for posting this! Makes sense to me as a simple solution to optimizing performance here.

When I use useDragLayer, I find that when you release the mouse when not over a drop target, there is a delay before isDragging returns false.

Is this a related performance issue? Or am I missing something about how this hook works.

EDIT: The issue I'm experiencing can be seen in this demo codesandbox.

If this is a separate issue, I can file a separate ticket.

dragDropBug

@hejtmii
Copy link

hejtmii commented Mar 23, 2021

Hi @jamesopti , I can't reproduce it on your sandbox, it drops immediately. Are you still experiencing that issue?

@fromi
Copy link

fromi commented Mar 31, 2021

Hi @kenticomartinh, you solution is a good step forward, however it does not work like a charm for me.

I had 2 issues left:

  1. When "sourceId" is updated, the old value is still used inside the hook.
    => Fixed by saving the current sourceId inside a "useRef" hook.

  2. The refresh rate is still far to high for the UI to follow up if you have a complex screen. And we do not need the "transform" to be updated at very high frequency anyway!

I fixed that by using a custom wrapper around "useDragLayer" hook based on "requestAnimationFrame":

import {shallowEqual} from '@react-dnd/shallowequal'
import {useState} from 'react'
import {DragLayerMonitor, useDragLayer} from 'react-dnd'

export default function useEfficientDragLayer<CollectedProps>(collect: (monitor: DragLayerMonitor) => CollectedProps): CollectedProps {
  const collected = useDragLayer(collect)
  const [previousCollected, setPreviousCollected] = useState<CollectedProps>(collected)
  const [requestID, setRequestID] = useState<number>()
  if (requestID === undefined && !shallowEqual(collected, previousCollected)) {
    setPreviousCollected(collected)
    setRequestID(requestAnimationFrame(() => setRequestID(undefined)))
  }
  return previousCollected
}

This solution really works like a charm for me. You can see some example in the games we run at https://game-park.com/!

Maybe that "requestAnimationFrame" idea could become part of the framework?

@AliakseiYakubuk
Copy link

AliakseiYakubuk commented Apr 23, 2021

Hi guys!
Recently I've also ran into this issue and after a while I came up to the solution with direct access to ref
It is kind of a summary of what I read above, so I just moved logic to a hook that could be reused wherever dnd preview is necessary
And as an optimization of numerous DOM updates I'm using requestAnimationFrame

const useDragPreview = () => {
  const previewRef = useRef();
  const monitorRef = useRef();
  const [item, setItem] = useState(null);
  const dragDropManager = useDragDropManager();
  const updateItem = useCallback(() => setItem(monitorRef.current.getItem()), []);
  const updateOffset = useCallback(() => requestAnimationFrame(() => {
    if (previewRef.current) {
      const offset = monitorRef.current.getSourceClientOffset() || {};
      previewRef.current.style.transform = `translate(${offset.x}px, ${offset.y}px)`;
    }
  }), []);

  useEffect(() => {
    let unsubscribeToStateChange;
    let unsubscribeToOffsetChange;

    if (!monitorRef.current) {
      monitorRef.current = dragDropManager.getMonitor();
      unsubscribeToStateChange = monitorRef.current.subscribeToStateChange(updateItem);
      unsubscribeToOffsetChange = monitorRef.current.subscribeToOffsetChange(updateOffset);
    }

    return () => {
      if (typeof unsubscribeToStateChange === 'function') unsubscribeToStateChange();
      if (typeof unsubscribeToOffsetChange === 'function') unsubscribeToOffsetChange();
    };
  }, [dragDropManager, updateItem, updateOffset]);

  return [item, previewRef];
};

and then I apply it in the preview component

const DragPreview = () => {
  const [item, previewRef] = useDragPreview();
  const showPreview = item && item.type === DRAG_TYPE.GROUP;

  return (
    <div style={{position: 'fixed', pointerEvents: 'none', ...}}>
      {showPreview && (
        <Preview 
          ref={previewRef}
          {...item}
        />
      )}
    </div>
  );
};

It works nice and looks pretty easy - subscribe and set position in the callback
PS. I might be missing something, so please let me know if something does not look good

@hejtmii
Copy link

hejtmii commented Apr 23, 2021

@AliakseiYakubuk I actually ended up with something similar (directly modifying the style) working with preview components with larger DOM trees which were slow even on React reconciliation.

What you may consider:

  • I am using an extra isBusy flag as the updates may pile up still causing delays and a queue of future CPU allocation on slower machines (not sure if your solution has some throttling). The isBusy flag is throttling it without the overhead of setTimeout.
  • I render the preview to body to achieve proper z-index placement over everything else with z-index. I also observed that using position: absolute instead of fixed gives better frame time in Chrome. Don't ask me why. So I use that (I can, because of the placement in body).
const DragPreview: React.FC<IDragPreviewProps> = ({
  renderPreview,
  sourceId,
  sourceRef,
}) => {
  const previewRef = useRef<HTMLDivElement>(null);
  const lastTransform = useRef<string | null>(null);
  const isBusy = useRef(false);

  const getShouldRenderPreview = useCallback(
    (monitor: DragLayerMonitor): boolean => {
      const isDragging = monitor.isDragging();
      const item = monitor.getItem();
      if (!isDragging || (item.sourceId !== sourceId)) {
        lastTransform.current = null;
        isBusy.current = false;
        return false;
      }

      if (isBusy.current) {
        return !!lastTransform.current;
      }

      const initialOffset = monitor.getInitialSourceClientOffset();
      const currentOffset = monitor.getSourceClientOffset();

      if (!initialOffset || !currentOffset) {
        lastTransform.current = null;
        isBusy.current = false;
        return false;
      }

      const { x, y } = currentOffset;

      const newTransform = `translate(${Math.round(x)}px, ${Math.round(y)}px)`;
      if (newTransform !== lastTransform.current) {
        isBusy.current = true;
        lastTransform.current = newTransform;
        requestAnimationFrame(() => {
          if (previewRef.current) {
            previewRef.current.style.transform = newTransform;
            requestAnimationFrame(() => isBusy.current = false);
          }
        });
      }
      return true;
    },
    [sourceId],
  );

  const shouldRenderPreview = useDragLayer(getShouldRenderPreview);
  if (!shouldRenderPreview) {
    return null;
  }

  const width = sourceRef.current?.offsetWidth;

  return ReactDOM.createPortal(
    (
      <div
        className="drag-preview"
        style={{
          ...(width ? { width: `${width}px` } : undefined),
          ...(lastTransform.current ? { transform: lastTransform.current } : undefined),
        }}
        ref={previewRef}
      >
        {renderPreview()}
      </div>
    ),
    document.body,
  );
};

@hejtmii
Copy link

hejtmii commented Apr 23, 2021

@fromi Sorry for late response, I had GitHub notifications redirected to a side folder due to too much spam from our internal repos, but the rule was not great and even mentions ended up there :-(

Anyway, yes, requestAnimationFrame totally helps, I also used it. See my current version with some commentary above.

@fromi
Copy link

fromi commented Apr 23, 2021

@kenticomartinh Your version contains a nice idea with not collecting when it is busy.
I integrated it in my solution to get a very generic and reusable hook:

import {shallowEqual} from '@react-dnd/shallowequal'
import {useCallback, useRef} from 'react'
import {DragLayerMonitor, useDragLayer} from 'react-dnd'

export default function useEfficientDragLayer<CollectedProps>(collect: (monitor: DragLayerMonitor) => CollectedProps): CollectedProps {
  const requestID = useRef<number>()
  const collectCallback = useCallback(monitor => requestID.current === undefined ? {data: collect(monitor)} : undefined, [collect])
  const collected = useDragLayer(collectCallback)
  const result = useRef(collected?.data)
  if (collected && !shallowEqual(result.current, collected.data)) {
    result.current = collected.data
    requestID.current = requestAnimationFrame(() => requestID.current = undefined)
  }
  return result.current!
}

I really think this should be the default implementation for "useDragLayer" actually.

Edit: outside I use it this way:

  const offset = useEfficientDragLayer(monitor => monitor.getDifferenceFromInitialOffset())
  const translation = dragging && offset ? `translate3d(${Math.round(offset.x)}px, ${Math.round(offset.y)}px, 0)` : ''
  [...]

Edit2: I had to wrap collected data inside an object so I can distinguish between no result because there is an ongoing requestID, or because the hook was refreshed from an outside event, and a legit "undefined" collected value.

That's the most efficient implementation I found so far. It is nearly smooth on my laptop using Chrome and CPU throttling slowdown X4!

@slavagoreev
Copy link

Thank you @AliakseiYakubuk for this requestAnimationFrame idea. It solved those freezes when dragging a preview

@bipinrajbhar-rh
Copy link

Hi @fromi and @kenticomartinh , currently i am using "usePreview hook from "react-dnd-multi-backend" to create a custom drag layer but i am facing some performance issue.

@bulat92
Copy link

bulat92 commented Aug 18, 2022

if you came here because your preview element moves worse than mouse pointers then check the transition property in your CSS file. This can save you several hours of no search results.

@bulat92
Copy link

bulat92 commented Aug 18, 2022

Здарова если ты пришёл сюда из за того что твой превью элемент двигается хуже указатели мыши то проверь свойство transition в твоем CSS файле. Это может спасти тебе несколько часов без результатных поисков.

@bulat92
Copy link

bulat92 commented Aug 18, 2022

Здорова якщо ти прийшов сюди через те, що твій превью елемент рухається гірше покажчики миші то перевір властивість transition у твоєму CSS файлі. Це може врятувати тобі кілька годин без результатів.

@bulat92
Copy link

bulat92 commented Aug 18, 2022

Здарова калі ты прыйшоў сюды з-за таго што твой прэв'ю элемент рухаецца горш паказальнікі мышы то правер уласцівасць transition у тваім CSS файле. Гэта можа выратаваць табе некалькі гадзін без рэзультатных пошукаў.

@corskaya
Copy link

const DragPreview = () => {
  const [item, previewRef] = useDragPreview();
  const showPreview = item && item.type === DRAG_TYPE.GROUP;

  return (
    <div style={{position: 'fixed', pointerEvents: 'none', ...}}>
      {showPreview && (
        <Preview 
          ref={previewRef}
          {...item}
        />
      )}
    </div>
  );
};

Can you share all files? I couldn't understand what horizontal and vertical are.

@claycoleman
Copy link

claycoleman commented Sep 2, 2022

@kenticomartinh Thanks for posting this! Makes sense to me as a simple solution to optimizing performance here.

When I use useDragLayer, I find that when you release the mouse when not over a drop target, there is a delay before isDragging returns false.

Is this a related performance issue? Or am I missing something about how this hook works.

EDIT: The issue I'm experiencing can be seen in this demo codesandbox.

If this is a separate issue, I can file a separate ticket.

dragDropBug

@jamesopti did you ever find a solution to this? i'm seeing the same issue!

@hejtmii i can repro this on the examples page of the react-dnd docs, see the below video i just recorded there:

broken.mov

@AliakseiYakubuk
Copy link

Can you share all files? I couldn't understand what horizontal and vertical are.

Hi, @corskaya!
I am sorry, updateOffset should have an empty array of dependencies.
I forgot to remove those two parameters: vertical and horizontal, they were
used to enable/disable changing offset.x or offset.y

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

No branches or pull requests