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

Piano roll themeing issues #64

Closed
coppolaemilio opened this issue Jan 18, 2014 · 16 comments
Closed

Piano roll themeing issues #64

coppolaemilio opened this issue Jan 18, 2014 · 16 comments
Assignees
Labels
Milestone

Comments

@coppolaemilio
Copy link

  • Using the middle click should be like the "hand" to move as in FLStudio.
  • Customization of the piano roll notes (probably with some png/svg so the notes are not ugly squares.
  • The min-width of the window is too big to fit on small screens.
@ghost ghost assigned tobydox Jan 20, 2014
@Sti2nd
Copy link
Contributor

Sti2nd commented Jun 21, 2014

  • What? Why? This is not FL studio?
  • Yes to that. Can probably be done better with CSS...
  • Yes. Should have option to have two toolbars instead of one. The notepad already got this function, isn't it just copying that code? Logical to have the dropdown menus in bar number 2.

@musikBear
Copy link

so the notes are not ugly squares.

  1. :/
  2. mo - no to odly shaped notes. just makes it difficult to see
  3. yes
    (This is enhancement /glitch -not bugs)

@badosu
Copy link
Contributor

badosu commented Dec 26, 2014

I made some experimentation using rounded rectangles to try improving the look of the notes. Let me know what you think:

2014-12-26-093445_219x283_scrot

@Sti2nd
Copy link
Contributor

Sti2nd commented Dec 26, 2014

I think that looks really cool! That could fit into the default theme :)

@badosu
Copy link
Contributor

badosu commented Dec 26, 2014

@Sti2nd Unfortunately we can't really just make some adjustments on the css as each note is drawn differently. These changes are being performed on the source code (specifically here).

I am still polishing this, as there are a lot of different cases. This one is with selection:

2014-12-26-101003_213x254_scrot

As you can see, the higher/lower volumn notes are too bright/dark.

@diizy
Copy link
Contributor

diizy commented Dec 26, 2014

On 12/26/2014 01:38 PM, Amadeus Folego wrote:

So, I made some experimentation using a Rounded Rectangle trying to
improve the look of the notes.

Let me know what you think.

Looks neat, however this would be better if implemented in CSS so that
this could be customized in themes. This however would require
separating the note to its own subwidget...

@badosu
Copy link
Contributor

badosu commented Dec 26, 2014

@diizy Agreed, it would be much more elegant. However that would require more extensive changes, we can target that.

Meanwhile, this is a really simple patch: badosu@1f011a6. As soon as it looks good and covers all cases I may sent a PR, of course if you think it looks good.

@diizy
Copy link
Contributor

diizy commented Dec 26, 2014

On 12/26/2014 03:07 PM, Amadeus Folego wrote:

@diizy https://github.com/diizy Agreed, it would be much more
elegant. However that would require more extensive changes, we can
target that.

Not all that extensive... just create a new QWidget-based class named
NoteView (or something), implement the paintEvent() function for it,
make it themeable (you can look at the textFloat class for example how
to easily write themeable custom widgets - this is even easier though
since we don't need any text).

Then just replace every instance of the note drawing code with
constructing a new NoteView object - this should also be easy, since the
note drawing is already separated into its own function. Maybe gather
all the NoteViews in a container to keep track of them, so they can be
easily disposed of when needed. Or if we want to be fancy, maybe we
don't actually need to constantly redraw the notes when they're in their
own objects - we could just keep track of the note objects, and make
sure each note has its associated NoteView...

Then, the style information can be written in the CSS, and that should
be that...

Better do things right the first time, even if it takes some extra
effort... it saves us work in the long run (and you may learn more stuff
in the process ;) )

@diizy
Copy link
Contributor

diizy commented Dec 26, 2014

On 12/26/2014 03:07 PM, Amadeus Folego wrote:

Meanwhile, this is a really simple patch: badosu/lmms@1f011a6
badosu@1f011a6.
As soon as it looks good and covers all cases I may sent a PR, of
course if you think it looks good.

I mean sure, you could just do that, but then that's going to be
replaced when the actual NoteView stuff gets implemented, so it's kind
of going to waste...

@badosu
Copy link
Contributor

badosu commented Dec 26, 2014

@diizy Thanks for the heads up and the tips! I'll try and see if I can make it.

The thing is that I am a novice at C++ and QT, this is my first time using these technologies so I am a bit afraid of changing lots of stuff, I was trying to find little bugs and changes before moving on.

@diizy
Copy link
Contributor

diizy commented Dec 26, 2014

On 12/26/2014 03:29 PM, Amadeus Folego wrote:

@diizy https://github.com/diizy Thanks for the heads up and the
tips! I'll try and see if I can make it.

The thing is that I am a novice at C++ and QT, this is my first time
using these technologies so I am a bit afraid of changing lots of
stuff, I was trying to find little bugs and changes before moving on.

No worries. With Git, it doesn't really matter if you mess up - you can
always roll back to the last good state and start over.

Also I didn't know almost any C++ and had never worked with Qt when I
started contributing to LMMS. You can learn this stuff as you go... it's
the best way to learn ;)

@badosu
Copy link
Contributor

badosu commented Dec 26, 2014

@diizy If we are going to build the NoteView, it would be good if it could be very flexible regarding css.

In particular I found out that any widget that inherits QFrame is able to have a border-radius, see reference.

What do you think, is it feasible?

A custom widget may have this kind of flexibility or we can use a widget and try to override some of it's properties?

I am on #lmms on freenode so we can chat there to not pollute this issue.

@diizy
Copy link
Contributor

diizy commented Dec 26, 2014

On 12/26/2014 04:01 PM, Amadeus Folego wrote:

@diizy https://github.com/diizy If we are going to build the
NoteView, it would be good if it could support the maximum of
flexibility regarding css.

In particular I found out that any widget that inherits QFrame is able
to have a |border-radius|, see reference
http://qt-project.org/doc/qt-4.8/qframe.html#details.

What do you think, is it feasible?

You don't need to inherit QFrame for that... just look at how it's done
in TextFloat.

@badosu
Copy link
Contributor

badosu commented Dec 26, 2014

I was taking a look at the structure of the PianoRoll and it looks like everytime the pattern is changed it is redrawn all over again.

For this widget model to be efficient I thought it would be best to draw all the PianoRollNote widgets and change only their properties once they move or are resized, etc...

I am having really a hard time on this one, I already spent some 3 hours just trying to figure out how to make this work and had no progress.

For now I am just going to polish what I feel is already an improvement over the old layout and take some other important bugs as you pointed out.

@badosu
Copy link
Contributor

badosu commented Jan 7, 2015

@coppolaemilio Since the issue with the notes appearance has been improved, and the "hand" idea probably is not going to be accepted: do you think we can rename this issue title and description to be about the min-width of the piano roll window?

@coppolaemilio
Copy link
Author

I think it's better to close this issue and create a new one instead, otherwise it will be super messy. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants