Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GUI: Fix SoundFileView grid behavior #2872

Merged
merged 6 commits into from
May 28, 2017
Merged

GUI: Fix SoundFileView grid behavior #2872

merged 6 commits into from
May 28, 2017

Conversation

mossheim
Copy link
Contributor

@mossheim mossheim commented May 10, 2017

This fixes #2033, an issue regarding the gridResolution and gridOffset behaviors of SoundFileView as well as a problem with selections not being visible.

Test code

(
w = Window.new("soundfile test", Rect(200, 300, 740, 100));
a = SoundFileView.new(w, Rect(20,20, 700, 60));
f = SoundFile.new;
f.openRead(Platform.resourceDir +/+ "sounds/a11wlk01.wav");

a.soundfile = f;
a.read(0, f.numFrames);
a.timeCursorOn_(true);
a.gridResolution = 2;
a.gridOffset = 0;
w.front;
)

// other resolution and offset values
a.gridResolution = 1;
a.gridResolution = 0.5;
a.gridOffset = 0.25;
a.gridOffset = 0.75;

// also try zooming with shift+mouse-drag

Previous behavior

  • The leftmost grid block was only half revealed.
  • The grid blocked the selection highlight.
  • Setting gridResolution <= 1 did not work: the entire view would become the grid color.
  • The grid resolution did not match the desired value.

New behavior

  • Leftmost grid block is fully revealed.
  • Selection highlight appears on top of grid.
  • Setting gridResolution to any value is now supported.
  • Grid resolution matches desired value.

I have also tested this to make sure that zoom and offset behavior is preserved.

Description of solution

The previous implementation had some incorrect math that I rewrote; see commit messages/code for details. I also fixed a few compiler warnings in the same file and changed the default selection color to something that I would call "better". Finally, I changed the documentation to correctly describe what gridOffset does; I noticed it was incorrect while working on this.

mossheim added 6 commits May 9, 2017 19:55
Previous implementation had several issues:
- using drawLine instead of more idiomatic drawRect
- unclear offset calculation
- only adding a single multiple of the grid resolution instead of
_gridResolution * 2, to account for the fact that the "other half"
of the grid is the previously-drawn background.

This commit fixes the "draw time grid" portion of QcWaveform::paintEvent
Move the grid-drawing block above the selection block,
so that selections are visible above the grid (see issue #2033)
construct initialization list was out of order for a few elements
The previous color (medium blue) was a little harsh against
the light pastel blue grid color. This commit changes the default
to medium grey with slight alpha, so that the grid is still visible behind
selections.
Use _beg to calculate the proper offset in the case that the view
is zoomed in.
Changes description of arg for `gridOffset_` to match actual behavior.
@mossheim mossheim added this to the 3.9 milestone May 10, 2017
@mossheim mossheim added the comp: Qt GUI sclang Qt components -- for IDE tickets, use "env: SCIDE" instead label May 10, 2017
@nhthn nhthn self-assigned this May 13, 2017
Copy link
Contributor

@nhthn nhthn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. zooming is not working for me on linux, but that's also true in master. nvm zooming is fine

@mossheim
Copy link
Contributor Author

could someone else take a look at this please? it's a really straightforward fix but there may be test cases I wasn't able to think of.

@patrickdupuis
Copy link
Contributor

I tested this out using some sccode patches. It all works. I'm on linux as well though...

@mossheim
Copy link
Contributor Author

Great thanks so much @patrickdupuis !

@mossheim mossheim merged commit 62119c3 into supercollider:master May 28, 2017
@mossheim mossheim deleted the patch/soundfileview branch May 28, 2017 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: Qt GUI sclang Qt components -- for IDE tickets, use "env: SCIDE" instead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants