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

Fix Mouse position in canvas with padding #120

Merged
5 commits merged into from Jun 5, 2014
Merged

Fix Mouse position in canvas with padding #120

5 commits merged into from Jun 5, 2014

Conversation

mpkorstanje
Copy link
Contributor

The mouse position in an element is measured from the top left corner. When a margin or padding is used this starts outside the drawable area of the canvas. This needs to be taken into account.

The mouse position in an element is measured from the top left corner. When a margin or padding is used this starts outside the drawable area of the canvas. This needs to be taken into account.
@ghost ghost added the needs review label Jun 3, 2014
@ghost
Copy link

ghost commented Jun 3, 2014

Can you make a quick test run or an image of this issue? without the fix.
Thank you.

@ghost ghost added the under review label Jun 3, 2014
@mpkorstanje
Copy link
Contributor Author

Sure. I added some padding. Now try touching any of the sprites.

http://htmlpreview.github.io/?https://github.com/mpkorstanje/Quintus/blob/master/bugs/touch/index.html

Have to retract the comment about margin btw, its just the padding that messes things up.

@ghost
Copy link

ghost commented Jun 3, 2014

This only happens when the canvas element has padding... of course.
But it's very unlikely that someone would apply css padding to a canvas, even more the automatically generated from Quintus.
Is there any reason to apply padding to it?

@mpkorstanje
Copy link
Contributor Author

Aside from being the patch being the technically most correct way to do it, I've got one rather specific use case and some issues.

flashgamelicense.com has 'html5 opportunities'. They require all games to be either maximized and adjust the game area to the available space or use a fixed landscape/portrait that is fit-to-scale. The user can't be asked to rotate the phone.

In terms of quick development scale-to-fit is easy as you can work with a fixed canvas size. This makes it preferable over working with fullscreen.

fgl also provides a lib that handles the fit to scale aspect. This lib uses padding. It seems to be the one thing that works reliably across a good selection of mobile browsers. fgl will also update this lib to deal with undiscovered issues in the future and update games without intervention of the original dev.

This means that Quintus can't wrap the canvas. But that is a rather personal use case I can (f/w)ork around on my own as long as Quintus accepts input correctly.

I did however also encounter some issues with the wrapper. Some mobiles browsers start with a page-width that is larger then window.innerHeight. This is a usability trick to keep web pages with poorly defined min-widths from being rendered so narrow they're all bunched up.

The end result of that trick though is that while the canvas is neatly centered, there is also space around the canvas that can be scrolled in to. This allows the game to go out of view. Bad user experience altogether.

Just having the canvas element and adjusting the padding seems to preempt that however.

I'll see if I can get some screen of that.

@ghost
Copy link

ghost commented Jun 4, 2014

I tested one of the examples of the repo, the touch one. I applied a padding to the canvas, yes the detection was incorrect, then i replaced my quintus_input.js and quintus_touch.js with yours... yet the problem was still there, i took off the padding, and no problem. Like nothing happened, you might check your code again. otherwise i will have to close both issues.
Thanks.

@mpkorstanje
Copy link
Contributor Author

Okay, thats is odd.

Close it. I'd look into this deeper but if this isn't the fix, figuring out why it works on my end is a bit too much work at the moment. Made the in hindsight poor decision to work against the -all.js version rather then the github so its a big tangle of patches.

@ghost
Copy link

ghost commented Jun 4, 2014

Send me the files that you're using from your computer, i downloaded your files from your repo, i will replace them and see what happens (if the problem is still there).

@ghost ghost added the near closing label Jun 5, 2014
@mpkorstanje
Copy link
Contributor Author

@ghost
Copy link

ghost commented Jun 5, 2014

This works now, but only the dragging, the hover is still incorrect. I guess this is a step in the right direction.

@mpkorstanje
Copy link
Contributor Author

Mmmh.

The hover is done in the touch.js example. I turned it off in my clone.
Quintus itself doesn't have mousein and mouseout events. Its a work around
for that.

Speaking of duplicated code:

These together seem to perform the same function

quintus_input.js: touchLocation
quintus_input.js: Q._mouseMove
quintus_input.js: Q.canvasToStageX & Y
quintus_input.js:_containerOffset

as:

quintus_touch.js: normalizeTouch

Might be a good idea to consolidate the lot into a single correct working
version. Not on this merge request though.

On 5 June 2014 19:15, Alessandro Reinoso notifications@github.com wrote:

This works now, but only the dragging, the hover is still incorrect. I
guess this is a step in the right direction.


Reply to this email directly or view it on GitHub
#120 (comment).

@ghost
Copy link

ghost commented Jun 5, 2014

Couldn't we just put the same logic of this fix into the function that handles the detection on mouseover?

@mpkorstanje
Copy link
Contributor Author

I just did. :)

@ghost
Copy link

ghost commented Jun 5, 2014

Make the update on this PR so i can test it

@mpkorstanje
Copy link
Contributor Author

Ah didn't know I could do that. Cheers.

@ghost
Copy link

ghost commented Jun 5, 2014

ooh sorry I though there was some kind of mouseover function inside quintus but is all handled in the example, my bad. This opens the need to enable mouse over inside quintus, so game devs avoid to write their own.
Another thing is that the touch example is only affected when i add/remove the:

- parseInt(style.paddingLeft)
- parseInt(style.paddingTop)

but only in quintus_touch.js, not quintus_input.js guess because the events are called using touch interface, but i am not emulating a touch device... so i'm guessing that there's some redundancy in quintus_input.js? like you said some functions do the same as normalizeTouch().

@ghost ghost changed the title Fix Mouse position in canvas with margins or padding Fix Mouse position in canvas with padding Jun 5, 2014
@ghost
Copy link

ghost commented Jun 5, 2014

Can you revert the changes in the touch example? i just need to merge the quintus_input.js, because the original example does not have padding applied.

@mpkorstanje
Copy link
Contributor Author

Done.

@mpkorstanje
Copy link
Contributor Author

but only in quintus_touch.js, not quintus_input.js guess because the events are called using touch interface, but i am not emulating a touch device... so i'm guessing that there's some redundancy in quintus_input.js? like you said some functions do the same as normalizeTouch().

_touch.js is listening to mouse clicks and touches and passing these onto sprites as touch, drag and touchEnd events. For the example _input.js doesn’t really come into play.

ghost pushed a commit that referenced this pull request Jun 5, 2014
Fix Mouse position in canvas with padding
@ghost ghost merged commit 9b1aed7 into cykod:master Jun 5, 2014
@ghost ghost removed near closing labels Jun 5, 2014
@mpkorstanje mpkorstanje deleted the patch-3 branch June 5, 2014 21:04
This pull request was closed.
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.

None yet

1 participant