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

Add pen pressure to annotations + various fixes #1657

Closed
wants to merge 0 commits into from

Conversation

frankrousseau
Copy link
Contributor

@frankrousseau frankrousseau commented Jan 12, 2025

Problem

  • The drawing tool doesn't support pen pressure
  • Sequence stats loading doesn't occur when shots are already loaded

Solution

  • Use PSBrush library to handle pen pressure and fake mouse pressure based on distance (or time) when using a mouse or a touch device. Solution implemented by @tetsuoanimation
  • Reload sequences if some data are missing

src/components/mixins/annotation.js Outdated Show resolved Hide resolved
src/components/mixins/player.js Outdated Show resolved Hide resolved
src/components/mixins/player.js Outdated Show resolved Hide resolved
src/components/mixins/player.js Outdated Show resolved Hide resolved
src/components/previews/PreviewPlayer.vue Outdated Show resolved Hide resolved
src/components/mixins/annotation.js Outdated Show resolved Hide resolved
Comment on lines 1175 to 1177
return Math.sqrt(
Math.abs(p1_rel.x - p2_rel.x) + Math.abs(p1_rel.y - p2_rel.y)
)
Copy link
Member

Choose a reason for hiding this comment

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

@tetsuoanimation I'm not sure to understand this one.
It doesn't look like a classic distance calculation (Euclidean or Manhattan distance formula).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a Pythagore formula. With two coordinates and their projections, you can make a triangle rectangle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But you're right, it seems that squares are missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will let it as it is because it works better than real distance and add some comments.

Copy link
Member

Choose a reason for hiding this comment

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

ok, notice that current usage of pow(..., 1) is useless.

Else, for the real distance you can use the Math.hypot() operator:
return Math.hypot(p1_rel.x - p2_rel.x, p1_rel.y - p2_rel.y)

and remove this weird line:
delta_dist *= 50 // magic number to scale to nicer values

Copy link
Contributor

Choose a reason for hiding this comment

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

But you're right, it seems that squares are missing.

I think in this case, I always ended up with squaring a 1 which does nothing - but I might be wrong?

I found a note that Math.hypot is very slow - since this is basically running every frame I preferred the faster sqrt way. It's totally ok if you'd prefer it.

The delta_dist scaling is weird indeed and I don't like using a magic number there. It just makes the resulting line and pressure values look prettier on screen and I couldn't find another way to make it look as nice.

Totally happy to adjust.

Copy link
Member

Choose a reason for hiding this comment

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

I found a note that Math.hypot is very slow - since this is basically running every frame I preferred the faster sqrt way. It's totally ok if you'd prefer it.

Your point about performance is relevant, see benchmark.

src/components/mixins/player.js Outdated Show resolved Hide resolved
src/components/mixins/player.js Outdated Show resolved Hide resolved
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.

3 participants