Code feedback on this custom envelope panel

dannytaurus

Looking for some feedback on the code for this minimal envelope panel I made.

I needed a tiny (130x42px!) envelope control for my synth and wanted to take the opportunity to try making a custom one. ChatGPT helped me a lot with the geometry and value conversions!

I kept it deliberately super minimal - not curve controls, no hold section. Just A/D/S/rR drag handles with fixed pixel ranges and abstracted value ranges from 0-50.

How does the code look? Never made a custom panel like this before. Am I on the right track?

Snippet and script below.

HiseSnippet 3916.3oc2arsaabbcos2TKkqNns4B5CiIJLHsoj4RcwVxIwjRjxRMRxJbocrgQfxxcGQtPK2kY2kRjwvn4Kn.8s9Veq8SnOU.CTz2yOPQ5mP.5GP64Lyr6NK4R5KMtQoBVhblyYl4b+blK9.eOSZPfmuRt4aMpOUI2anpOxMr6lcMrcU1otRt2SstuQmNFscnMbOg530m1hFDprwn9FAATKkb4N+cPjyM2ETX+782dCCGCWSZRWJJ22y1jtqcO6vjdOn5mZ63rkgEskcOIrWt5Nldta543M.HryqVVoug4wFcn6afncNUksMB5pj6pplVKatr4psWVyv3nan0dUZ6apcCM5MMWwZMSSCyUZapszRFJ4dsFV1gd95gFgz.kbWXCOqQ5c8N0ku.22NvF3PrglhNrx7t2xywBYQrWkM6Z6XcPjDKPQIm5AIxuyyke+b08rsri6OQN9NL.jjQHK.yctzj24SQdZxjWYIxKCRJmDIcANIcIUcSe69gIPP540U2wMj5ejAnmjIENtJm6u+qT2zCvvMbwdFGS2xGZDOhBKWtbIRkxkKdq4ivJfFdu.511c51jF34LHz1ycKO+CLboNAEB8GPAjmGTrAgj9tNfwD4iIQCtCMbSud88bgFExyAmu3sHW+5jsrGRsHA1eMknsT4gKWQLGT1DvrVwgumm0.GCPFVHOL1Z85KFdssqq2jzCgRmed9LizZg7FNNdmtI721f0UP9Rj7a5XCeqDYauSn9jqPXV91tcxiTNLWMoCBPGAReivtji77IV9FmBHPB6RQBh4djPeGfnkvjl9TP8hcV.lPNVAg9dGSAjdLoUWX0cAMz5DsEKSdBeM20nMH+DX6z1o1TkZHv7wSLzp9rPsdJT0mEp5oPs4rPsYJTObuZO.vdkxKVlyLbkgQXnuc6Afkt.0cp+fCQ9BDY3TVKB9NtVzgExCsAMTxLinWe5nWmZZLJM15SGa8AAgfOZZ7aNc7aRcnFAzHSBLxEw2vsCMfYPvo0RDFQThHvVL06sy9GtGRKZn.QzWsGv6C7lhkSBhh.+yZCAh52SuUMXBpuwgv7.CXAM9.lDLSpGOYbGn9vecHldd9VQh8CpseicO7yQ5YIoYh281P2KWIo2F6e+CePYnyajtuGlQeOPCmyJKMFlXuKsRDYssgqE3J0y1858LFRNwvYPrAAXNzpUsM+TAmJM8IPXLoLcCFEM1r1COTGXfo.hOnUSAQH3f9WKU+MaraiZ5MDjvTfwlOsJbYMG5F01cWznG5hjvl9FV1CBjP4vJ.RLbuJohLlAe0.CepEJitLY6F6dPil5yO+QCbMwHpDSGid8KLrDwnDocQHrg8QjBCIeDwnHwmFNv2kXPfHFTAfOAwR.nMGfn0vaQdBSSrumeOCGL.aeLXsvHgTn7hKpUjYUiwrl210w1kRhoD2gEFhDPzzQtdjEENwSf8nBijvdTL1ayHiwQuuO3XcOW6vM7F3ZETnew4e77y0eQSvexGCeBeG7Q7C2mdp9f1rfpfVnDZ1mMTMDpFCZVqGfxtPO0ZCqUIxPsRjQvuCq.eVQr1iOin.PqXIFqoUjup3r1xiAph.Tkh7kDClCPM7IgXPidF86ioNlQTiwIxdAs7PkEjVERMcZgdALJCUz8BHezGKBvDquYlkyIZvMbPDWHFuqSJHh+D2GSBNCAkKr9s71K.nAVJWZA2XhvEoATADQ.74LgFDA.ulfXbkVLvMXBZIRroeL8THDlrDKJBYgEzp.wHgbJPjDqE56Y6FVbBhNfic81s7dXAq1LB1wyzvgXAdgbhwpcowhwVZrfpXHeFeZQ9DL7KDcr37yIlHWVH2U.dC.eMBCHHewOAVCf.RAz6K0..TWXrEkoTXycFfDSTr7TDVcARg3uwiIin5lo9aTKO8HoA3QlHIFMJVT.lf7oQ7oFZaKQ0wK1nQLxc7EORN4hxIfhQojfh4b10PXKv.AiGEaWkIoRjQBzGKoGabWmOtqNUwjvr4xj6z3t60nUyGNoT.RreeCnrTF+KVK3ayUa8Ix6WfUcRwR.z5SAZcFT8o.UmAs4Tf1DDNOISMkITYEf0AnIsfR45.rt2XF.k07tMvPC.nwiRbxh0RzeFrHMXVqT4XulXzfPMUF1ERglzhI7ndHLQobtNYQcIrrlJIUOgjrvfd6YL7Aw1eQD40RkSuTDwWRNe9XyCNIIC2JhoRViEh.KQl9SkLalL897fxbQmbwAWSLdlrStxfERiXQIGWzVikNYcHk3v0EUWA4ZVOxm9Ik3liLvwLdD7xb30Ev4rNCbrh4IBaRFBwk4jANME3DwfSPETWKYxTKEBQVurpU1oEoUC8V6r+clzb1xNHrRACrxEH9Ra3y1xgerFhZtgfPq8vDUJFSxXD14njrHVCuJfMncGcUqQY55z0NjWMEKWNPuRKTaJTA1GSxm+Vx8TOoZL7iXXGSGE.fdTd87kxWC9sN7ay7eApKw7PENFKRGwB7iebbvcbae8ezw.ZyIkngKBP5oDo6hCw+LBMvDIUfjmLRACWhS0bQDl0shZAMNFafh8X4ABHVGrasMZrqdVE3XUQjpV3AyDBfXbOnVlE8wxr.3IR4BAK5Pc6f6fEJwsH41jB4Ki3GTj.lPYJ2QGn6Y+rqKIc5jmUIIiWCiDAmYcDhcdhkxTJtQlzqUaF89+QUDLgDx8ERfLnukQHkelCEhUhgcsCVD.XrnkuQmEM.bOgVjjXtOVFIn61N03mxRHcXHd1J0VGscXVgBqDLuTwHrqmF65Yfc8Dr0SisdB1BMJlFJF6lowtYFycSN1OguAoGmMGzpU9oQtM1L+zns6omeZDRicYfdRju6Aflsk7gTc.DkNro2fPPIUHRIUnCSw.0EiIoH3oW0goqIPHORaXuaT+4mqyhGY63Tywgq85fKb6N7CUMOiY6fq.uCIbPhKMV3Yb0DmepUSpYngaGHt5iJWpbIskJWZ4JeQIxxkHUX9BQDUzYgw1W07ycBrom9neU5RZvcLM197DmZFCjzVxDcWhH1EFF7L5qivuVi2SMd.0oLzDjvOpyaUelCIAI7CcdK8YNjDjvOZxa0blCIAI7CHeKuM9kQSWUYGR6Mophs0z34NB6cQ+TgLtLDHfetiozYcYYLC.W9wiRBP2v.rjfzbQoSwDTD3GL7.zMliXrnDm101raQNV3OOe7.hIOn3y63pvGHe.bq9FNN18C.iTrbBLcNPxweie3KQe9EbGPX3wbIlleBKpDn0yOowSBT87SZmj.sY9ILIdB+fDuLYu6dO8FxN+64MHfFc9zId+Tl2+T8n3QroKZhmkM0RTURV5FBL7TEJQQ5hJJIIyX9XBpLmpKi0RvEkQD.BkbkqPxN4AmrPt3TR1lNHqvlpSQiKPoDWVDNpgwYoYTs7tTJk5PBYLCaHrzqCGeKMXx0Yukm4vJfYZjw1SXoLOLjhh5ylK9P33LP8zLvj65gYxMwddDFhisimLEBBTim4LXcFJLdVdGQ7dmNmV+EiS0S3zLmN8RoOEBzjaZyUS9bM2T06RavpT5SlMSMu79w3p9YuisowCMmoHA9S5Joh8n7o8wb5ER6qzCc0uW+rcVic8NBJuhdqY3My1Syy2JGG1Ym82AJ4X7wHhBEuFrKnhSHqyHiRD1ptNrCBX+ewoJ7b22KjdWWdsiPcMjwAczQYBCuQIeOGGpelfwqYweVCrf6fdso9k32iPLhJ4tP5aVUc52rp7E+ZxufKID8b2ApR4t8otS65fUD2JF7s6sScPxgWGqnO.u9T+PajDxUmdhsIke4ryoVmFbbnWeFthqRSI2aDxf9lQWcKdt7J1VJ4tnJ66ZJCiuu7u4QUGkb446T8TaqvtIcXWsK0tSWo6c+gU4EIpa+0otidduMY2TQp96HeW7IYdwWKvxUtoVYs0ztwZR.pLFjjZJ4SQNUIV8slNqxsBkY0Zxr5xiwpU+xIX0FUSeKuJ49P0ocKuR7Ii5KeyadiUVaoIY3UWc4kVa0aNNCWob4kWa4JqrVEkrjue+syV99NUkEO7UtxZqr7Jqpz2vGDQRxpXsuhTmymV.xbhYBvWSE2DyTEe2ksvfxPER2oLlUS0pGAVto4fBfvztiaO1pBSN8nPEJ3cYD+zD9gffqKQvO8frIXHW3YGBVWhfAeqLIXXWfmcH3lxR3uLaBFxB9Jmfm7op7NpwOiC4WNC99hD.ff9odtJ3SRwMvNbjri1OXOmlmWR7RpGXGZ1MaZ7bYPiPpgWEzn3QH8lpMN5HXq5ID3ET25AuZdwQxK+GvW9WW8yMNgxdYNrE+WvZeDdF42g5R8QYm1Ld.Ye6y6CHS449AjcWyPfDZ4a3Fz2KfpIMGOUm1ytEXbFH263inRliPtWk5zvAtolZQWovBEEPEi8jv6hUi5rhrUL3ljZxf1olo8rGJ2rgK5zoSAN25tAlffAkyolvAPgaeN5TKMu+SotqH281F9VfNzTdMzwCb4ftFokfRcmh.yc9Y8n6zdIezcu1YtGc2OABjk5cANWDMpa2qexy5jQiuqnSRTuocT2yy0qeWOW6TlEMovVi5zglxZKSFh+PGROV1sNkTD2erJ+UR.xI5KorP6E9MRlo958U4jK+Qd8S2DPm+UaBnTxyKEkBXKG5P1y8iQe+RVaw6+KSSqy8BXZsgceOGC+ojaMKqrsgXJoCLaZDMju4O8g+lphSiPN.3DVl+1piaSxWnco.+LY2aNv+jHre5+5e7c2lsnx8pz5u8cQKSp9wk+GL68ejKPHqPOuq5AdNi3Z6srcBo72j7bp7Fyp3.Eks7oe0.pqYj9i7691+b0OSV9mVM89U+rAPUpos9ElP+25Y7LhzbgmS2Wv8HhoH+3me3s4T5OSMw+8hp7GU8qfjASX3OoaB9SJe5p+0A+gpY57MaWcV5kzt5O8WuyUdZZWcxk9K+9pML8d0kAR84zt3Mky.cFLsyaEQebsvYOB7MTYp+ypxu2VM5cLdVU.9VpBeiyphvKwJp9rc0YefpHcCACye1lVeK0O6rMAh6oa3YaR78TgMfWgvJIm7iRM4J7GMRm8LfjwCgDN6Onmt2.eSJPkt3+irf9xcN7RK3swCkOGau7TWKVi+M7i.nF1Nm.nVDv+mrF8LL88NzjeIOXpyKx5A3aW1+Y.mScOrMQSgcwOxBydvV+OzzL8TMw.q7xNvkdYG3xurCbkW1At5K6.uwK6.u4ydf3IyTaPnWOtughxdGzfcqY4xwOJKlahx+AkFwJS.

