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

Build Surge with a fork of VSTGUI #527

Merged
merged 1 commit into from
Feb 8, 2019

Conversation

baconpaul
Copy link
Collaborator

This diff begins building Surge with a fork of VSTGUI rather than the
VSTGUI which ships with the vst3sdk pointer. The rationale is described
in #515 and elsewhere.

It accomplishes it by the following:

  1. there is a fork of vstgui called surge-synthesizer/vstgui with a branch
    called surge which is master + diffs
  2. There is a submodule in surge called vstgui.surge which points at that
    branch
  3. There are changes to premake to point our vstgui implementation

This first version points to a branch which has one and only one
vstgui fix in it; which is submenu checkmarks on windows. But
by building this way we address that problem.

@baconpaul
Copy link
Collaborator Author

@falkTX @jsakkine @kzantow this is mostly a git change; would very much appreciate your once over to make sure I have done it correctly. I will certainly not merge this without some consensus building.

doc/vstgui-dev.md Outdated Show resolved Hide resolved
@baconpaul
Copy link
Collaborator Author

OK @falkTX I pushed cleaner documentation. Clearly I was too close to it when I wrote it.

https://github.com/surge-synthesizer/surge/blob/cbe193d05e67b1277424120852e97a48d957fa9f/doc/vstgui-dev.md

thanks for looking. Sorry it was so sloppy. Again this was fiddly to do and tricky to describe.

doc/vstgui-dev.md Outdated Show resolved Hide resolved
@baconpaul
Copy link
Collaborator Author

OK thanks for the looks @falkTX - all updated.

This is a big change so I'm going to leave this open until the end of the weekend so others can offer comments. @jsakkine @kzantow especially want to make sure you get an opportunity to weigh in.

Best.

@jarkkojs
Copy link
Collaborator

jarkkojs commented Feb 8, 2019

You should document the reasoning. We should have an architecture document that should evolve over time. Now it can just have this documented. Otherwise, sounds good.

@baconpaul
Copy link
Collaborator Author

Yeah will copy some of the text I. #515 into the document before I push good idea

@esaruoho
Copy link
Collaborator

esaruoho commented Feb 8, 2019

@baconpaul did you see the conversation on the surge-irc chat where rgh had switched all Arials to Lato and there were less crashes on the gui?

Files in question:

vst3sdk/vstgui4/vstgui/lib/cfont.cpp
src/common/gui/SurgeGUIEditor.cpp:SharedPointer<CFontDesc> patchfont = new CFontDesc("Arial", 14);

@baconpaul
Copy link
Collaborator Author

Not yet; but the problem with using Lato isn't changing the code (tho that's good to know it helps) but is getting the font file loadable on the users machine. That's the work in #214. I'll say the same in slack!

Copy link
Collaborator

@kzantow kzantow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and makes sense to take this approach 👍 ; hope Steinberg moves reasonably quickly to get fixes merged though

@baconpaul
Copy link
Collaborator Author

Thanks. As discussed on slack, as well as @jsakkine comment on adding a motivation section to the doc, I'll also add a comment on the steinberg branch strategy

This diff begins building Surge with a fork of VSTGUI rather than the
VSTGUI which ships with the vst3sdk pointer. It accomplishes it by the following:

1. there is a fork of vstgui called `surge-synthesizer/vstgui` with a branch
called `surge` which is master + diffs
2. There is a submodule in surge called `vstgui.surge` which points at that
branch
3. There are changes to premake to point our vstgui implementation

This version points to a surge which integrates three of @sagantech's
changes

Closes surge-synthesizer#394: Cannot move window after selecting preset
Closes surge-synthesizer#396: Diacritical marks in menus
Changes for submenu checkmarks on windows

Also this diff implements our fork strategy described in surge-synthesizer#515 and
closes surge-synthesizer#515 as a result.
@baconpaul
Copy link
Collaborator Author

OK after review from each of you I"m going to merge this.

@baconpaul baconpaul merged commit 7314657 into surge-synthesizer:master Feb 8, 2019
@baconpaul baconpaul deleted the vstgui-fork-515 branch February 15, 2019 14:44
baconpaul added a commit to baconpaul/surge that referenced this pull request Jul 10, 2019
This diff begins building Surge with a fork of VSTGUI rather than the
VSTGUI which ships with the vst3sdk pointer. It accomplishes it by the following:

1. there is a fork of vstgui called `surge-synthesizer/vstgui` with a branch
called `surge` which is master + diffs
2. There is a submodule in surge called `vstgui.surge` which points at that
branch
3. There are changes to premake to point our vstgui implementation

This version points to a vstgui which integrates three of @sagantech's
changes

Closes surge-synthesizer#394: Cannot move window after selecting preset
Closes surge-synthesizer#396: Diacritical marks in menus
Changes for submenu checkmarks on windows

Also this diff implements our fork strategy described in surge-synthesizer#515 and
closes surge-synthesizer#515 as a result.

Former-commit-id: ce8bc4274c88df2915b9b3abac26078f273959a6 [formerly 7314657]
Former-commit-id: 0c72f1c331f2281cbd9de138a495d999c172a39c
Former-commit-id: c97dce7fb3bbc8c8d3a44af05e4c7b8022c6fde1
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.

5 participants