search

kometbomb / prototracker-modular

A modular synth tracker
MIT License
46 stars 5 forks source link

Add DisplayModule, fix OscilloscopeModile to show Pos/Neg and add Xcode project #21

Closed cupcake99 closed 6 years ago

cupcake99 commented 6 years ago

I made a module to display input values as floats to help debug/understand what is happening on outputs of other modules. Just a useful utility. I made it cos i like the Pd/Max paradigm of seeing the numbers as they happen. Works ok but not sure if my method is most efficient or whatevr.

Also, as i had been trying to use the oscilloscope for the same purpose i noticed that it was displaying positive and negative values inverted, so i fixed it so that it shows positive above the zero line etc.

cupcake99 commented 6 years ago

I've added a switch to the oscilloscope now to allow it to show either inverted or normal values. Stole the method from VUMeter to change on mouse wheel. Also changed name of my Value module to Display module to be more clear that it isn't a value outputter but a value displayer.

The module also now initialises the value to zero correctly on load whereas before it was sometimes displaying junk instead of a float.

kometbomb commented 6 years ago

I think the oscilloscope is fine even without the ability to switch the axis. It might even confuse someone if it's accidentally flipped. I can make the change later to the channel oscilloscopes as well so that they match.

Also, if you can remove the commented out code, then this is good to go. :+1:

cupcake99 commented 6 years ago

Hey, all comments removed. Got rid of the switch in oscilloscope module also.

I'll try and fix up the xcode build later this eve.

cupcake99 commented 6 years ago

Hey Tero,

I've moved the sprint() call into its own function now, and added a modified render method. I'm very new to all this C++ stuff and was close to tears last night trying to wrangle all the const stuff. Couldn't get my method to call sprintf() at all, and was thinking it was due to the way i had declared my function, tried every variation. No joy. Then i realised that calling sprintf with a class member char as the string to write to was the error. Declaring a local string in my function solved the problem. It's not ideal as i don't quite understand whether the variable lives until the end of the program run time, which is why i marked it static. Maybe you can think of a smarter way to get the same effect?

kometbomb commented 6 years ago

Ah, sorry! Didn't realize the render method indeed is declared const. That of course means you can't update anything while rendering and that makes things a bit more complicated.

Perhaps the best alternative would be to simply do the sprintf() stuff in the render method to a local variable (like you are doing now). It would still be much better than the 40K sprintfs a second. :) And of course the value probably changes constantly so it probably changes every frame anyways. I can add some notes in the code for this.

cupcake99 commented 6 years ago

Hey, glad it looks good.

I managed to get the render method to write to the string buffer set as a class member by marking it 'mutable'. Is that a bad thing to do?

Not sure, anyway, i updated my repo with the change. It seems the cleanest way to go as now there is an explicit char buffer instantiated with every display module, and the memory thing makes more sense than having to make static temporary variables.

It also cleans up the usage of my stringMeUp method, and allows it to be called more easily from any other place in future.

Umm, embarrassingly enough, i don't know what to do now, do i make a new pull request or kill this one? Not too familiar with the Github process.

All de best.

kometbomb commented 6 years ago

Actually, I would vote against using mutable. You will want to keep your const methods const, that is you know there will be no side-effects. The mutable keyword is useful for basically only something like reference counts or mutices (you need to be able to lock a mutex even in a const method). You do not want to normal members change values etc. when you render. Clearly something outside the actual scope of the whole class. Just use a local variable instead of a member and pass it to stringMeUp() (or just do the sprintf() in the render method).

2018-05-24 23:04 GMT+03:00 cupcake99 notifications@github.com:

Hey, glad it looks good.

I managed to get the render method to write to the string buffer set as a class member by marking it 'mutable'. Is that a bad thing to do?

Not sure, anyway, i updated my repo with the change. It seems the cleanest way to go as now there is an explicit char buffer instantiated with every display module, and the memory thing makes more sense than having to make static temporary variables.

It also cleans up the usage of my stringMeUp method, and allows it to be called more easily from any other place in future.

Umm, embarrassingly enough, i don't know what to do now, do i make a new pull request or kill this one? Not too familiar with the Github process.

All de best.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kometbomb/prototracker-modular/pull/21#issuecomment-391841600, or mute the thread https://github.com/notifications/unsubscribe-auth/ABCK6Z1GDaqLAn7oxeO7htXcq1QYNbUGks5t1xI2gaJpZM4UHBDh .

cupcake99 commented 6 years ago

Mutable changed back to local variable declared within the floatToString method (formerly known as stringMeUp).

I kept the same name just to save myself typing 9 more characters. But it's still prefixed with 'm' so maybe its not correct to have that name as it's no longer a member but a static declared inside a method. I was reading up on the way local variables are created and it looks like c++ will instantiate the variable just once on first call of the method and then it will exist until the program ends. But i was worried the static identifier would make it shared amongst all instances of the class but seemingly that's not the case and each display will have their own one.

