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: fromScreen not using viewport scale, mousePos() not working with letterbox/stretch #436

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lajbel
Copy link
Collaborator

@lajbel lajbel commented Oct 6, 2024

Description

Issues or related

Copy link

pkg-pr-new bot commented Oct 6, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/kaplayjs/kaplay@436

commit: 7381226

Copy link
Member

@mflerackers mflerackers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong. World and local have nothing to do with viewport scale.
Viewport scale only makes sense between screen and world.

@mflerackers
Copy link
Member

Additionally, you don't want to divide each time, it is better to store 1 / factor and multiply instead.

@lajbel lajbel marked this pull request as draft October 6, 2024 14:53
@lajbel lajbel changed the title fix: fromScreen not using viewport scale fix: fromScreen not using viewport scale, mousePos() not working with letterbox/stretch Oct 6, 2024
@dragoncoder047
Copy link
Contributor

So scale is fine now, it's stretch that is bugged. If letterbox: true then it works since the canvas's aspect ratio is as specified. stretch makes it not be as specified, so it gets off.

kaplay({
    scale: 3,
    width: 20,
    height: 40,
    stretch: true
});

const x = add([
    anchor("center"),
    circle(3),
    color(RED),
    pos(),
]);

getTreeRoot().use(pos());

onMouseMove(pos => {
    x.pos = getTreeRoot().fromScreen(pos);
});

@mflerackers
Copy link
Member

Yeah, when stretch is used, you no longer have a uniform scale. You need a vec2 in that case.

@lajbel lajbel marked this pull request as ready for review October 13, 2024 14:09
@lajbel
Copy link
Collaborator Author

lajbel commented Oct 13, 2024

So scale is fine now, it's stretch that is bugged. If letterbox: true then it works since the canvas's aspect ratio is as specified. stretch makes it not be as specified, so it gets off.

kaplay({
    scale: 3,
    width: 20,
    height: 40,
    stretch: true
});

const x = add([
    anchor("center"),
    circle(3),
    color(RED),
    pos(),
]);

getTreeRoot().use(pos());

onMouseMove(pos => {
    x.pos = getTreeRoot().fromScreen(pos);
});

The question is if there's an user case using only Stretch without letterbox

@niceEli
Copy link
Member

niceEli commented Oct 30, 2024

resolved conflicts in commit 894d9df

@niceEli
Copy link
Member

niceEli commented Oct 30, 2024

no idea for fail
should of worked properly

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.

bug: fromScreen() and friends ignore global scale
4 participants