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

Ruler refactor #4171

Merged
merged 28 commits into from
Jul 29, 2021
Merged

Ruler refactor #4171

merged 28 commits into from
Jul 29, 2021

Conversation

JacksonRG
Copy link
Collaborator

@JacksonRG JacksonRG commented Jun 2, 2021

Super Zoom!

I've changed the ruler code so that you can zoom as much as you want with +/- keys. This does some math so that the number of pixels between ruler ticks is always a factor of the FPS (rounded). That way, as I start drawing the tick marks on a the first frame of a second, every following second on the timeline should be marked.

The ruler math

I did the factoring by adding to functions.js:

  • I added code to find prime numbers
  • A set to store the primes in, to avoid repeated work
  • Starting at 1 frame per ruler tick, I multiply by prime factors of FPS until the tick marks are at least 40px apart.

Issues:

  1. On Webkit, the timeline's shift-scroll (horizontal scroll) doesn't work.
  2. On Webengine, the ctrl-scroll (zoom) doesn't work.

(These two issues are present in dev)

@JacksonRG
Copy link
Collaborator Author

JacksonRG commented Jun 2, 2021

Shoot! Before anyone says it, I need to get it to draw times on first load.
That bug is only a problem in browser. Its working in app.

Also, should I be worried about the build error?

@jonoomph
Copy link
Member

@JacksonRG This PR seems to include all the other changes made in develop. It's a bit crazy looking now. @ferdnyc Is this normal, and will this merge correctly? Just double checking...

@JacksonRG
Copy link
Collaborator Author

@JacksonRG This PR seems to include all the other changes made in develop. It's a bit crazy looking now. @ferdnyc Is this normal, and will this merge correctly? Just double checking...

Sorry, Totally missed your comment. Anyway, I just rebased onto develop (I think without breaking anything).

Also since we were talking about openshot just letting users keep zooming in, I removed the scroll limmits. (at least if you scroll with - and =.

@jonoomph
Copy link
Member

Looking much better!

@jonoomph
Copy link
Member

@JacksonRG I just resolved the conflict on src/timeline/js/functions.js... but please double check everything still works.

@JacksonRG JacksonRG merged commit 6ba194b into develop Jul 29, 2021
@jonoomph jonoomph deleted the ruler-refactor branch August 6, 2021 03:12
@JacksonRG JacksonRG mentioned this pull request Aug 12, 2021
6 tasks
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.

Timeline should be able to zoom in MUCH further than 1 second increments
2 participants