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

circular rhythm ruler #1290

Open
walterbender opened this issue Aug 16, 2018 · 38 comments
Open

circular rhythm ruler #1290

walterbender opened this issue Aug 16, 2018 · 38 comments
Labels
Component-Widget relating to a widget GSoC24 Issues for GSoC students to work on. Issue-Enhancement Priority-Minor Nice to have
Milestone

Comments

@walterbender
Copy link
Member

Since the rhythm wraps, it would be nice to have a circular display.
screenshot from 2018-08-15 20-38-27

@a-ritwik
Copy link
Contributor

a-ritwik commented Aug 19, 2018

Nice design. I have a question. How would we show two rulers with different meters, and where they start to align with each other? @walterbender @pikurasa

@walterbender walterbender added this to the future milestone Nov 10, 2019
@walterbender
Copy link
Member Author

They'd be concentric.

@walterbender walterbender modified the milestones: future, GCI 2019 Nov 11, 2019
@walterbender walterbender modified the milestones: GCI 2019, future Feb 10, 2020
@ricknjacky
Copy link
Member

@walterbender the issue is redundant now, ig.

@walterbender
Copy link
Member Author

It is still waiting an implementation.

@walterbender walterbender added the Priority-Minor Nice to have label Nov 28, 2020
@pikurasa pikurasa added the GSoC24 Issues for GSoC students to work on. label Jun 3, 2023
@belginkoc
Copy link
Contributor

Hello,
I am looking to contribute to SugarLabs and while looking over the issues I came across this one. At first glance, it seems like this enhancement issue has already been implemented on music blocks. Should this issue be closed?

@walterbender
Copy link
Member Author

I did a sketch at one point, but AFAIK, it has not been implemented.

@ChiragJS
Copy link
Contributor

this has to be implemented for rhythm maker ,right?

@walterbender
Copy link
Member Author

Yes. It would be a replacement of the current rhythm maker UX.

@ChiragJS
Copy link
Contributor

ChiragJS commented Jan 14, 2024

Yes. It would be a replacement of the current rhythm maker UX.

any thing specific to be looked out for while working on this issue? any specific function might help

@walterbender
Copy link
Member Author

