classlib: Plotter: extend tick labels

[mtmccrea]

Purpose and Motivation

This follows up #5161, in response to feedback about retaining previous functionality in how the tick labels are handled in DrawGrid.

In particular this adds functionality so that you can achieve the same look as previous version of SC:

• Move and align tick axis labels
• Append a string to axis labels
• Constrain labels to the edges of the grids
• en/disable labels independently
• set the Font and Color of tick and axis labels independently

This PR is unfinished but as it's blocking the next RC, I'm posting it early so those involved can comment on what's been done so far. @dyfer @sternsc.

This required basically a complete overhaul of how the new tick/axis label spacing is calculated so that everything resizes properly. As such, I have a good amount of testing to do.

This PR is now ready for review (I'll clean up the commit history at the final stage).

Depending on the timeline of the RC, documentation won't be the priority, but I'll submit that as a separate PR later if the RC makes it out before I get to it.

Here's a few examples of the new interface to set Plot and DrawDrid properties from Plotter.

(     // default behavior plus axis labels
p = [10, 34, 1672].collect({ |f|
	101.collect{ |i| sin((i/1000) * 2pi * f) }
}).plot;
p.axisLabelY_("Amp")
.axisLabelX_((1..3).collect{ |elem| "X label " ++ elem })
)

(    // vertically compact
p = [10, 34, 1672].collect({ |f|
	101.collect{ |i| sin((i/1000) * 2pi * f) }
}).plot;
p.setGridProperties(\y,
	\labelAnchor, \right, \labelAlign, \center,
	\constrainLabelExtents, true, \labelOffset, -3 @ 0
).setGridProperties(\x,
	\labelAnchor, \leftBottom, \labelAlign, \left,
	\constrainLabelExtents, true, \labelOffset, 3 @ -1,
	\fontColor, Color.red, \labelAppendString, " sec"
).specs_([-1.5, 1.5].asSpec) // give data headroom to see inlaid y tick labels
)

(    // minimal
p = [10, 34, 1672].collect({ |f|
	101.collect{ |i| sin((i/1000) * 2pi * f) }
}).plot;
p.setProperties( \showTickLabelsX, false, \showTickLabelsY, false)
)

The three above examples produce these plots:

Note that because I opted not to add convenience methods to Plotter that would allow direct access to setting grid properties at this time, that is accomplished with the Plotter:-setGridProperties, which mimics the behavior of Plotter:-setGridProperties:-setProperties, in that it accepts a list of key-value pairs.

The advantage is that setting multiple parameters (as is likely when customizing your grid) is more compact than multiple method calls to individual parameters for each axis. The downside is the user doesn't get the benefit of auto-complete for hints about to the available parameters. So I'll add a section to the docs in the future that will list the grid and plot parameters.

Types of changes

• Documentation
• Bug fix
• New feature

To-do list

• Code is tested
• All tests are passing
• Updated documentation (More to follow this PR)
• This PR is ready for review

[dyfer]

Thanks @mtmccrea ! I have some cosmetic comments/questions. I'm dismissing my comments for now :)

[telephon]

I think it is completely fine to have a discussion like this before a final version.

[mtmccrea]

OK, I would consider this "review-ready" at this point 😆.

I've updated the topic post with some details and examples. As mentioned there, I won't get to complete docs before RC2, but I think the interface is stable at this point (pending any feedback), so could go into the RC.

And I plan to do a hefty cleanup of the commit history once the discussion concludes here.

I've resolved a number of the Conversations above, but left some open in case you'd like to respond, otherwise feel free to Resolve them if you're satisfied.

Thanks!

[sternsc]

Thanks everyone for the efforts you are putting in on this! I’m just a bit worried that you may be overdoing it. As far as I am concerned, the .labelOffset functionality in SC 3.12 was good enough, it just wasn’t documented - and then it broke in 3.13 RC 1. From: Marcin Pączkowski ***@***.***> Sent: Wednesday, 4 January 2023 11:55 To: supercollider/supercollider ***@***.***> Cc: sternsc ***@***.***>; Mention ***@***.***> Subject: Re: [supercollider/supercollider] classlib: Plotter: extend tick labels (PR #5942) @dyfer commented on this pull request.

________________________________ In SCClassLibrary/Common/GUI/PlusGUI/Math/PlotView.sc<#5942 (comment)>:

- gridRect = viewRect.insetBy(hinset, vinset) + [hshift, vshift, 0, 0];

- drawGrid.bounds = gridRect; + // First calculate vertical space needed for tick and axis labels + // to determine if X labels will be hidden based on the view height. + // (If so, Y labels that overhang vertically may need to also be hidden). + + tkBHang = max(xTkBHang, yTkBHang); + + // if there's label overhang, give it its margin + tkBMargin = if(tkBHang > 0) { labelMargin } { 0 }; + tkTMargin = if(yTkTHang > 0) { labelMargin } { 0 }; + + // only the largest margin of all elements is needed + bottomMargin = maxItem([xAxMargin, borderMargin, tkBMargin]); + + // add up all elements extending below the grid My preference is to not include any manual non-standard formatting. I acknowledge that it looks better, but I believe even more strongly that (almost?) everything should be formatted automatically. OTOH, the automatic liner<https://github.com/supercollider/supercollider/actions/runs/3832847520/jobs/6523627143#step:3:59> for the class library files (which is set to not cause CI to fail since some existing .sc files don't conform) does not seem to catch this as an issue. So... maybe this is acceptable? — Reply to this email directly, view it on GitHub<#5942 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAXQZNAOWQSYINC6FPLCE4DWQVJHFANCNFSM6AAAAAATI7OX5A>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[mtmccrea]

@sternsc
This PR can be considered as an extension of the previous one which added much more functionality than a modification of labelOffset. Because you noticed there was this breaking change, I wanted to make sure the old functionality was still replicable. But again, that's only a small part of the additions.

So just as a heads up, your code will still likely need to be updated. Here's an example that recreates some of the things you missed about the previous version (in particular labelOffset, and strings appended to tick labels):

// ~ Old Plotter replicated functionality: x labels at top, with strings appended
(
var gridHeight, freq = 22;
p = 101.collect({ |i| sin((i/1000)*2*pi* freq)}).plot;
gridHeight = p.plots[0].plotBounds.height;

p.setGridProperties(\x,
	\labelAnchor, \topLeft,
	\labelAlign, \left,
	\labelOffset, 8 @ gridHeight.neg,
	\labelAppendString, " xxx",
	\constrainLabelExtents, true,
).setGridProperties(\y,
	\labelAnchor, \bottomLeft,
	\labelAlign, \left,
	\labelOffset, 3 @ -3
);
)

If you plan for your views to dynamically resize, you can make sure extreme tick label offsets stay in place this way:

(
// Because use the view height to set the x tick label offset, update label offset on resize.
p.interactionView.onResize_({ |v|
	var gridHeight = p.plots[0].plotBounds.height;
	p.setGridProperties(\x, \labelOffset, 8 @ (gridHeight.neg+10))
});
)

[mtmccrea]

I've cleaned up the commit history.

[telephon]

I've only found one typo.

All the rest is good, I think. You made a TODO-note in the code, it would be good to resolve further issues, but I think these are minor things. Thanks a lot!

[telephon]

drid -> grid

[telephon]

(you can leave out the comment, it is redundant)

[mtmccrea]

Thanks for catching this. Removed.

[mtmccrea]

Thanks @telephon and @dyfer for your helpful reviews!

[dyfer]

Thank you @mtmccrea for your work on this and @telephon for the review.
@sternsc could you confirm that the changes here fix the issues you've encountered?

[sternsc]

Thanks to all! Unfortunately I won’t have time soon to try the new code – I don’t have a build environment for all of SC set up and ready to go. Would it work to get only the relevant .sc files (Plotter, GridLines etc) from the latest version, and replace the ones in 3.12.2, just to try? If so, which files exactly? From: Marcin Pączkowski ***@***.***> Sent: Tuesday, 10 January 2023 22:28 To: supercollider/supercollider ***@***.***> Cc: sternsc ***@***.***>; Mention ***@***.***> Subject: Re: [supercollider/supercollider] classlib: Plotter: extend tick labels (PR #5942) Thank you @mtmccrea<https://github.com/mtmccrea> for your work on this and @telephon<https://github.com/telephon> for the review. @sternsc<https://github.com/sternsc> could you confirm that the changes here fix the issues you've encountered? — Reply to this email directly, view it on GitHub<#5942 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAXQZNDBRHIFAIR4NKXFJUDWRXH5DANCNFSM6AAAAAATI7OX5A>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[dyfer]

Unfortunately I won’t have time soon to try the new code – I don’t have a build environment for all of SC set up and ready to go.

For macOS and Windows you can download the builds from GitHub actions (see "details" next to the CI checks, or the "Checks" tab on top, then click "Summary" and find artifacts on the bottom right, after scrolling down).

Would it work to get only the relevant .sc files (Plotter, GridLines etc) from the latest version, and replace the ones in 3.12.2, just to try? If so, which files exactly?

If might. I'd try all the .sc files changed in this PR (see "Files changed" on top) and additionally the ones which were changed in #5161 (but of course taken from the branch in this PR).

[telephon]

the first thing you can try to just copy the whole class library over. If there weren't any changes to kernel classes and primitives, it might just work. If it doesn't, you could try copy the GUI classes.

[dyfer]

the first thing you can try to just copy the whole class library over.

I didn't suggest this since AFAIR there were some changes to the interpreter and when I tried using class library from a recent develop on the SC 3.12 build it didn't work. But yes, this would typically be the first thing to try.

[sternsc]

OK, thanks. I won’t have time to do this soon, I have confidence in the changes made and in the process, and I don’t want to hold up the next RC release, so don’t wait for me. From: Marcin Pączkowski ***@***.***> Sent: Wednesday, 11 January 2023 14:00 To: supercollider/supercollider ***@***.***> Cc: sternsc ***@***.***>; Mention ***@***.***> Subject: Re: [supercollider/supercollider] classlib: Plotter: extend tick labels (PR #5942) the first thing you can try to just copy the whole class library over. I didn't suggest this since AFAIR there were some changes to the interpreter and when I tried using class library from a recent develop on the SC 3.12 build it didn't work. But yes, this would typically be the first thing to try. — Reply to this email directly, view it on GitHub<#5942 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAXQZNFFPPBYUX7YTZXIFCDWR2VEXANCNFSM6AAAAAATI7OX5A>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[mtmccrea]

@sternsc here is code that reproduces the grids you showed in your previous post.

Example Code

```
( // Example 1
	var win = Window("DrawGrid test",
		Size(350, 300).asRect.center_(Window.screenBounds.center)
	).front;
	var uview = UserView(win, win.bounds.size.asRect);
	var xspec = [30, 95].asSpec;  // MIDI
	var yspec = [45, 120].asSpec; // dB
	var inset = 2; // border around the grid
	
	d = DrawGrid(uview.bounds.insetBy(inset),
		xspec.grid, yspec.grid
	);
	
	uview.drawFunc_({ |uv| d.draw; })
	.onResize_({ |uv|
		d.bounds = uv.bounds
		.insetBy(inset)
		.moveTo(inset, inset);
	})
	.resize_(5)
	.background_(Color.white)
	;
	
	d.x
	.labelAnchor_(\bottomLeft)
	.labelAlign_(\left)
	.labelOffset_(3 @ -3)
	.labelAppendString_(" MIDI")
	.gridColor_(Color.green)
	.fontColor_(Color.green)
	.font_(Font(Font.defaultSansFace, 11))
	;
	
	d.y
	.labelAnchor_(\topLeft)
	.labelAlign_(\left)
	.labelOffset_(3 @ 3)
	.labelAppendString_(" dB")
	.gridColor_(Color.green)
	.fontColor_(Color.green)
	.font_(Font(Font.defaultSansFace, 11))
	;
)

( // Example 2
	var win = Window("DrawGrid test",
		Size(350, 200).asRect.center_(Window.screenBounds.center)
	).front;
	var uview = UserView(win, win.bounds.size.asRect);
	var xspec = [-4.8, 0].asSpec; // s
	var yspec = [0, 28].asSpec;
	var inset = 2; // border aroudn the grid
	var xtickoffset = 10;
	d = DrawGrid(uview.bounds.insetBy(inset),
		xspec.grid, yspec.grid
	);
	
	uview.drawFunc_({ |uv| d.draw; })
	.onResize_({ |uv|
		d.bounds = uv.bounds
		.insetBy(inset)
		.moveTo(inset, inset);
		d.x.labelOffset_(0@d.bounds.height.neg + xtickoffset)
	})
	.resize_(5)
	.background_(Color.black)
	;
	
	d.x
	.labelAnchor_(\topLeft)
	.labelAlign_(\left)
	.labelOffset_(0@d.bounds.height.neg + xtickoffset)
	.labelAppendString_(" s")
	.gridColor_(Color.gray)
	.fontColor_(Color.gray)
	.font_(Font(Font.defaultSansFace, 11))
	;
	
	d.y
	.labelAnchor_(\topLeft)
	.labelAlign_(\left)
	.labelOffset_(3 @ 3)
	.gridColor_(Color.gray)
	.fontColor_(Color.gray)
	.constrainLabelExtents_(false)
	.font_(Font(Font.defaultSansFace, 11))
	.numTicks_(5)
	;
)
```

Which produces:

(Including behavior for update on resize).

So I think the functionality you've described is covered here.

[sternsc]

Thanks for your kind example @mtmccrea , it looks great! Looking forward to it!

[dyfer]

Thanks so much @mtmccrea !
Sorry for looking into the details late in the conversation here.
Do I read correctly that in the process we lost automatic display of ControlSpec units? If so, I'd consider that as a regression from SC 3.12, where units were always displayed.
IMO they should still be displayed by default, but the question is whether that default should be next to the value, or along the axis.
I think it would be best to have a method to switch between these two options easily, e.g.;

DrawGridX/Y -unitDisplay(arg) where arg is \tick (appended to tick labels, 3.12 behavior), \axis (on axis label, "new" behavior), nil (not displayed). Either \tick or \axis` could be the default, depending what we all feel like.

What do you think?

Sorry for dragging this out!

[mtmccrea]

Yes spec unit auto label functionality was removed (IIRC, in response to the last PR).

Some background:

`3.12.2` default behavior:

Just plotting a stereo buffer:

(
p = (4 ** (-5..0)).plot;
p.specs = \delay;
p.domainSpecs = [0, 10, \lin, 0, 0, " xxx"].asSpec; p.refresh;
)

`3.13-rc` default behavior:

`3.13-rc` to achieve the old behavior (with new space padding):

// ... after plotting the buffer
p.axisLabelX = nil;
p.setGridProperties(\x, \labelAppendString, " " ++ p.domainSpecs.first.units)

(
p = (4 ** (-5..0)).plot;
p.specs = \delay;
p.domainSpecs = [0, 10, \lin, 0, 0, " xxx"].asSpec; p.refresh;

p.setGridProperties(\y, \labelAppendString, p.specs.first.units);
p.setGridProperties(\x, \labelAppendString, p.domainSpecs.first.units);
)

Notice Buffer:-plot still shows units by default, to align with previous behavior, but shown in the axis label.
Function:-plot does as well, because in both cases the units can be inferred confidently, unlike, e.g., Array:-plot.
I opted to put the units in the axis label because I thought having units on every tick label was redundant and crowded, and axis labels can replace that functionality. The extra vertical space taken is a consideration, so it will omit the unit label when the channel count is >= 4.

Moving forward:
So you can still achieve the same behavior in 3.13. Now the questions are

• whether spec units should be shown by default (in all cases where a spec has units, rather than just Buffer and Function plots)
• if so, whether the units should display with tick labels or as the axis label

If I'm not mistaken, @telephon authored the original approach, so may have thoughts on this.

[dyfer]

Moving forward: So you can still achieve the same behavior in 3.13. Now the questions are

Yes, but it requires extra code.

• whether spec units should be shown by default (in all cases where a spec has units, rather than just Buffer and Function plots)

I do think it should.

• if so, whether the units should display with tick labels or as the axis label

With this one I could go either way. I like the visual aspect of the "new" way, but it does take a bit more space around the plot.

[telephon]

I'm fine either way, there is much improvement.

[mtmccrea]

OK thanks, I've made the units show by default (in the axis label—easy to change this default if we hear feedback from users in the RC).

I've also added these methods to the docs.

[telephon]

good! let us know when it is ready for merge.

[mtmccrea]

It's ready on my end :)

[dyfer]

Thank you @mtmccrea and @telephon !

[sternsc]

Super, thanks for 3.13.0-rc2. I've tested the new Grid-related stuff and found the following:

1. in ExponentialGridLines.getParams there is a small bug (Grid.sc line 751 and following:
   ) { drawLabel = false // drop labels for tightly spaced upper area of the decade };
   But drawLabel must be made true again when increasing to the next decade:
   ) { drawLabel = false // drop labels for tightly spaced upper area of the decade } { drawLabel = true } ; // sternsc: bug fix: restore labels for the next decade
2. The warp property of a ControlSpec is set using a symbol (\lin or \exp), but the getter function now returns instead an object instance of a subclass of AbstractGridLines. This seems asymmetric and makes it more awkward to inquire for the type of warp. I suggests a getter method called .warper that returns the object, and keeping .warp to return the symbol. Maybe.
3. It would be great if .formatLabel could have a drawUnits argument in addition to the drawLabel argument. The reason is that it helps to plot the unit only on whole decades, as shown in this figure.

I've been able to work my way around to this result by overriding .getParams and twiddling with calls to .appendLabel, as shown by this code excerpt from the end of ExponentialGridLines.getParams:
`

		// p['labels'] = lines.collect({ arg arr, inc;
		// 	var val, drawLabel, thisLabel;
		// 	#val, drawLabel = arr;
		// 	[val, this.formatLabel(val, nfrac ? nfracarr[inc] ? 1), nil, nil, drawLabel.not];
		// });

		//// sternsc's exponential version: draw the label units only at whole decades
		//// It could be neater if .formatLabel had also a drawUnits argument
		p['labels'] = lines.collect({ arg arr, inc;
			var val, drawLabel, thisLabel;
			var saveUnits, retVal;
			#val, drawLabel = arr;
			saveUnits = this.appendLabel;
			if (val.abs.log10.frac > 0.01, { this.appendLabel_(nil) });
			retVal = [val, this.formatLabel(val, nfrac ? nfracarr[inc] ? 1), nil, nil, drawLabel.not];
			this.appendLabel_(saveUnits);
			retVal
		});
	};
	^p

`
4. The usage of the numTicks argument with exponential axes needs to be clarified. So far as I can tell, it means approximately the number of ticks per decade: 1 or 3 or 10; where three gives 1,2,5 (fine!) and anything more than 4 gives up to 10. The default behavior is a bit clunky, so I found myself changing numTicks in the view's .onResize method. This probably can't be automated because it depends on font sizes and units strings as well.

All the linear stuff works fine and I have gotten the grids plotted as I like them. Thanks a lot for your work on this!

Hope this is of some help. And I can live with overriding - you can't cater to everyone's preferences...