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
Comments
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 // 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: Of course the default @gaearon Is such a specialized but way more performant implementation interesting for you to be integrated as alternative into react-dnd? |
@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 |
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. |
What exactly do you mean by laggy:
|
I'd say both |
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. |
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. |
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? |
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 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. |
I'm not sure I follow! I only have the one custom drag layer in my app, I believe! |
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. |
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? |
It seems like the react-dnd-html5-backend has terrible performance with a custom DragLayer whereas the react-dnd-touch-backend has OK performance. |
Kinda obvious, but it helps: let updates = 0;
@DragLayer(monitor => {
if (updates++ % 2 === 0) {
return {
currentOffset: monitor.getSourceClientOffset()
}
}
else {
return {}
}
}) |
Another workaround is to use the |
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) |
@dobesv change to react-dnd-touch-backend solved the problem. I also tried react-throttle-render, but still seems not good. |
Yeah, I know the touch backend works, but it doesn't support drag and drop file uploads, which I want to use. |
@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. |
Kind of a no brainer, but I used the performant drag layer fix and spent a
LOT of time looking at my code to limit the number of renders that were
being called by the app (lots of switching Component -> PureComponent) and
in the end I didn't even need to use the react-throttle-render.
I DEFINITELY recommend looking into using PureComponent and trying to
maximize your code instead of looking for an easy fix on this thread if
your render is slow! I'll post more if we improve the Performant model at
all though!
… |
@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. |
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>
);
}
}
); |
@stellarhoof Thank you for the great answer! Unfortunately the solution does not work on IE11 for me. 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>
);
}
); |
@stellarhoof I notice |
@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 |
This worked for me! The others solutions seems to work only for older versions. |
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 |
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 |
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.
|
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
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 |
@kenticomartinh Thanks for posting this! Makes sense to me as a simple solution to optimizing performance here. When I use 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. |
Hi @jamesopti , I can't reproduce it on your sandbox, it drops immediately. Are you still experiencing that issue? |
Hi @kenticomartinh, you solution is a good step forward, however it does not work like a charm for me. I had 2 issues left:
I fixed that by using a custom wrapper around "useDragLayer" hook based on "requestAnimationFrame":
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? |
Hi guys! 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 |
@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:
|
@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, |
@kenticomartinh Your version contains a nice idea with not collecting when it is busy.
I really think this should be the default implementation for "useDragLayer" actually. Edit: outside I use it this way:
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! |
Thank you @AliakseiYakubuk for this requestAnimationFrame idea. It solved those freezes when dragging a preview |
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. |
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. |
Здарова если ты пришёл сюда из за того что твой превью элемент двигается хуже указатели мыши то проверь свойство transition в твоем CSS файле. Это может спасти тебе несколько часов без результатных поисков. |
Здорова якщо ти прийшов сюди через те, що твій превью елемент рухається гірше покажчики миші то перевір властивість transition у твоєму CSS файлі. Це може врятувати тобі кілька годин без результатів. |
Здарова калі ты прыйшоў сюды з-за таго што твой прэв'ю элемент рухаецца горш паказальнікі мышы то правер уласцівасць transition у тваім CSS файле. Гэта можа выратаваць табе некалькі гадзін без рэзультатных пошукаў. |
Can you share all files? I couldn't understand what horizontal and vertical are. |
@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 broken.mov |
Hi, @corskaya! |
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
The text was updated successfully, but these errors were encountered: