surge-synthesizer / surge

Synthesizer plug-in (previously released as Vember Audio Surge)
https://surge-synthesizer.github.io/
GNU General Public License v3.0
3.08k stars 394 forks source link

Update Surge-plugin About screen? #157

Closed esaruoho closed 5 years ago

esaruoho commented 5 years ago

mainly talking about the image below..

surge_50-surge

says 2005-2015, Vember Audio, and Version is 1.6.0.. where are these texts coming from? I tried doing git greps but was unable to come up with anything.

whydoubt commented 5 years ago

https://github.com/kurasu/surge/blob/master/resources/bitmaps/bmp00158.png

baconpaul commented 5 years ago

Yeah this is just an image blatted on.

In an ideal world, our build pipeline would create some file which had some defined strings we could paint on the screen. But it doesn't yet. @jsakkine is it possible cmake could construct "versions.h" or some such at build time so we could do this?

jarkkojs commented 5 years ago

What do you want to that file exactly to have? With some scripting we could construct authors list from the commit log. Should the copyright be like that the copyright holders are the Surge contributors? Really would want @kurasu also share his views on this.

kurasu commented 5 years ago

I'm not so sure either but we definitely need something automatic that we just render at runtime.

I think we at least want

Not sure if copyright is relevant, but the GPL license might be.

jarkkojs commented 5 years ago

Does SVG exist for the Surge logo?

kurasu commented 5 years ago

No, it's a 3D render, but I should have a higher res version somewhere.. But it could also be time for a new logo, it's been >10 years.

jarkkojs commented 5 years ago

At least the author list is dead easy to acquire:

$ git log --format='%aN' | sort | uniq
Alexandre Bique
Claes Johanson
Esa Juhani Ruoho
Jarkko Sakkinen
Jeff Smith
Keith Zantow
Kjetil Matheussen
Paul
Paul Walker
Rich Elmes
Richie
簡志瑋

The only question is that what to do people who have only the first name? :D Wild guesses: Richie and Rich are the same person, as are Paul and Paul Walker. Not sure about the last one. Probably a good idea to check in code reviews that people have their full name as the commit author.

The problem with the above is that it does not work on Windows but I found something that looks usable: https://github.com/jgehrcke/git-authors (even explicitly mentiones the Windows support).

Before making hard decisions, can someone test if sort.exe /unique works as a substitute (under cmd.exe, not PowerShell) for sort | uniq? If it does, then that is probably the easiest route.

baconpaul commented 5 years ago

So the reason people appear twice - or at least I do - is that my GitHub id has proper name "paul" and my mac git global configuration has my full name. If you use "%aN %ae" you can see the fact that at least for me the "paul" is baconpaul@users.noreply.github.com and Rich and Richie are the same email address.

I agree with @kurasu list of what should go in the screen. I would also include a build time and build date.

jarkkojs commented 5 years ago

I could easily write a Python script using GitPython that grabs the authors list and filters out the broken names. Seems like the best option really. Once it is done, it is done.

esaruoho commented 5 years ago

yeah, auto-generated "all that's been discussed in the comments above" feels like the right way to go.

incidentally, there's a TON of lines in the code to the tone of

Copyright 2004 Claes Johanson
Copyright 2005 Claes Johanson & Vember Audio
Copyright 2005 Claes Johanson & vember|audio
Copyright 2005-2006 Claes Johanson & Vember Audio
Copyright 2006 Claes Johanson & Vember Audio

-- what should these actually say? I could take a hand and fix them all, but I don't know what they should say.

esaruoho commented 5 years ago

@jsakkine cool, seems like a good step forwards!

jarkkojs commented 5 years ago

I go with the Python script. It adds a build dep but otherwise is the only non-flakky solution.

baconpaul commented 5 years ago

We probably don't want to change those copyright lines, @esaruoho - that copyright still applies to that bit of code. With GPL3 and no contributor agreement (which is how we are working) each contributor maintains copyright to their changes, basically, and the GPL3 makes it open. I would be hesitant to remove any copyright information from the code.

jarkkojs commented 5 years ago
from git import Repo, Commit

if __name__ == "__main__":
    repo = Repo('.')
    authors = set()

    for commit in repo.iter_commits('master'):
        authors.add(str(commit.author))

    for author in sorted(authors):
        print(author)

A screenshot:

$ python3 authors.py
Alexandre Bique
Claes Johanson
Esa Juhani Ruoho
Jarkko Sakkinen
Jeff Smith
Keith Zantow
Kjetil Matheussen
Paul
Paul Walker
Rich Elmes
Richie
簡志瑋

Easy this too and the most cross platform way to go. And probably the generator could be a single Python script with "all included".

jarkkojs commented 5 years ago

Ok, the next problem: what the generated header would look like? What kind of data structures wants the GUI code to consume?

baconpaul commented 5 years ago

How about you just make a reasonable xml file and drop it in resources directory at end of build process - then we can try and load it and do whatever we want. Resources/build-info.xml or some such?

jarkkojs commented 5 years ago

JSON or XML. Either WFM. Does XML have any particular benefits over JSON?

jarkkojs commented 5 years ago

Revamping the about dialog raises an interesting question. What would be a good multi-platform way to present text and translations? There must be some best practice for that? Completely out of my territory. Ah, localization was the word I was looking for :) What is the best way to localize stuff like this? Would feel wrong to implement something complicated like this and hard code texts all over the place...

baconpaul commented 5 years ago

JSON or XML. Either WFM. Does XML have any particular benefits over JSON?

Pragmatically we already have an xml parser integrated with the codebase whereas we don’t have a Json parser. So less code expansion if we go xml.

baconpaul commented 5 years ago

What would be a good multi-platform way to present text and translations

Well there’s well defined apis on all platforms. They do a variety of things but here’s a reasonable mental model. Basically you make a strings file which contains mapping from ids to display strings, write one for each language, put it in resources, call a platform specific way to pick one, and then use it. Then walk your code and bitmaps to replace string literals and pre rendered strings with the id lookup or a draw. It’s a well understood path but it is a pain to retrofit.

jarkkojs commented 5 years ago

@baconpaul: OK, XML is perfectly fine choice.

jarkkojs commented 5 years ago

Isn't this a valid option for translations: https://en.cppreference.com/w/cpp/locale/messages.

baconpaul commented 5 years ago

Sure for some subset. But we also need bitmaps in each language and so need a way to inclement that. And if you read closely you will see that there’s still some implementation details on how to find the string files. That class is an encapsulation of some of the things I had mentioned when you asked how to do it though.

jarkkojs commented 5 years ago

... or you can use the same bitmaps and just translate text? I'm fine of not doing any type of translations for 1.6.

baconpaul commented 5 years ago

The text is in the bitmaps. So words like “about” are not painted with a font in c code so we could use an indirection to a string, but instead are painted by drawing a bitmap which contains pixels which spell about. So we either need to change that in the drawing code everywhere or do one set of bitmaps per language

And yeah I agree this isn’t a 16 thing!

jarkkojs commented 5 years ago

@baconpaul, Ugh, that I didn't know. Makes it fairly obvious not to include...

baconpaul commented 5 years ago

I’ll take a crack at this over the weekend but just doing a little pre thinking and wanted to remember this https://stackoverflow.com/questions/9240188/how-to-load-a-custom-binary-resource-in-a-vc-static-library-as-part-of-a-dll for when I turn to it

baconpaul commented 5 years ago

OK starting to plumb this together.

This diff https://github.com/baconpaul/surge/commit/7f47a746693ab4096abb9174c67150cf7f2815ce removes the pre-painted text and instead paints text which is at least coded. It also includes a date and time of build on the about screen. It refers people to GitHub for contributors and information.

screen shot 2019-01-03 at 7 31 42 pm

here's what it looks like.

So the extra step here is to create an XML file in a portable way (tough; can we assume python with the git module is everywhere? etc) and then on Mac put it in the resources directory after build and on windows put it in the premake and redo it with each build so that it is a compiled in resource. That's tricky.

I sort of want to get at least a date and time into the about screen though since if we start doing more regular builds can help with bugs.

So any thoughts on the above screen and diff? How would folks feel if I put that interim step in as a PR while someone wrangles with the build steps to create more information?

esaruoho commented 5 years ago

@baconpaul sounds good!

baconpaul commented 5 years ago

OK I'll put that first change in as a PR and leave it linger for a day or so to see if there are any comments.

kurasu commented 5 years ago

@baconpaul looks good!

cyanit commented 5 years ago

aboutscreen

This is the current (exept for the logo) about screen in the 1.6 skin (ofc only as long as everyone is happy about it). The temporary pink and green boxes are 20x20 pixels/20x10 pixels. Would it be possible it draw the version ino this way: Font-Family: Lato Font: Lato Regular Size: 12pt Space between Lines: 10 pixels

The first lines position would be X: 37/ Y: 442. position

baconpaul commented 5 years ago

We actually render with a draw here so use font metrics for a system font. We want the about screen to have build information. So if you can just leave the text out we can adapt when we get a skin together

We do need the vst and au icons I think. Also a gpl and GitHub icon would be lovely

cyanit commented 5 years ago