Full script:

Content.makeFrontInterface(400, 200);
Content.setUseHighResolutionForPanels(true);

const pnlEnv = Content.getComponent("pnlEnv"); // Fixed size 130x42
const env = Synth.getModulator("EnvAmp"); // AHDSR module

pnlEnv.set("allowCallbacks", "Clicks, Hover & Dragging");

// Reusable path for drawing the envelope
const envPath = Content.createPath();
const stroke = { Thickness: 1.0 };

// Labels
const lblA = Content.getComponent("lblA");
const lblD = Content.getComponent("lblD");
const lblS = Content.getComponent("lblS");
const lblR = Content.getComponent("lblR");
const lbl_MAX = 50.0;

// AHDSR attributes
const IDX_A = env.getAttributeIndex("Attack");
const IDX_D = env.getAttributeIndex("Decay");
const IDX_S = env.getAttributeIndex("Sustain");
const IDX_R = env.getAttributeIndex("Release");

// Time ranges for Attack, Decay, Release
const MIN_MS = 1.0;
const MAX_MS = 2000.0;

// Sustain in dB
const SUSTAIN_DB_MIN = -100.0;
const SUSTAIN_DB_MAX = 0.0;

// Fixed pixel coords
const PANEL_W = 130.0;
const PANEL_H = 42.0;
const ENV_X0 = 7.0;
const ENV_Y0 = 7.0;
const ENV_X1 = 123.0;
const ENV_Y1 = 35.0;

