-
Notifications
You must be signed in to change notification settings - Fork 757
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
Conversation
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.
There was a problem hiding this 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
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. |
I tested this out using some sccode patches. It all works. I'm on linux as well though... |
Great thanks so much @patrickdupuis ! |
This fixes #2033, an issue regarding the
gridResolution
andgridOffset
behaviors ofSoundFileView
as well as a problem with selections not being visible.Test code
Previous behavior
gridResolution <= 1
did not work: the entire view would become the grid color.New behavior
gridResolution
to any value is now supported.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.