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

Calculating mouse position based on canvas bounding rectangle for emb… #2942

Merged
merged 6 commits into from
Aug 8, 2017

Conversation

calrk
Copy link
Contributor

@calrk calrk commented Aug 8, 2017

…edded scenes

Description:
For this issue:
#2938
Not sure about the performance impacts, and whether this code should only execute for if scenes have the 'embedded' attribute

Changes proposed:

  • Using sceneEl.canvas.getBoundingClientRect() to properly calculate the mouse position relative to the canvas

@ngokevin
Copy link
Member

ngokevin commented Aug 8, 2017

This will be a bit more involved, but I think we want to cache the bounds (e.g., in this.canvasBounds, and then on window resize event in a throttled handler (utils.throttle), we want recalculate those bounds.

@calrk
Copy link
Contributor Author

calrk commented Aug 8, 2017

Is this what you meant by using the throttledTick and window resize event together?

@ngokevin
Copy link
Member

ngokevin commented Aug 8, 2017

Close, I meant just utils.throttle the handler, not throttleTick. In case someone is dragging to resize the window, many resize events would fire.

@calrk
Copy link
Contributor Author

calrk commented Aug 8, 2017

I think this is what you mean, copying the style of 'throttledEmitComponentChanged' in component.js.
I made an example which I have also uploaded here to test it out by mousing over the central cube. The problem with using the throttle wrapper is that if the user does a lot of small quick drags, it won't update to the final result. Doing a single resize works fine though.

The Library drag component library that was being used has the same issue.
https://github.com/jesstelford/aframe-click-drag-component/blob/master/src/index.js#L132

@ngokevin
Copy link
Member

ngokevin commented Aug 8, 2017

OK, I see. We want to use debounce instead. We need to add a utility function for that. In utils/index.js next to throttle, we can add:

module.exports.debounce = function (func, wait, immediate) {
	var timeout;
	return function() {
		var context = this, args = arguments;
		var later = function() {
			timeout = null;
			if (!immediate) func.apply(context, args);
		};
		var callNow = immediate && !timeout;
		clearTimeout(timeout);
		timeout = setTimeout(later, wait);
		if (callNow) func.apply(context, args);
	};
};

And use utils.debounce instead. Thanks!

@calrk
Copy link
Contributor Author

calrk commented Aug 8, 2017

Ok I've added the debounce function in and it works better now.

@ngokevin ngokevin merged commit 4c92620 into aframevr:master Aug 8, 2017
@ngokevin
Copy link
Member

ngokevin commented Aug 8, 2017

Thanks!

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.

2 participants