Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
diff --git a/vstgui/lib/platform/common/genericoptionmenu.cpp b/vstgui/lib/platform/common/genericoptionmenu.cpp
index 58fe6a89..d4408813 100644
--- a/vstgui/lib/platform/common/genericoptionmenu.cpp
+++ b/vstgui/lib/platform/common/genericoptionmenu.cpp
@@ -16,6 +16,15 @@
#include "../../controls/coptionmenu.h"
#include "../../controls/cscrollbar.h"
+/// Surge ///
+#ifndef VSTGUI_OPTION_MENU_NEVER_ANIMATE
+# if LINUX // serious lag with animation on the Linux platform
+# define VSTGUI_OPTION_MENU_NEVER_ANIMATE 1
+# else
+# define VSTGUI_OPTION_MENU_NEVER_ANIMATE 0
+# endif
+#endif
+
//------------------------------------------------------------------------
namespace VSTGUI {
namespace GenericOptionMenuDetail {
@@ -264,6 +273,11 @@ private:
}
else
{
+#if VSTGUI_OPTION_MENU_NEVER_ANIMATE
+ /// Surge ///
+ subMenuView->getParentView ()->asViewContainer ()->removeView (subMenuView);
+ subMenuView = nullptr;
+#else
auto view = shared (subMenuView);
subMenuView = nullptr;
view->addAnimation (
@@ -279,6 +293,7 @@ private:
if (auto frame = db->getFrame ())
frame->setFocusView (db);
}
+#endif
}
}
}
@@ -517,7 +532,6 @@ CView* setupGenericOptionMenu (Proc clickCallback, CViewContainer* container,
viewRect.inset (-1, -1);
viewRect.offset (1, 1);
auto decorView = new CViewContainer (viewRect);
- decorView->setAlphaValue (0.f);
decorView->setBackgroundColor (
GenericOptionMenuDetail::makeDarkerColor (theme.backgroundColor));
decorView->setBackgroundColorDrawStyle (kDrawStroked);
@@ -538,10 +552,13 @@ CView* setupGenericOptionMenu (Proc clickCallback, CViewContainer* container,
decorView->addView (browser);
container->addView (decorView);
+#if !VSTGUI_OPTION_MENU_NEVER_ANIMATE /// Surge ///
using namespace Animation;
+ decorView->setAlphaValue (0.f);
decorView->addAnimation ("AlphaAnimation", new AlphaValueAnimation (1.f, true),
new CubicBezierTimingFunction (
CubicBezierTimingFunction::easyIn (theme.menuAnimationTime)));
+#endif
frame->setFocusView (browser);
if (!parentDataSource && optionMenu->isCheckStyle ())
{
@@ -605,21 +622,29 @@ void GenericOptionMenu::removeModalView (PlatformOptionMenuResult result)
if (impl->listener)
impl->listener->optionMenuPopupStopped ();
+ /// Surge ///
+ auto onCompletion = [result](GenericOptionMenu *self) {
+ if (!self->impl->container)
+ return;
+ auto callback = std::move (self->impl->callback);
+ self->impl->callback = nullptr;
+ self->impl->container->unregisterViewMouseListener (self);
+ self->impl->frame->endModalViewSession (self->impl->modalViewSession);
+ callback (self->impl->menu, result);
+ self->impl->container = nullptr;
+ };
+#if VSTGUI_OPTION_MENU_NEVER_ANIMATE
+ onCompletion(this);
+#else
auto self = shared (this);
impl->container->addAnimation (
"OptionMenuDone", new AlphaValueAnimation (0.f, true),
new CubicBezierTimingFunction (
CubicBezierTimingFunction::easyIn (impl->theme.menuAnimationTime)),
- [self, result] (CView*, const IdStringPtr, IAnimationTarget*) {
- if (!self->impl->container)
- return;
- auto callback = std::move (self->impl->callback);
- self->impl->callback = nullptr;
- self->impl->container->unregisterViewMouseListener (self);
- self->impl->frame->endModalViewSession (self->impl->modalViewSession);
- callback (self->impl->menu, result);
- self->impl->container = nullptr;
+ [self, onCompletion] (CView*, const IdStringPtr, IAnimationTarget*) {
+ onCompletion(self.get());
});
+#endif
}
}
@baconpaul

This comment has been minimized.

Copy link

commented Sep 5, 2019

Well this is fantastic!

But I'm not sure in the case where false came in that we want to reset the view to null. I'm not sure we don't but it does seem like a change in that case.

Perhaps a better way to do it is

  1. Retain the default argument as true
  2. Leave the false case as is
  3. Make the true case
     subMenuView->getParentView ()->asViewContainer ()->removeView (subMenuView);
				subMenuView = nullptr;
#if 0
 /// prior true code here
#endif

(or appropriate flow control) which we know works and makes the true case the same as what you have here. (I think the function is never explicitly called with true; only false)

That way we have the minimal change.

If that works I can apply the patch easily!

How cool!

@baconpaul

This comment has been minimized.

Copy link

commented Sep 5, 2019

Also there's two other places where the genericoptionmenu triggers timers; I wonder if we want to whack those also with the end point similarly rather than have an animation?

@jpcima

This comment has been minimized.

Copy link
Owner Author

commented Sep 5, 2019

Ok, I undertood what you ask to do. I will find where are these other animations located.
A thing: should this pseudo-fix be guarded by #if LINUX and left reserved for this OS?
Also, should I leave a comment mark to indicate this modif is a thing of ours? as /// Surge /// or something.

@baconpaul

This comment has been minimized.

Copy link

commented Sep 5, 2019

This code is only used on Linux but we Could offer it

A comment with surge in it is a good idea. I’ve been using the git log to know the differences but that’s hard on inspection

Thank you for doing this!

@jpcima

This comment has been minimized.

Copy link
Owner Author

commented Sep 5, 2019

I modified the code at the call sites of addAnimation.
At least in Surge's context, the menus seem fine to me.

Since I'm not entirely comfortable with anything of Vstgui, does this code seem logical to you ?

@baconpaul

This comment has been minimized.

Copy link

commented Sep 5, 2019

Yes it seems logical to me. And this fork of vstgui is only used for surge so it is almost definitely the correct one. Do we still want the animation on the decorView?

I am happy to apply this and walk it through the (somewhat complicated) build and branch process if that's OK with you. Let me know.

@jpcima

This comment has been minimized.

Copy link
Owner Author

commented Sep 5, 2019

Do we still want the animation on the decorView?

Not sure what you mean here. decorView animation is disabled.
It's the one that occurs on the menu opened. (and, note: it must not show at alpha 0.0, or nothing is seen)

I am happy to apply this and walk it through the (somewhat complicated) build and branch process if that's OK with you.

You can go ahead. If it's about being credited for the commit, I don't care, take it as yours.

@baconpaul

This comment has been minimized.

Copy link

commented Sep 5, 2019

I can easily add you as credited on the commit. OK I'll walk it through. Thanks!

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.