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

Make auto-panning speed uniform #1690

Merged

Conversation

elbertronnie
Copy link
Collaborator

@elbertronnie elbertronnie commented Mar 15, 2024

Closes #1527

Currently the speed of auto-panning depends on hardware performance or the number of shapes in the document.

This PR attempts to resolve it by making timestamp and time-delta information available inside the InputPreprocessorMessageHandler and updating it every frame.

@Keavon
Copy link
Member

Keavon commented Mar 15, 2024

Shouldn't timestamp be called frame_delta_time instead?

@elbertronnie
Copy link
Collaborator Author

Shouldn't timestamp be called frame_delta_time instead?

No, because it is truely the timestamp and not a time-delta. I thought that providing timestamp info would be better since one can easily derive time-delta from timestamp but the other way round is much harder.

And its type is Duration since that is what is available in core. Instant would not make sense since there is no way to create a Instant from the timestamp given by requestAnimationFrame.

@Keavon
Copy link
Member

Keavon commented Mar 15, 2024

Ah, I just checked the requestAnimationFrame docs and I see now that it says it's a high DOMHighResTimeStamp. I had recalled that it gives a delta time which is why I thought that was what this was. Disregard my confusion, thanks for explaining.

@Keavon
Copy link
Member

Keavon commented Mar 15, 2024

!build

Copy link

📦 Build Complete for 79c5693
https://7f6bcd07.graphite.pages.dev

@elbertronnie
Copy link
Collaborator Author

Or I just had an Idea: We can make a custom struct TimeInfo which will be kept inside InputPreprocessorMessageHandler. It will keep track of previous and current timestamp so both kinds of information(timestamp and time-delta) will be available to every tool.
We could probably also put frame count if required.

@Keavon
Copy link
Member

Keavon commented Mar 15, 2024

If you think that's cleaner, go ahead. Or if the current approach is simpler, feel free to keep it.

We could probably also put frame count if required.

At some point I'd like to have an option to enable a framerate counter debug overlay. Although I don't know if this is a good way to implement that. It's also not that high priority since the browser dev tools can be used to show a framerate too.

@elbertronnie
Copy link
Collaborator Author

elbertronnie commented Mar 15, 2024

It will be cleaner, since in the future if some other feature requires time-delta, it can reuse it from input instead of keeping track of previous timestamp.

Although I am thinking to skip frame count for now. It can be implemented when its need arises.

@Keavon
Copy link
Member

Keavon commented Mar 15, 2024

Sounds good, I'd say go ahead with what's needed right now and it can always be extended later as you point out.

@elbertronnie elbertronnie marked this pull request as draft March 15, 2024 22:54
editor/src/consts.rs Outdated Show resolved Hide resolved
@elbertronnie elbertronnie marked this pull request as ready for review March 16, 2024 05:42
@elbertronnie elbertronnie force-pushed the make-auto-panning-speed-uniform branch 2 times, most recently from 399a875 to f1a3033 Compare March 16, 2024 05:50
@Keavon Keavon force-pushed the make-auto-panning-speed-uniform branch from f1a3033 to 53cc0dc Compare March 16, 2024 06:02
@Keavon Keavon enabled auto-merge (squash) March 16, 2024 06:32
@Keavon Keavon merged commit 42c8220 into GraphiteEditor:master Mar 16, 2024
2 checks passed
@elbertronnie elbertronnie deleted the make-auto-panning-speed-uniform branch April 14, 2024 14:24
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.

Autoscroll when mouse is outside viewport and dragging
2 participants