// Handle min/max values
const X_ATTACK_MIN = 7.0;
const X_ATTACK_MAX = 30.0;
const X_DECAY_SPAN = 30.0;
const X_DECAY_MAX = 60.0;
const X_SUSTAIN = 90.0;
const X_RELEASE_MIN = 90.0;
const X_RELEASE_MAX = 120.0;

const BALL = 5.0; // Handle radius
const BALL_2 = BALL * 2; // Handle doubled

//! HELPERS

function clamp(x, a, b) { if (x < a) return a else if (x > b) return b else return x; }

// Normalize panel coords (0..1) for Path
inline function nx(x) { return x / PANEL_W; }
inline function ny(y) { return y / PANEL_H; }

inline function primeUnitBounds(p)
{
	p.clear();
	p.startNewSubPath(0.0, 0.0);
	p.startNewSubPath(1.0, 1.0);
}

inline function pathLineAbs(p, x1, y1, x2, y2)
{
	p.startNewSubPath(nx(x1), ny(y1));
	p.lineTo(nx(x2), ny(y2));
}

// Linear time mapping for Attack, Decay, Release
inline function msToNormForDraw(ms)
{
	if (ms <= MIN_MS) return 0.0;
	return clamp((ms - MIN_MS) / (MAX_MS - MIN_MS), 0.0, 1.0);
}

