no-menu-animation.diff

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
 	}
 }
 

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!

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?

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.

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!

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 ?

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.

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.

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