Zoom control for paperstrip #29
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Create some utilities for creating menu items so there isn't so much copy and paste of the code.
It's quite large. Split it into new -functions that create the various parts of the output panel. The waveforms (tic, toc, period, debug), the paper strip chart and its controls, and the window under the output display that contains them, each get their own function. There is some minimal support for a vertical vs horizontal paperstrip chart, but it's not used yet.
This lets the size of the paperstrip vs the waveforms be adjusted by sliding the pane between them. To get the layout right, we need to give the waveforms a minimum size. 300x150 seems about the minimum. It should look basically the same as it did before. Each snapshot has its own unique pane position. Adjusting one tab in the notebook doesn't adjust the other tabs. It might be nice if that weren't the case and they were all synced.
jpolton
reviewed
Mar 22, 2021
| @@ -18,6 +18,13 @@ | |||
|
|
|||
| #include "tg.h" | |||
|
|
|||
| // Zoom slider ranges from 1 to 100 | |||
| static const double zoom_min = 1, zoom_max = 100, zoom_mid = (zoom_min + zoom_max)/2; | |||
There was a problem hiding this comment.
My compiler didn't like the assignment of 'zoom_mid = (zoom_min + zoom_max)/2'. It didn't like a variable being used to initialise a 'const'
// Zoom slider ranges from 1 to 100
static const double zoom_min = 1, zoom_max = 100;
static const double zoom_mid = (1 + 100)/2;
works but is not very nice. (Sorry I don't do C)
There was a problem hiding this comment.
What compiler is that? Gcc even in pedantic ANSI mode accepts it just fine, so it's probably an error in the compiler.
There was a problem hiding this comment.
@xyzzy42 Looks like gcc6.3.0:
from config.log:
... snip
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/x86_64-apple-darwin15.6.0/6.3.0/lto-wrapper
Target: x86_64-apple-darwin15.6.0
Configured with: ../gcc-6.3.0/configure --enable-languages=c++,fortran
Thread model: posix
gcc version 6.3.0 (GCC)
...
Here is output of the ./configure
% ./configure
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... ./install-sh -c -d
checking for gawk... no
checking for mawk... no
checking for nawk... no
checking for awk... awk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking whether gcc understands -c and -o together... yes
checking whether make supports the include directive... yes (GNU style)
checking dependency style of gcc... gcc3
checking for pthread_mutex_init in -lpthread... yes
checking for sqrt in -lm... yes
checking for pkg-config... /usr/local/bin/pkg-config
checking pkg-config is at least version 0.9.0... yes
checking for gtk+-3.0 glib-2.0... yes
checking for portaudio-2.0... yes
checking for fftw3f... yes
checking for windres... no
checking if gcc supports -Wl,--as-needed flag...
checking if gcc supports -Wall flag... yes
checking if gcc supports -Wextra flag... yes
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating Makefile
config.status: creating icons/Makefile
config.status: executing depfiles commands
Tg 0.5.2
=====
prefix: /usr/local
compiler: gcc
cflags: -g -O2 -Wall -Wextra
ldflags:
Happy to try stuff if you can direct.
There was a problem hiding this comment.
Looked into this more. While perfectly fine in C++, the ability to use const variables in initializer constant expressions in C is an extension that was added in gcc 8: See Why “initializer element is not a constant” is… not working anymore? Since not strictly following the rules on initializer constant expressions is not a required diagnostic, it's not required for the pedantic mode to generate an error.
jpolton
reviewed
Mar 22, 2021
| * a scale of 0.10 would be one tenth of a beat length. */ | ||
| static double get_beatscale(GtkScaleButton *b) | ||
| { | ||
| const double σ = log(scale_max/scale_min) / (zoom_max - zoom_min); |
There was a problem hiding this comment.
special character "sigma" and "omega" break my compiler, and should be replaced E.g.:
const double sigma = log(scale_max/scale_min) / (zoom_max - zoom_min);
const double omega = pow(scale_max/scale_min, -1.0/(zoom_max - zoom_min)) / scale_max;
const double zoom = gtk_scale_button_get_value(b);
const double scale = omega * exp(sigma*zoom);
jpolton
reviewed
Mar 22, 2021
|
I've got an old 2012 Macbook Air and it wasn't too much trouble to brew install gtk+, automake, etc. and build it. More trouble than Linux and brew took forever to install everything. It installed Apple clang version 12.0.0, which had no trouble with unicode in the source and extended constant expressions. I think I get the same error you are, and the details from the error dialog that OS X throws up tell me enough to fi x it. Should be fixed now. |
|
Tested with a newer gcc compiler. It all works as expected. The zoom is a nice touch. |
This allow changing the layout so the paperstrip is horizontal, scrolling to the left, with the three waveforms arraigned horizontally below the paperstrip. The effect is to make the paperstrip longer and give it more space. For many tasks the paper strip is much more useful than the waveform displays. This allows adjusting the layout to be more useful in those cases. A new checkbox in the menu is added to control this. It can be changed on the fly and the widgets will be re-arraigned. A config file option, "vertical_paperstrip", is added to remember the current setting. Default vertical layout to true if not present in config file. If multiple snapshots are present, the change of orientation will affect all of them, rather that just the current viewed one. It would be possible to change this.
Using the size of the main app window, what was done, doesn't work so well now that the space allocation between the paperstrip and waveforms can be changed. This way making the waveforms (or paperstrip) smaller will give a smaller font that fits in the smaller space better.
Create a new struct for the output_panel snapshot display parameters. Instead of being mixed into the snapshot data from the computer, there will just be a pointer to the display parameter struct in the snapshot. The clone and delete snapshot functions will clone and delete the display parameters, if they exist. The computer's snapshots don't have display parameters so the pointer will just be NULL for them. Each time a new snapshot is passed from the computer to the interface, the pointer to the display parameters can be kept so that each parameter doesn't need to be copied. paperstrip_draw_event() will allocate a new display parameter when it gets a snapshot for the first time. This will make it a lot easier to add new display parameters just by adding them to the struct. And they can be initialized in the output panel code that uses them and knows what they should be. The computer doesn't need to know anything about them. This does break the savefile format slightly since the struct field names are coded into the savefile. Maybe that design has some drawbacks when it comes to backward compatibility. In this case the trace centering parameter is the only one broken and it doesn't really matter.
Since the space allocation between the paperstrip and waveforms can be changed now, using just the main window size to control font size doesn't work as well. E.g., a small waveform in a large app window that is mostly paperstrip will still get a large font, because the app window is large. Also adjust font spacing some. Use the vertical size of the font to adjust legend in paperstrip and provide consistent margins in waveforms.
Doesn't depend on state having the widget saved or what name is used this way. Also avoids unused parameter warning.
Add a pair of buttons that overlay the paperstrip. One is a Gtk slider button which allows zooming in and out with a slider. The other will reset to the default zoom. This zoom is just zoom the chart in the horizontal direction, i.e. error in each beat. Not vertically, which would show more or fewer beats at once. The zoom ranges from the 1.0 to 0.01. E.g., for a 10 beat/sec movement, the chart's width will be at most 100 ms, an entire beat, to 10 ms, the default, down to 1 ms. The forced switch from 0.1 in normal mode to 0.01 in calibration mode is removed. If one wants to zoom in for calibration, then one is free to do that. Due to the way the paperstrip is drawn, only zoom values that are exact integer fractions will work. E.g., 0.5 = 1/2, 0.2 = 1/5, and 0.10 = 1/10, will all work. But 0.12 = 1/8.333 will not, because 8⅓ is not an integer. This makes the initial zoom jumps quite large.
Re-write most of the code for generating the paperstrip. A new algorithm is used to place the dots which has an advantage in that it can display correctly with any level of zoom, not just at integer scales. What we do is find each event's relative position from the one before it. A relative offset of 0 would mean the event is exactly a multiple of the beat time from the previous event. A positive or negative value means it is earlier or later than expected. We can then scale this offset by any value. Since only the relative position between events is used there is no longer a problem when the accumulated offset wraps around a beat length but not the chart width. With all event positions relative, we need some absolute position from which to start. This is done by saving the absolute offset of the most recent event as an anchor. The next display will place the previous anchor at the same absolute offset and update the anchor to the new most recent event. This way the dots do not shift side-to-side as they scroll. Having done this, much of the other code relating to the paperstrip becomes simpler and there is a net reduction in lines of code. The dots are placed in nearly the same positions as before, with one significant difference. The slope is reversed, i.e. a movement running fast will slope up and to the left. This works better for the math, and appears to match what other timegraphers do.
The snapshot display parameters should be initialized to NULL. Really, it would be better to always use calloc() so that every field is automatically initialized to zero/NULL rather than needing to remember to manaully do it for each field.
Check to see if the computer has figured out a rate, beat error, amplitude, etc. before drawing the rate slope lines. Otherwise the value of rate is uninitialized.
When at the default zoom scale, hide the zoom reset button. No need to have it on the screen since it won't do anything. And it tells you that the scale is already at default. I noticed the zoom reset button in Chrome disappeared when at 100% and liked this feature.
Rounding the current time down might cause the newest event to appear to be from the future. That confuses the paper strip math. Round up so that doesn't happen.
Turn a boolean vertical flag into a gtk orientation. Was tired of repeating this long expression.
Rotating the paperstrip just does the graphics, not the GUI widgets, so they weren't working as well in horizontal mode. Add some code to pack and orient them well for either vertical or horizontal layout mode.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This builds on the horizontal chart PR #28.
This adds zoom buttons that overlay the paperstrip drawing area. They allow one to smoothly zoom from displaying a full beat per row to one tenth a beat (default) to one hundred of a beat (calibrate default).
To do this, I had to pretty much re-write the paperstrip drawing code to use a different algorithm that supports smooth zooming. See commit messages for detailed information.
This also reverses the slope, so a fast movement slopes up and left instead of up and right. This was easier to code, seemed more correct to me, and, more importantly, appears to match what other timegraphers like Witschi's use.
Example, with zoom popup opened as zoom is adjusted. Notice chart width of 112.5 ms is not integral fraction of 200 ms beat length.