inline function normToMsForModule(n)
{
	if (n <= 0.0) return MIN_MS;
	return MIN_MS + clamp(n, 0.0, 1.0) * (MAX_MS - MIN_MS);
}

// Skewed mapping for Sustain (-12 dB at mid-point)
inline function sustainDbToY(db)
{
	local d = clamp(db, SUSTAIN_DB_MIN, SUSTAIN_DB_MAX);

	if (d >= -12.0)
		local n = 0.5 + (d + 12.0) / 12.0 * 0.5;
	else
		local n = (d - SUSTAIN_DB_MIN) / (-12.0 - SUSTAIN_DB_MIN) * 0.5;

	return ENV_Y1 - (ENV_Y1 - ENV_Y0) * n;
}

inline function yToSustainDb(y)
{
	local yy = clamp(y, ENV_Y0, ENV_Y1);
	local n = (ENV_Y1 - yy) / (ENV_Y1 - ENV_Y0);

	if (n >= 0.5)
		return -12.0 + (n - 0.5) / 0.5 * 12.0;
	else
		return SUSTAIN_DB_MIN + (n / 0.5) * (-12.0 - SUSTAIN_DB_MIN);
}

//! GEOMETRY

inline function getVals()
{
	return {
		A: env.getAttribute(IDX_A),
		D: env.getAttribute(IDX_D),
		S: env.getAttribute(IDX_S),
		R: env.getAttribute(IDX_R)
	};
}