I had done a sketch using the piemenu codebase (https://github.com/sugarlabs/musicblocks/blob/master/js/piemenus.js). It can try to find it.

@ChiragJS
Copy link
Contributor

ChiragJS commented Jan 14, 2024

I had done a sketch using the piemenu codebase (https://github.com/sugarlabs/musicblocks/blob/master/js/piemenus.js). It can try to find it.

Sorry, didn't get you here, do you mean like referring to piemenu codebase will help with implementing the design you made?

@walterbender
Copy link
Member Author

I mean that I used the same wheel widget library as is used in the pie menus.

@ChiragJS
Copy link
Contributor

A few questions regarding this issue:
So on clicking on the pie menu ruler, it will get divided by itself? So hence, we wont need the current rhythm maker UX.
Secondly, what would happen in this case?
Screenshot from 2024-01-20 22-34-01
All the rows will be concentric circles?
and if you look at the ruler , row 1 , it has 3 (1/1)s , (for this I copied the rhythm block and placed it under the same intrument block), so this has to be shown in the same circle?
And let's say there are n number of rows in the ruler, so would there be n number of concentric circles, what if the number is huge , wouldn't the circle get too big?

@walterbender
Copy link
Member Author

Yes, we'd use concentric circles. There are only so many we can have at the same time, eventually they will run off the screen.

Clicking will divide. We need a way to join. And a way to specify a rest (currently a long press).

@ChiragJS
Copy link
Contributor

ChiragJS commented Jan 20, 2024

A way to join ? Sorry , didn't get you here? you mean undoing the divide?
And what about the 1st row here, there are 3 (1/1)s how is it supposed to be rendered? I didn't get it

@walterbender
Copy link
Member Author

Not quite undo, since you could do the following:

1 --> 1/2, 1/2 --> 1/4, 1/4, 1/2 --> 1/4, 1/4, 1/4, 1/4 --> 1/4, 1/2, 1/4

The final operation was a join of the middle two quarter notes.

@ChiragJS
Copy link
Contributor

Oh got it !
And sorry to ask again , what about the 1st row, i sent in the pic earlier, how should that be rendered? 3 divisions in the circle each saying 1/1?

@walterbender
Copy link
Member Author

The total note value of notes in the ruler is determined by the argument passed to the widget block. By default you get 1, but you can override that and get 2, 3, 1.5, whatever you want. So the concentric circles are all the same length in terms of degrees (360), if the note values don't match, the smaller ones will be segments of circles. I could imagine something where when we play the collection of rhythms, the segments rotate to stay aligned with the full circle, but that is not necessary to do in the first pass.

@apsinghdev
Copy link
Member

apsinghdev commented Jan 30, 2024

@walterbender sir, should there be any limit on how many rhythms we can add to it? I experimented with more rhythms and can see only 8 on the screen.

Screenshot 2024-01-30 165917

( also it seems there is no scroll bar to see the rest of them ). would it be good to have a limit of rhythms we can add in case of adding circular rulers?

@walterbender
Copy link
Member Author

I think 6-8 is a reasonable limit.

@apsinghdev
Copy link
Member

I think 6-8 is a reasonable limit.

got it. thanks.

@walterbender
Copy link
Member Author

Maybe even 6 is enough.

@ChiragJS
Copy link
Contributor

Hey @apsinghdev are you working on the issue now? Because I've been working on it, been a bit busy but will be working on it now actively

@apsinghdev
Copy link
Member

yes @ChiragJS I am working on this.

@ChiragJS
Copy link
Contributor

ChiragJS commented Feb 1, 2024

Would you mind working on something else because I'm already halfway through it?

@apsinghdev
Copy link
Member

Would you mind working on something else because I'm already halfway through it?

same here brother. invested almost 3 weeks of time and effort.

@pikurasa
Copy link
Collaborator

pikurasa commented Feb 3, 2024

@ChiragJS and @apsinghdev it's not necessarily a waste to have multiple people working on the same issue.

Perhaps you two could trade notes and work collaboratively towards a solution?

@ChiragJS
Copy link
Contributor

ChiragJS commented Feb 3, 2024

I'm open to work collaboratively , how about you @apsinghdev ?

@apsinghdev
Copy link
Member

I'm open to work collaboratively , how about you @apsinghdev ?

Yeah sure @ChiragJS. On which part you are working?

@ChiragJS
Copy link
Contributor

ChiragJS commented Feb 4, 2024

Let's talk about it on Matrix then? I'll dm you? @apsinghdev

@apsinghdev
Copy link
Member

Let's talk about it on Matrix then? I'll dm you? @apsinghdev

Yeah sure.

@apsinghdev
Copy link
Member

apsinghdev commented Mar 31, 2024

@walterbender I have added splitting logic by the following steps :

  • listen to the click event on the wheel
  • store the index of the clicked wheel
  • store the index in an array named indexes
  • store up to 6 indexes in the array
  • store that array in local storage
  • re-render the page ( a simple refresh or hard reload both works )
  • on re-render, retrieve the array
  • when the iteration of creation to the circular table takes place, check if the index of the current ruler present in that array
  • if the index is present, split it by the dissectNumber value.

it looks like this

2024-03-31.13-56-38.mp4

(I'd like you to please ignore the text inside the notes for now since I have not added the logic for ratio.)

I wanna ask is

  1. is this the optimal approach to add split logic?

  2. should I add multi-dimensional splitting? (1/2 → 1/4, 1/4) if yes, to what extent?

  3. currently, I store up to 6 indexes to split in one iteration, and when that array has more than 6 elements it clears out the array and again starts from 1. Is this flow good enough or there is a need for enhancement?

  4. In what condition, should I clear that array to 0?

  5. I think it is not good to refresh each time the user splits a note. so should we store a specific number of clicks and then direct the user to refresh? or let'em just refresh each time?

and lastly, do you have any bare minimum idea that must be implemented regarding the splitting of notes?

thanks

@apsinghdev
Copy link
Member

here is how it looks when dissectNumber value changes

dissect

(again, please ignore the text value inside the notes for now)

@ChiragJS
Copy link
Contributor

I think hard reloading the page everytime you make changes to it would not be the most practical solution we should be working towards ,as it would ruin the user experience .

@walterbender
Copy link
Member Author

  • is this the optimal approach to add split logic?

I had a hard time from your video to see how the splitting worked. It was only clear after the refresh that things were split.

  • should I add multi-dimensional splitting? (1/2 → 1/4, 1/4) if yes, to what extent?

Yes. That is the whole point of the rulers.

  • currently, I store up to 6 indexes to split in one iteration, and when that array has more than 6 elements it clears out the array and again starts from 1. Is this flow good enough or there is a need for enhancement?

I am not sure what you are referring to here. 6 indexes to what? What is an iteration? Are you talking about caching user input? If so, that is not a good desigh.

  • In what condition, should I clear that array to 0?

There is an erase button (and an undo button).

  • I think it is not good to refresh each time the user splits a note. so should we store a specific number of clicks and then direct the user to refresh? or let'em just refresh each time?

I disagree. If it is slow, the user will accommodate. But there is no reason it should be that slow. If the problem is the number of rulers (max 6 in your implementation) then lets use fewer rulers. But I think it needs to be at least two.

  • and lastly, do you have any bare minimum idea that must be implemented regarding the splitting of notes?

The straight rulers implement a minimum -- I don't recall what it is. Many one sixtyfourth? Even one thirtysecond would be OK

@apsinghdev
Copy link
Member

apsinghdev commented Apr 2, 2024

@walterbender

Thanks for your feedback, in previous discussions, we discussed that we wouldn't be able to get live splitting of rulers as wheelnavjs library doesn't provide such functionality. so we concluded that we could add the splitting logic by re-rendering the page. that's what I've tried to show in the video. for more clarity, the flow would be like :

  1. user clicks on the rulers
  2. a message gets prompted to the user like "please refresh to see your splits" ( I've not implemented it yet )
  3. they refresh
  4. the rulers clicked by user gets split according to the dissectNumber.

Please feel free to inquire if further clarification is needed. Thank you.

@walterbender
Copy link
Member Author

I don't recall coming to that conclusion. We refresh the pie menus in many different situations. For example in the Mode menu. Why do you think it cannot be done?

@apsinghdev
Copy link
Member

I don't recall coming to that conclusion. We refresh the pie menus in many different situations. For example in the Mode menu. Why do you think it cannot be done?

Oh, my bad. I think I misunderstood the issue. You are right; we have other refresh situations besides just refreshing for splitting. One such situation is switching back to straight rulers and then switching again to circular ones, as it rerenders the rulers when we switch to circular rulers. does this make sense?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component-Widget relating to a widget GSoC24 Issues for GSoC students to work on. Issue-Enhancement Priority-Minor Nice to have
Projects
None yet
Development

No branches or pull requests

7 participants