Once you're happy with the pull i will update my repo to show the key settings i am using now, so you can see what i did to make things easier for OSX. Also the Xcode project is ready to go, and i have Proto running as a native cocoa .app which is fully portable (tested on second computer that knows nothing about SDL, all went as expected).

kometbomb commented 6 years ago

Yeah, the m* (or whatever) prefix indeed should always reflect the variable properties. A static variable will always just be there globally, which makes it inherently thread-unsafe because it's shared by all instances as you reasoned. It's safe to use if you have something like the constant strings for input/output names - it doesn't matter if many instances read it at the same time because it never changes. A local variable is on the stack so it is instantiated every time you reach that part in the code (it is always thread-safe in that way, unless you pass it to other threads... you can always make things unsafe if you try).

A common pattern in C/C++ context (and others too) is to have a local buffer in the stack and then pass it forward. This way the "workspace" used by the callee is not shared (BTW, note the use of snprintf(), that should be used instead so that you won't overflow the buffer):

void doStuffWithTheBuffer(int a, int b, char *buffer, int bufferSize) {
  snprintf(buffer, bufferSize, "%d %d", a, b);
}
...

char buffer[100];
doStuffWithTheBuffer(10, 50, buffer, sizeof(buffer));
printf("Result: %s", buffer);

I can merge this and make the little changes myself, no need to bounce back and forth. I need to look into thread safety in the sprintf context, this is in a few other modules so it needs some generic solution shared by all. But if you're interested in learning then we can continue this. :)

Is it possible for you to make the Xcode changes a PR but for the main prototracker project? That sounds really useful. Those changes will then of course eventually bubble to this project as well.

cupcake99 commented 6 years ago

Yes, if you want to merge and then alter the module to make it work the way you think best, i'd be quite happy. The thing is that really i'm very new to c++ so don't know much about the general conventions of usage and such. Though i can say this 'back and forth' has indeed been very educational and i've been doing a lot of reading. Mostly i'm just having to familiarise myself with how c++ deals with things in the way i already understand from more 'friendly' languages that have a lot of syntactic sugar. I finally got my head around the use of references and pointers which were truly making me miserable. I think the big problem is the operators c++ uses are so opaque at times (& is a real tricky one). And then there's the const thing... yikes.

Last night i read a whole article about string formatting and was thinking to change my method to use snprintf for the same reason you mention. I thought that you had made the string buffer [50] chars wide in the const module so that the issue of overflow would be less immediately problematic, and that's why i copied the way you had used sprintf.

One thing that's bothering me slightly with the display module is that it shifts the numbers right when going negative as it has to display the '-' in front. It is also currently useless at displaying the what's coming out of the const '/1000' output due to a lack of precision in the display. Of course, space constraints make it awkward to deal with this. I'll have a think and see if i can come up with a solution. At least it works ok for now!

I'll also make an Xcode project for the main prototracker branch then, and see if i can get it building as nice and cleanly as i have this one. It shouldn't need much more than a swap out of the source files and names basically. Famous last words...?

The modifications i made to the Xcode template taken from the Klystrack project in fact work backwards and i have applied them to Kylstrack successfully. The chap who made that one was being doubly sure that the binary understood to look for SDL in the app folder, but it didn't need the shell script he added after all. Xcode already provides a stock setting to control that. Anyway, not to go on too much, but it's nice to see there is a clear path for getting these progs on to Macs.

kometbomb commented 6 years ago

Yeah, reserving a lot of buffer for sprintf sort of makes it safe but it's not clean and depending on implementation it might for some reason overflow the buffer with lots of decimals (even if the format limits them). Using snprintf is the way to go.

You can force sprintf to always display the sign of the number with a format like %+.2f. But that needs one character. Maybe you could do something like rendering the text in a different color if it's negative? And about the /1000 scaling, maybe add another input that reverses that by multiplying the input by 1000 before printing? Something like valueToBePrinted = getInput(0) + getInput(1) * 1000.

IMO it's awesome that you are taking all these C++ troubles educational without giving up. :+1: :+1: I am not in a hurry to merge this if you want to play with the code still.

cupcake99 commented 6 years ago

Haha, well, i secretly have selfish plans to use this knowledge to make my own music program one day... But keep that info under your hat for the moment! Also, the challenge is engaging. I really dislike things feeling opaque to my understanding, so have to muddle my way through until it makes sense. I had the same situation when i first started using Pure Data.

About the /1000, i thought something similar. Definitely involving a second input to make it clearer what's happening. I really like this idea of using colours to show positive and negative. Very 3D! Red and green instantly jump to mind but then red seems as if it could be showing something 'wrong'. Will have a play around later.

I have forked the main prototracker branch and added an Xcode project to my forked repo if you want to have a look. It's probably totally cryptic if you have no ability to open the proj file in Xcode yourself, but basically all it does is make a wrapper around your source code and tells OSX how to build it, the same as the original Klystrack project that i used as a template. If someone on a Mac clones the repo it should work straight away (as long as they have the SDL frameworks installed) because the Xcode project will look in ../src for its source files. So, it really depends on the directory tree remaining the same. Most of the files inside Xcode are basically just pointers to the files outside, it doesn't copy them anywhere (except when building the app, at which time it copies the assets files into the app bundle), but it can modify them like any ordinary text editor.

It's quite fascinating looking at the logs that Xcode produces, it's all just command line stuff that you could type in yourself, and certain build options are just ways to pass in flags to clang. For instance, i have it setting 'SCALE=2' the same as your makefile, so that it doesn't require modifying the source at all to build.

kometbomb commented 6 years ago

Thanks for the Xcode project! Does it need a manual update in Xcode when adding new files? If so, then it's probably a good idea to mention it in the docs. Need to get a cheap Mac to test my projects on some day. :)

I just had an idea: maybe you could also render the sign manually: render a small line (i.e. use Renderer::drawRect()) that's maybe a few pixels wide. But on that note, I have been meaning to update the GUI to be a bit higher resolution so that the font can be smaller for similar reasons.

cupcake99 commented 6 years ago

You're welcome!

Yes, any files you add in your repo won't be seen by Xcode until they are explicitly added to the project. Anyone familiar with Xcode shouldn't have a problem being able to do that but i can also keep on top of it and do the update if you let me know. The great thing (at least i think so...) is that it just looks for file names and is totally agnostic about the contents, so when switching branches and stuff it will build whatever is in the branch version of the files. There is some kind of 'source control' inside Xcode which uses git to pull new files automatically i believe, but i found it confusing to use so didn't bother with it. Could be added in the future.

Talking of branches i have made a new one in my modular repo called 'wilderness' that i'm using as an experiment lab. So far i have added a method to cut the whole synth grid using 'shift-F3' which is really useful for containers. I also fixed the ExtIn and ExtOut objects to display the output clearly as i couldn't see them on my machine and it was confusing to say the least trying to mousewheel them and ending up with a shitload of inputs/outputs leading nowhere.

I also made a new module called Int just for doing large ints quickly. And i changed VU meter to show mode 0 on mouse wheel up and mode 1 on mouse wheel down, because the %mod method made it too fiddly to get to the right one when using a trackpad on my Macbook. Also i changed the display module to not show the sign at all but to turn slightly darker when negative. I tried using special colours for pos and neg but it just looked too cheesy. Adding a little sign display in the corner would be the best, perhaps will wait for the higher res update to do that.

Next up i want to add keyboard control to the synth grid so that the arrow keys move around modules and 'shift + up/down' modifies the param of the current module. Think i can figure it out, fingers crossed!

My mods to the key commands are also there inside MainEditor.cpp and SynthGrid.cpp. So far i have added:

None of this is in my master repo yet. Thought it would be easier to merge it in stages. I also have a bug when copy & pasting containers where it doesn't draw the wires to the container correctly until you change track and then go back (audio still works fine), but i want to see if i can fix it myself...

Ok, have a nice Sunday!

'''''''''''

(edit: haha, my silly little face didn't come out quite right)

cupcake99 commented 6 years ago

Hey Tero,

Just wanted to give you an update on the terrible things i've been doing to your poor helpless piece of software in my wilderness branch...

I may have forgotten to list some things i did. Pretty much all the major ideas i had about what could be possible have been added. It's not always pretty or implemented in the most sensible way, but i just wanted to see what could be done, and learn some things along the way without breaking any functionality.

Ok, all the best, Byee.

kometbomb commented 6 years ago

Those all sound very good, especially the keyboard accessibility things. As with all generic changes it would be really cool to have those in the main prototracker branch.

I have been thinking about adding the ability to edit the ConstModule value by keyboard so it would be nice to see your SynthGrid changes in case they are related. Don't be afraid to make a separate PR of them. :+1:

It would be good to merge this PR now. I will now do that and we can continue the discussion in a new PR.

neauoire commented 6 years ago

I've been secretly following @cupcake99's XCode fork, and I'm glad that's a thing. It works really quite well 👍

cupcake99 commented 6 years ago

@neauoire very glad it works for you!

If you're interested in the seeing all the extensions to the prog i've been making please have a look at my 'wilderness' experimental branch which has tons of additions. I'm planning to merge them eventually, but just for now it's there to see what i can get going.

The build is pretty stable and has some good usability improvements for Macbooks in particular.