inline function computePoints()
{
	local v = getVals();

	local aNorm = msToNormForDraw(v.A);
	local attackX = X_ATTACK_MIN + aNorm * (X_ATTACK_MAX - X_ATTACK_MIN);

	local sustainY = sustainDbToY(v.S);

	local dNorm = msToNormForDraw(v.D);
	local decayMaxX = clamp(attackX + X_DECAY_SPAN, attackX, X_DECAY_MAX);
	local decayX = attackX + dNorm * (decayMaxX - attackX);

	local rNorm = msToNormForDraw(v.R);
	local releaseX = X_RELEASE_MIN + rNorm * (X_RELEASE_MAX - X_RELEASE_MIN);

	return {
		start: { x: ENV_X0, y: ENV_Y1 },
		A: { x: attackX, y: ENV_Y0 },
		D: { x: decayX, y: sustainY },
		S: { x: X_SUSTAIN, y: sustainY },
		R: { x: releaseX, y: ENV_Y1 },
		end: { x: ENV_X1, y: ENV_Y1 }
	};
}

//! HIT TESTING

inline function dist2(ax, ay, bx, by)
{
	local dx = ax - bx;
	local dy = ay - by;
	return dx*dx + dy*dy;
}

inline function hitHandle(p, x, y)
{
	local best = "";
	local bestD = BALL * BALL;
	local keys = ["S","A","D","R"];

	for (k in keys)
	{
		local h = p[k];
		local d = dist2(x, y, h.x, h.y);
		if (d <= bestD)
		{
			bestD = d;
			best = k;
		}
	}
	return best;
}

//! LABELS

inline function pad2(n)
{
	local s = "" + Math.round(n);
	return (s.length == 1) ? ("0" + s) : s;
}

inline function msToUi(ms)
{
	if (ms <= MIN_MS) return 0;
	local n = (ms - MIN_MS) / (MAX_MS - MIN_MS);
	return clamp(Math.round(clamp(n, 0.0, 1.0) * lbl_MAX), 0, lbl_MAX);
}

inline function dbToUi(db)
{
	local d = clamp(db, SUSTAIN_DB_MIN, SUSTAIN_DB_MAX);

	if (d >= -12.0)
		local n = 0.5 + (d + 12.0) / 12.0 * 0.5;
	else
		local n = (d - SUSTAIN_DB_MIN) / (-12.0 - SUSTAIN_DB_MIN) * 0.5;

	return clamp(Math.round(n * lbl_MAX), 0, lbl_MAX);
}

inline function updateLabels()
{
	if (this.data.drag.active) {
		local v = getVals();
		lblA.set("text", "A:" + pad2(msToUi(v.A)));
		lblD.set("text", "D:" + pad2(msToUi(v.D)));
		lblS.set("text", "S:" + pad2(dbToUi(v.S)));
		lblR.set("text", "R:" + pad2(msToUi(v.R)));
	} else {
		lblA.set("text", "ATT");
		lblD.set("text", "DEC");
		lblS.set("text", "SUS");
		lblR.set("text", "REL");
	}
}

//! PAINT

pnlEnv.setPaintRoutine(function(g)
{
	// Draw background and border
	g.fillAll(this.get("bgColour"));
	g.setColour(this.get("textColour"));
	g.drawRoundedRectangle([0,0,130,42], 4, 2);

	// Draw envelope path
	var p = computePoints();
	primeUnitBounds(envPath);
	pathLineAbs(envPath, p.start.x, p.start.y, p.A.x, p.A.y);
	pathLineAbs(envPath, p.A.x, p.A.y, p.D.x, p.D.y);
	pathLineAbs(envPath, p.D.x, p.D.y, p.S.x, p.S.y);
	pathLineAbs(envPath, p.S.x, p.S.y, p.R.x, p.R.y);
	pathLineAbs(envPath, p.R.x, p.R.y, p.end.x, p.end.y);
	g.setColour(this.get("itemColour"));
	g.drawPath(envPath, this.getLocalBounds(0), stroke);

	// Draw handles
	inline function drawBall(key, x, y)
	{
	    if (key == this.data.drag.which)
	        g.setColour(this.get("itemColour"));
	    else
	        g.setColour(this.get("itemColour2"));
	
	    g.fillEllipse([x - BALL, y - BALL, BALL_2, BALL_2]);
	}
	
	drawBall("A", p.A.x, p.A.y);
	drawBall("D", p.D.x, p.D.y);
	drawBall("S", p.S.x, p.S.y);
	drawBall("R", p.R.x, p.R.y);
});