yes, understood that the text is just there for a visual representation of what it could look like. Github and GPL icons are a nice idea. Will look into that.

baconpaul commented 5 years ago

Cool. Is lato a free font? If so maybe we could load it from a resource in the bundle. If not we can’t redist of course

cyanit commented 5 years ago

yes, I think so. Just head over to google fonts. Should be one if its there.

baconpaul commented 5 years ago

Ok great No promises on being able to load a ttf but openframeworks can do it crossplatform so I bet we could coerce vstgui to do so also somehow

jarkkojs commented 5 years ago

Question about Python script. I can generate XML but wouldn't make more sense to generate a header with data structures that contain what we want? Authors and version are both static data that is directly associated with the build where they are generated. I do not see why take the extra parsing step run-time.

baconpaul commented 5 years ago

I thought about that. Where are you going to put it so it won't get checked in, still builds, and works with premake on all platforms. Seemed more fiddly to me. Better to just blat an xml file into resources I thought.

But I'm easy!

We sure python is configured on all our build hosts btw? I was worried about that a tiny bit.

jarkkojs commented 5 years ago

Doesn't premake have some kind of environment variable for the build folder? Output <build folder>/include and add it to the include path. You can use prebuildcommands for invoking script and outputting the header.

Of top of my head, locating the Python interpreter is done with PYTHONPATH environment in all OSs. GitPython does not come with default installation and must be installed by issuing pit install gitpython.

baconpaul commented 5 years ago

Probably. Yeah.

I know you know this - but we want to generate the header at each build time, not at each premake time.

Also we need to make sure the header goes some place that is .gitignored.

Does windows ship with a python interpreter by default now? Are you going to run the pip install in the premake or test for it and ask the user to, like we do with sub modules? Also python 2 or python 3. And so on. I wish python were more reliably installed on rando users box. But all surmountable. Just remember to put in a bunch of defensive checks for things like python isn’t there; python 3 isn’t there; pip doesn’t run; that sort of thing.

baconpaul commented 5 years ago

Actually thinking: One reason to do an XML file into resources rather than a header.

If the python blah de blah isn’t installed on a users machine and no XML file is created, I can fail to parse the XML gracefully at runtime and they still get a build. They don’t come here and open a github issue saying “how do I get python”.

If the python blah de blah isn’t installed in header land, though, #include “build_info.h” will puke.

So if you go python for header route we need to make sure that in the event python environment isn’t set up we copy some default build_info with all the fields defined to some state so the code still builds for that user.

Obviously we wouldn’t do a distribution build from that state. But it will save @esaruoho some pain in the Kvr forum one day I’m sure!

jarkkojs commented 5 years ago

@baconpaul, if you don't mind, I'll ignore your other comment just focus the one on premake. Doesn't prebuildcommands actually become part of the generated Makefile? What premake does is to build to the build files.

baconpaul commented 5 years ago

Yeah it does! And ignoring me is often smart. I think the thing we really need to be careful of, though, is being super defensive about the python environment on the machine with clear error messages. Do that and get it working all three platforms and a header file is definitely the way to go.

jarkkojs commented 5 years ago

@baconpaul, those were decent comments. Just not sure what to say to them at this point yet :)

baconpaul commented 5 years ago

Ha! Yeah I got that @jsakkine no worries here. Made me chuckle.

One solution I thought of out of this, though, was super simple. Write your python script assuming everything is in place. Then make a wrapper script (or two; one in sh one in cmd) which does

python blah
if ( exit code not 0 )
   copy precanned header file into place so it builds and has error values

then at least if the python fails the build continues and the about screen is wrong. Then we can disable the silent failure in azure builds later.

But I'm just tossing out ideas which pop into my head. Use them iff they are useful.

jarkkojs commented 5 years ago

I think we should have these two files in the root of the project:

I think AUTHORS must be maintained manually. The initial file can be generated from the commit log but already there we have inconsistencies that need to be manually resolved. Things like that might happen also in the future. Automation here probably is a bad idea in the end of the day.

As a side-effect this sorts out the GitPython dependency.

baconpaul commented 5 years ago

I had asked on the PR

What's the mechanism by which these stay up to date?

I pushed back on @kzantow when he added a file called "VERSION" also. Is that some sort of standard? Using a single file for that seems so odd to me.

curious on your thoughts?

jarkkojs commented 5 years ago

With the version file premake5.lua could probably create SURGE_VERSION macro definition by using io.readfile().

esaruoho commented 5 years ago

looking good at HEAD but i guess this could be tweaked further re: what @jsakkine wrote. vember_audio__surge__ 64bit__and_renoise__intel_64bit_

or is this ticket ready to be closed? @baconpaul whatcha think?