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

Temperament widget #1220

Merged
merged 103 commits into from Sep 29, 2018

Conversation

Projects
None yet
3 participants
@riyalohia
Copy link
Contributor

commented May 24, 2018

This PR is with reference to Temperament Widget. Temperament Widget has been developed as a part of GSoC'18 program.

@walterbender

This comment has been minimized.

Copy link
Member

commented May 24, 2018

All I see when I run this is the widget toolbar with 3 buttons (play, save, exit). Is there supposed to be more in the widget yet?

@riyalohia

This comment has been minimized.

Copy link
Contributor Author

commented May 24, 2018

@walterbender , Yes. I will add more buttons.

@walterbender

This comment has been minimized.

Copy link
Member

commented May 24, 2018

I know you will be adding more, but my question was, am I not seeing something I should be seeing?
screenshot from 2018-05-24 07-41-45

@riyalohia

This comment has been minimized.

Copy link
Contributor Author

commented May 24, 2018

@walterbender , you are not missing anything. As of now, I have added these 4 buttons only.

@walterbender

This comment has been minimized.

Copy link
Member

commented May 24, 2018

Looks good so far :)

@riyalohia riyalohia force-pushed the riyalohia:Temperament_Widget branch from 04bea82 to 7406cde May 25, 2018

@walterbender

This comment has been minimized.

Copy link
Member

commented May 28, 2018