//! MOUSE

pnlEnv.setMouseCallback(function(e)
{
	var p = computePoints();

	if (e.clicked)
	{
		this.data.drag.which  = hitHandle(p, e.x, e.y);
		this.data.drag.active = (this.data.drag.which != "");
	}

	if (e.drag && this.data.drag.active)
	{
		var w = this.data.drag.which;

		if (w == "A")
		{
			var x = clamp(e.x, X_ATTACK_MIN, X_ATTACK_MAX);
			var n = (x - X_ATTACK_MIN) / (X_ATTACK_MAX - X_ATTACK_MIN);
			env.setAttribute(IDX_A, normToMsForModule(n));
		}
		else if (w == "D")
		{
			var decayMaxX = clamp(p.A.x + X_DECAY_SPAN, p.A.x, X_DECAY_MAX);
			var x = clamp(e.x, p.A.x, decayMaxX);
			var n = (x - p.A.x) / (decayMaxX - p.A.x);
			env.setAttribute(IDX_D, normToMsForModule(n));
		}
		else if (w == "S")
		{
			env.setAttribute(IDX_S, yToSustainDb(e.y));
		}
		else if (w == "R")
		{	
			var x = clamp(e.x, X_RELEASE_MIN, X_RELEASE_MAX);
			var n = (x - X_RELEASE_MIN) / (X_RELEASE_MAX - X_RELEASE_MIN);
			env.setAttribute(IDX_R, normToMsForModule(n));
		}

		updateLabels();
		this.repaint();
	}

	if (e.mouseUp)
	{
		this.data.drag.active = false;
		this.data.drag.which  = "";
		updateLabels();
		this.repaint();
	}
});

//! INIT

updateLabels();
pnlEnv.data.drag = { active:false, which:"" };

Christoph Hart

Why don't you use the Flex AHDSR envelope, this is basically the same UI?

David Healey

@dannytaurus

Looks very nice, can't you achieve the same thing with the flex ahdsr and some laf?

Here are my comments:

I'd draw the labels on the panel rather than using individual label components.

@dannytaurus said in Code feedback on this custom envelope panel:

function clamp(x, a, b) { if (x < a) return a else if (x > b) return b else return x; }

Why a function and not an inline function? Doesn't Math.range do the same thing?

@dannytaurus said in Code feedback on this custom envelope panel:

if (n >= 0.5)
return -12.0 + (n - 0.5) / 0.5 * 12.0;
else
return SUSTAIN_DB_MIN + (n / 0.5) * (-12.0 - SUSTAIN_DB_MIN);

You don't need the else in statements like this. The first return will exit the function anyway.

@dannytaurus said in Code feedback on this custom envelope panel:

if (d >= -12.0)
local n = 0.5 + (d + 12.0) / 12.0 * 0.5;
else
local n = (d - SUSTAIN_DB_MIN) / (-12.0 - SUSTAIN_DB_MIN) * 0.5;

I think it's cleaner to declare n outside of the if statement rather than have two declarations for it (I know only one will actually be hit but it has more potential for error if you expand the function in the future).

@dannytaurus said in Code feedback on this custom envelope panel:

// Draw handles
inline function drawBall(key, x, y)
{

I didn't know you could declare a function within a paint routine. Somehow that feels wrong to me :) I'd declare it outside, but if it works it works.

@dannytaurus said in Code feedback on this custom envelope panel:

  var w = this.data.drag.which;

  if (w == "A")

This is a perfect place for a switch statement instead of the else ifs

dannytaurus

@Christoph-Hart I'll take a look at Flex AHDSR.

I just thought this would be a good time to investigate how to make custom panels. A lot of the stuff I want to do in the future will need custom controls, so I wanted to learn the ins and outs.

dannytaurus

@David-Healey said in Code feedback on this custom envelope panel:

I'd draw the labels on the panel rather than using individual label components.

The labels aren't set in stone yet. I tihnk eventually I'd like the user to be able to drag the label values up/down to change the values too, as well as the handles.

Why a function and not an inline function? Doesn't Math.range do the same thing?

I was getting some errors calling it when it was an inline function. I'll look at Math.range too

You don't need the else in statements like this. The first return will exit the function anyway.

Yep, good point

I think it's cleaner to declare n outside of the if statement rather than have two declarations for it

That's just me squeezing a couple of line out of the code, but yeah probably best to define outside

I didn't know you could declare a function within a paint routine. Somehow that feels wrong to me :) I'd declare it outside, but if it works it works.

Yep, new to me too but it works. But yeah, might move it outside for clarity. Especially since it's multiple lines. It was a one-liner until I added the handle highlight colour.

This is a perfect place for a switch statement instead of the else ifs

I read you shouldn't declare vars in a switch statement, so I stuck with if/else. I can move the x and n vars outside I guess

Thanks for all the tips!

Oli Ullmann

@dannytaurus
That looks great!

@Christoph-Hart I also created my own panel because I had performance issues with theFlexEnvelope FloatingTile. It caused significant lag in the user interface and the interface of my DAW (Cubase).

ustk

@David-Healey said in Code feedback on this custom envelope panel:

I didn't know you could declare a function within a paint routine. Somehow that feels wrong to me :) I'd declare it outside, but if it works it works.

I tend to do it this way nowadays, inlining as many functions as I can for paintRoutines, mouseCB... And sub functions can be used elsewhere when necessary

MainPanel.setPaintRoutine(drawMainPanel);

inline function drawMainPanel(g)
{
	subFunction1();
	subFunction2();
}


inline function subFunction1()
{
	
}

inline function subFunction2()
{
	
}

David Healey

@ustk Yes that's my approach too.

A few things in my mind when writing code:

Don't indent more than 2 levels - there are exceptions, for example when using nested loops.
Make the code readable, even if it ends up a little more verbose.
The code should be self documenting as much as possible, no need for comments
Be consistent with naming, use of brackets, etc.

Basically the stuff I put in the style guide.

I also use type-safety for all my functions now, except in a few instances where I want to allow for undefined return types.

ustk

@David-Healey all the same except for type-safety which I didn't really see the gain for me now as I am very careful on the caller side... But I understand where it can help

dannytaurus

@ustk said in Code feedback on this custom envelope panel:

I tend to do it this way nowadays, inlining as many functions as I can for paintRoutines, mouseCB...

Is this mainly for readability and reusability? Or is there a performance improvement too?

ustk

@dannytaurus inline functions tend to perform better

dannytaurus

@ustk I thought inline functions were placed inline at their call spot during compilation, hence the name? Which would end up putting them all back where they were refactored out from.

Although I now think I remember Christoph saying recently that wasn't the case?

David Healey

@ustk said in Code feedback on this custom envelope panel:

@dannytaurus inline functions tend to perform better

For paint routines and mouse callbacks I'm not sure it makes a difference since the paint routine or mouse callback is still a regular function.

@dannytaurus said in Code feedback on this custom envelope panel:

Although I now think I remember Christoph saying recently that wasn't the case?

You remember correctly, he was lying to us this whole time :p

dannytaurus

@David-Healey @ustk I'm not against the concept of refactoring stuff out to inline functions at all, it's a very common pattern in Ruby. Lots of small well-named methods can make the code a lot easier to reason about.

Also helps with the self-documenting code idea.

@David-Healey

ustk

@David-Healey said in Code feedback on this custom envelope panel:

For paint routines and mouse callbacks I'm not sure it makes a difference since the paint routine or mouse callback is still a regular function.

Yes but the way I understand it (might be wrong though) is that it's faster to execute even if the outside part isn't inlined. The inline part containing locals, the code execution inside it seems faster especially for long paint routines or mouseCB. Then what happens after this isn't affected but at least the script part might be improved.

Code feedback on this custom envelope panel

15

2.1k

13.2k

114.5k