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

Draw flat, borderless, semi-transparent and sharp-cornered notes #2827

Merged
merged 8 commits into from
Jun 9, 2016

Conversation

Umcaruje
Copy link
Member

@Umcaruje Umcaruje commented Jun 6, 2016

Closes #2777.
screenshot from 2016-06-07 00 08 37

Flat notes with the end-point were the most popular choice, and I added transparency (adjustable by a qproperty) so the user can distinguish overlapping notes.

I also changed the selected notes color to the same color TCO's have when selected, because the current selected notes color had the same hue and saturation as the normal note color, and as such it did not affect the notes at all.
Here's a not selected and a selected note:
screenshot from 2016-06-07 00 16 22

I increased the grid color lightness, as I was having a hard time distinguishing the grid.

As always, review and comments welcome.

float PianoRoll::noteOpacity() const
{ return m_noteOpacity; }

void PianoRoll::setNoteOpacity( const float & f )
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the reference (&) needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, it isn't. Good catch.

Copy link
Member

@Spekular Spekular Jun 7, 2016 via email

Choose a reason for hiding this comment

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

Copy link
Member Author

@Umcaruje Umcaruje Jun 7, 2016

Choose a reason for hiding this comment

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

Could we maybe use note color to denote velocity?

... ? We already use the color to denote velocity, the lightness of the color gets changed. The thing is, having two identical notes on top of each other would probably sound just as loud as a note with a double velocity, and if you need to know the exact velocity, you have the velocity bars on the bottom.

@IvanMaldonado
Copy link
Contributor

IvanMaldonado commented Jun 6, 2016

Why do some notes fade into dark from side to side?

@Umcaruje
Copy link
Member Author

Umcaruje commented Jun 6, 2016

Why do some notes fade into dark from side to side?

Those are panned notes.

@Moth-Tolias
Copy link

Moth-Tolias commented Jun 7, 2016

hm, it's kind of hard to see where that one panned note at the right begins...
i'm probably just stating what's gone before, but i really think having an outline is a valuable thing worth keeping.

@Umcaruje
Copy link
Member Author

Umcaruje commented Jun 7, 2016

hm, it's kind of hard to see where that one panned note at the right begins...

True. I converted the gradient to a horizontal one (thanks for the idea @grejppi) so now you can clearly see the start/end of a note:
screenshot from 2016-06-07 16 09 46


to @musikBear's concerns from #2777, I can make the grid even lighter, if you still find it hardly distinguishable.

cc @RebeccaDeField

@Umcaruje Umcaruje changed the title Draw flat, borderless, semi-transparent and sharp-cornered notes; Increase the contrast of the grid Draw flat, borderless, semi-transparent and sharp-cornered notes Jun 7, 2016
@Umcaruje
Copy link
Member Author

Umcaruje commented Jun 7, 2016

per @liushuyu I increased the opacity to 65% for better visibility:
screenshot from 2016-06-07 16 40 23

@musikBear
Copy link

musikBear commented Jun 8, 2016

to @musikBear's concerns from #2777, I can make the grid even lighter, if you still find it hardly distinguishable.