@riyalohia I have been experimenting with pie menus (See #1204). I'm using wheelNav.js (See http://wheelnavjs.softwaretailoring.net/test/). Since I am already adding that lib to the project (and because I think it would be an easy way to add the pitch circle to your widget), I recommend you check it out.

@riyalohia riyalohia force-pushed the riyalohia:Temperament_Widget branch 2 times, most recently from b9ffd43 to dd48bbb May 31, 2018

@riyalohia

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2018

Currently, temperament systems with pitch number greater than 12 gives incorrect note information. This is because we need to change pitchToNumber and numberToPitch function in musicutils according to the selected temperament.

To do list (for circle of notes hover) :

  • Add frequency of notes.
  • Create an edit button so that user can edit frequency.

@riyalohia riyalohia force-pushed the riyalohia:Temperament_Widget branch 4 times, most recently from 587a9ce to 2d099d7 Jun 6, 2018

@riyalohia riyalohia force-pushed the riyalohia:Temperament_Widget branch from 9a6e1f2 to 0b66cfe Jun 19, 2018

@riyalohia riyalohia force-pushed the riyalohia:Temperament_Widget branch from b3929b8 to c8b0a29 Jun 26, 2018

@walterbender

This comment has been minimized.

Copy link
Member

commented Jun 27, 2018

This was what I had in mind: click on the outer circle to indicate where you want to insert in the inner circle... but in looking at it, I am not so sure now.

Maybe with some better use of color and different labels (an inward pointing arrow) it would work better.

screenshot from 2018-06-27 01-26-18

Here is the code I used:

wheel = new wheelnav('wheelDiv3', null, 600, 600);
wheel.slicePathFunction = slicePath().DonutSlice;
wheel.slicePathCustom = slicePath().DonutSliceCustomization();
wheel.slicePathCustom.minRadiusPercent = 0.9;
wheel.slicePathCustom.maxRadiusPercent = 1.0;
wheel.sliceSelectedPathCustom = wheel.slicePathCustom;
wheel.sliceInitPathCustom = wheel.slicePathCustom;
wheel.animatetime = 300;
		var minutes = [];
		for (i=0; i < 60; i++) {
                    minutes.push((i%10).toString());
		}
		wheel.createWheel(minutes);

wheel = new wheelnav('wheelDiv2', wheel.raphael);
wheel.wheelRadius = 200;
wheel.maxPercent = 1.6;
wheel.slicePathFunction = slicePath().MenuSlice;
wheel.initWheel(["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11"]);wheel.navItems[0].slicePathFunction = slicePath().MenuSliceWithoutLine;
wheel.createWheel();
@walterbender

This comment has been minimized.

Copy link
Member

commented Jun 27, 2018

		wheel.colors = ['#c0c0c0', '#e0e0e0'];
		wheel.titleRotateAngle = 90;
wheel.animatetime = 300;
		var minutes = [];
		for (i=0; i < 60; i++) {
                    minutes.push('|');
		}

screenshot from 2018-06-27 07-00-32

Maybe a bit closer to what we want?

@riyalohia

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2018

@walterbender, According to your design, there are 4 points between two circles (pitches), but I guess there can be any number of points.

@walterbender

This comment has been minimized.

Copy link
Member

commented Jun 27, 2018

Yes. I just chose 60 as a placeholder. Really you probably only need 1 "tick" between each and once you create a new note in between, you can fine-tune its position. (and then create a new set of "ticks"). Make sense?

@riyalohia

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2018

@walterbender , yes. This will only be applicable for arbitrary edit?

@walterbender

This comment has been minimized.

Copy link
Member

commented Jun 27, 2018

I think so... let me think it through. It's 4 am here, so I am going to need to sleep on it :)

@pikurasa

This comment has been minimized.

Copy link
Collaborator

commented Jun 27, 2018

These extra lines help one orient themselves as they choose their note?

@riyalohia

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2018

If a user chooses arbitrary as an edit option, circle of notes appear in the format shown above (inner and outer circle). Once a user selects a tick between two notes (For eg. selected tick is between 0 and 1), a slider appears. (In this case, range of slider would be between frequencies of 0 and 1).
@walterbender , @pikurasa Is this idea appropriate for arbitrary edit tool?

@riyalohia riyalohia force-pushed the riyalohia:Temperament_Widget branch 2 times, most recently from 8eb6cc4 to b87a4d0 Jul 2, 2018

@walterbender

This comment has been minimized.

Copy link
Member

commented Jul 14, 2018

This looks correct. Please test using the semi-tone transpose to ensure you are seeing what you expect.
We also need to test the scalar transpositions. @pikurasa do you have some test cases we can try?

@pikurasa

This comment has been minimized.

Copy link
Collaborator

commented Jul 15, 2018

I did test the latest changes yesterday and today, but I am a little confused what I am actually able to test at this point.

This is what I tried:
I noticed that there is a new export feature from the widget for custom, and it works kind of how we agreed when we started this project. What I am expecting to see that I do not see yet is different names for the pitches. by default, all the pitches are named "C", which is problematic as we cannot refer to all the various pitches by the same name and expect different results. I have yet to try specifying names for each pitch myself, but will try in a little bit. That being said we need to find a workaround to this problem.

All the pitches are named "C" (not just the calculation that creates the pitch, which is fine, but the name for the pitch itself)
screenshot at 2018-07-14 20 12 08 naming

I tried to workaround this problem by using numbers, but this functionality does not seem to have been implemented. Please implement this functionality as it would be the "least common denominator" for testing and communication purposes. The tonic is always "0"; the pitch before tonic is "-1"; the pitch after tonic is "1"; the octave repeats for however total pitches there are (e.g. for 5 pitches, 5 would be next octave above and -5 would be the next octave below). Having this functionality could be a first phase for testing purposes.

This is my current test file:
13-equal-test.txt

If I am testing this all wrong, of course, please let me know.

@pikurasa

This comment has been minimized.

Copy link
Collaborator

commented Jul 15, 2018

...also I expect to be able to name the custom temperament.

The default block, however, is the pie menu for temperament. Instead, it should be a text block (similar to how action and box work) so that the user can choose a name. Then, the new custom name should appear in the pie menu.

@riyalohia

This comment has been minimized.

Copy link
Contributor Author

commented Jul 15, 2018

@walterbender , I have tested using the semi-tone transpose and scalar transpose blocks.
@pikurasa , I will work on the above mentioned issues asap. Before making recent changes, E# was played as F, Fb as E and so on (which is not valid in all temperament systems). But after the latest patch, set temperament block (set to temperaments like meantone) will play E# and F differently. Please test these changes. (Currently, you can test using pitch blocks)

@riyalohia riyalohia force-pushed the riyalohia:Temperament_Widget branch from 0c013cb to e3f2ac1 Aug 23, 2018

@pikurasa

This comment has been minimized.

Copy link
Collaborator

commented Aug 23, 2018

Please fix this overlap problem when exporting custom temperament. It is small on the one hand, but could cause confusion.

screenshot at 2018-08-23 10 03 54 overlapping custom pitch

@pikurasa

This comment has been minimized.

Copy link
Collaborator

commented Aug 29, 2018

@riyalohia Are you able to work on the loose ends of this today and/or tomorrow?

I have some free time from work these next two days and want to focus on MB development. Please let me know. Hopefully, with a day or two we can have something that feels more or less "finished". Thanks!

@riyalohia

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2018

@pikurasa Currently, my academic schedule is in a very hectic mode and I would not be able to focus properly on the octave space issue as it requires more attention for a longer time. I suggest that we make this issue public so that others can work on it till my academics become a bit more relaxed. Meanwhile I will work on smaller issues whenever I get some free time.

@pikurasa

This comment has been minimized.

Copy link
Collaborator

commented Aug 29, 2018

@riyalohia That is fine. Thanks for letting us know. @walterbender please note Riya's comment above (the octave space issue will require some time).

@riyalohia as for small issues, this might be one that you could do #1220 (comment)

@walterbender walterbender merged commit 9ba16c7 into sugarlabs:master Sep 29, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.