I do find it hard to see the grid, and i know that it will be a strain on my poor eye-sight, but as long as the grid are CSS controlled, i can choose to make the gridlines all white.
So if CSS grid-drawing-controll is preserved in 1.2 with Rebeccas theme ( ?? I cant see anything regarding Piano-roll in #2770 ?) the whole default design should not change because of one half-blind user.

@Umcaruje
Copy link
Member Author

Umcaruje commented Jun 8, 2016

Ok, I'll self-merge this pr in 8 or so hours unless there are objections.

CC @tresf @RebeccaDeField @unfa

@jasp00
Copy link
Member

jasp00 commented Jun 8, 2016

How would it look like if hue depended on note speed? Short notes, red; long notes, green (red + green = constant).

@tresf
Copy link
Member

tresf commented Jun 8, 2016

I tend to prefer the ones with solid borders. Notes are like the text on a document, they must standout with the utmost significance. The 65% I feel is a bad idea unless they have solid borders. We need to make sure not to let a cosmetic decision cloud a functional one.

@IvanMaldonado
Copy link
Contributor

I also do not like where this is heading but this is just a personal opinion, I don't like how the Semi transparent notes look and I also feel borders are very important.

@Umcaruje
Copy link
Member Author

Umcaruje commented Jun 8, 2016

Ok then, should I reintroduce borders, and just have 50% transparency on the inside of the notes?

How would it look like if hue depended on note speed? Short notes, red; long notes, green (red + green = constant).

That's a cool idea, but I don't see its point and also that would mean that the color of the notes becomes hardcoded

@Umcaruje
Copy link
Member Author

Umcaruje commented Jun 8, 2016

screenshot from 2016-06-08 21 06 00

Here's the notes with the border and just the filling transparent. This allows us to see overlapping notes and not sacrifice clearness.

@grejppi
Copy link
Contributor

grejppi commented Jun 8, 2016

Honestly I like the borderless ones more. The contrast between bright edges and dark fill on the quieter notes makes them stand out more than the louder ones, which is somewhat counter-intuitive.

@BaraMGB
Copy link
Contributor

BaraMGB commented Jun 8, 2016

I have tested this branch today with borderless notes and liked it. All notes are clear to see. Those with lowest velocity, too.

@Umcaruje
Copy link
Member Author

Umcaruje commented Jun 8, 2016

I too agree with @grejppi and @BaraMGB, I prefer the flat notes and find them giving me less eye-strain than the ones with the borders

@tresf
Copy link
Member

tresf commented Jun 8, 2016

The contrast between bright edges and dark fill on the quieter notes makes them stand out more than the louder ones, which is somewhat counter-intuitive.

Then reverse it, but I don't necessarily agree that it makes them stand out less.

That green is a pretty obnoxious color IMO. Here's some other mockups...

image

image

image

@tresf
Copy link
Member

tresf commented Jun 8, 2016

Also, I'd like to add that this can be merged any time. I'm not trying to hold up any progress, just stating my opinion since I was @mentioned.

@IvanMaldonado
Copy link
Contributor

IvanMaldonado commented Jun 8, 2016

I seriously believe we should find a colorblind solution for this, that may be easy for you to see but for people like me, colors are a bit messy and those notes confuse me a lot.

Edit: Sorry for not being @mentioned.

@IvanMaldonado
Copy link
Contributor

Let me explain my situation:

green

Notes linked by the white line look same to me.

You can also tell that:
note 'a' ends in point '1'
note 'b' ends in point '2'
note 'c' ends in point '3'

But since note 'b' and 'c' look the same color to me in the two sections pointed I could believe that 'b' ends in '3', I have to look at the pattern of the notes to conclude which note is which and that won't be always as easy.

@BaraMGB
Copy link
Contributor

BaraMGB commented Jun 8, 2016

And what if the note on mouse hover changes the color or hue or something like that?

@Umcaruje
Copy link
Member Author

Umcaruje commented Jun 8, 2016

And what if the note on mouse hover change the color or hue or something like that?

When you click on a note, it changes its color. If we implemented hover change for notes it'll become insonsistent with the rest of LMMS, because the BB steps and TCOs don't react on hover.

@grejppi
Copy link
Contributor

grejppi commented Jun 8, 2016

@IvanMaldonado How often do you lay out your notes like that?

@IvanMaldonado
Copy link
Contributor

@grejppi Not often at all, but I don't believe I'm the only colorblind out there who uses lmms, It is fine for me I just think it is important to take these things in count.

About the borders, I would like to see how the notes look in this positon:

notese

@Umcaruje
Copy link
Member Author

Umcaruje commented Jun 8, 2016

Borderless:
screenshot from 2016-06-09 01 03 52 (Note: transparency is 50%, because I'm lazy)

Borderfull:
screenshot from 2016-06-09 01 04 59

@IvanMaldonado
Copy link
Contributor

Alright, fair enough... since most people here like the borderless one better, can I ask for the borders to be enabled/disabled by css?

@Umcaruje
Copy link
Member Author

Umcaruje commented Jun 8, 2016

Implemented a qproperty that makes the border width themeable. Please note it only works if you input values such as 1 or 0. If you make the width wider all hell breaks loose and notes have a 1px offset. I'm gonna sleep now.

@IvanMaldonado
Copy link
Contributor

@Umcaruje Thank you, good night :)

@RebeccaDeField
Copy link
Contributor

RebeccaDeField commented Jun 9, 2016

Just reading over this. Looks like there are a lot of great solutions here. 👍

My 2 cents: from reading over the comments, it looks to me that we have two options. Either "borderless" with the opacity at a level that works for LMMS users or "borderfull" with slightly less bright borders.

Because the Song Editor is a very important part of the program I personally feel like having the color match the primary LMMS color is important, but if it had to be changed I would like the Instrument Pattern to match new color in the Song Editor.

@Umcaruje
Copy link
Member Author

Umcaruje commented Jun 9, 2016

Ok, so here's what I implemented. We have a boolean qproperty borderlessNotes that enables the user to choose if they want to have borderless notes. I think that the default look should be borderless notes with 65% transparency (alpha = 165). The classic theme has notes with borders and 50% transparency.

Here is the borderless version of notes, with different velocities, different pan and overlapping + stacked notes:
screenshot from 2016-06-09 13 28 49
screenshot from 2016-06-09 13 30 43
screenshot from 2016-06-09 13 31 43

Here are the same notes with borders, styled like the classic theme, but I did the changes in the default theme because I'm simply lazy to switch to classic (The grid and background are almost the same, the only different thing is the note hue, but you get how the notes look):

screenshot from 2016-06-09 13 32 33
screenshot from 2016-06-09 13 32 48
screenshot from 2016-06-09 13 33 03

@musikBear
Copy link

@tresf wrote

We need to make sure not to let a cosmetic decision cloud a functional one.

That is simply the most important that has been written!
If both borderless and borderfull options now are available through CSS, then I believe @Umcaruje has fixed this issue as good as possible 👍
One thing i saw that i need to ask: Is gridcoler depending of chosen note-color? In the 3 suggested schemes here #2827 (comment) , The gridcolor are the same as the notecolor. Is that mandatory? Are the CSS options grid-color and note-color the same? That to me is a functional mistake.

And now i did reference tresf 3 images #2827 (comment)
Try this experiment:
Size the picture so only one can be in view.
Then take the 3 pictures into view in turns, and just gaze at each one for at least 2 mins.
How does that 'feel' in the eyes?
Can you feel the eye-strain differences?
Imo the eyes does significantly best with the middle-one (sea-green), and worst with the blue. Gray (last) is almost as good as sea-green (middle).
Is that also what your eyes tell you?
Colors and contrast are unbelievably important.
Being sight-challenged (bat-blind really.. :p ) I also strongly feel the need to respect other peeps challenges. The color-blindness issue should definitely not be regarded as trivial. CSS is also needed there.
I have NO protest against a default factory theme that is stunning, but not useable for bats (me) or color-blinds IF a simple CSS edit can fix that. Just as Umcaruje now have done with borders. Brilliant work! This 🎉 also goes out to all that has implemented the extended CSS abilities 🍰

@Umcaruje
Copy link
Member Author

Umcaruje commented Jun 9, 2016

The gridcolor are the same as the notecolor. Is that mandatory? Are the CSS options grid-color and note-color the same? That to me is a functional mistake.

I believe @tresf has changed the hue in gimp or similar, so the total color of the picture was changed, as those were mockups.

Nothing is linked to anything in the CSS. All colors are independent, and you can set them however you like. If bright orange on a rainbow gradient is your thing and you like it, you can set that up.

qproperty-noteBorderRadiusX: 5;
qproperty-noteBorderRadiusY: 2;
qproperty-noteOpacity: 128;
qproperty-borderlessNotes: false; /* boolean property, set true to have borderless notes */
Copy link
Contributor

Choose a reason for hiding this comment

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

borderlessNotes is confusing as it causes a double negative. How about something like useNoteBorders with inverse logic?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe even noteBorders in order to be consistent with gradient

@Umcaruje
Copy link
Member Author

Umcaruje commented Jun 9, 2016

Travis passed, as the borders are now themeable, I'm merging. A lot of the users liked the borderless notes, myself included, so I'm making them the default. If you want to change the color of notes, please do so in another PR, as that is out of scope of this PR.

@Umcaruje Umcaruje merged commit 1abbbc2 into LMMS:master Jun 9, 2016
@musikBear
Copy link

Nothing is linked to anything in the CSS. All colors are independent

Very good! 🍊 Thanks!

sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
…S#2827)

* Draw flat, borderless, semi-transparent and sharp-cornered notes; Increase the contrast of the grid

* Convert the note gradient to a horizontal one

* Increase opacity for better visibility

* Reinstate borders, make the fill semi-transparent

* Some cosmetic touch-ups

* Make border width themeable

* Set a boolean for borderless properties

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

Successfully merging this pull request may close these issues.

10 participants