Skip to content

Conversation

@Nymbius
Copy link

@Nymbius Nymbius commented Sep 24, 2018

This should fix the minor GUI issues described here : #4597

This should fix the minor GUI issues described here : LMMS#4597
@tresf tresf added the gui label Sep 24, 2018
@tresf tresf added this to the 1.2.0 milestone Sep 24, 2018
@tresf tresf mentioned this pull request Sep 24, 2018
@Umcaruje
Copy link
Member

I agree with the LFO graph change, thanks for that.

Can you explain your reasoning for the CPU LED change? I don't find the original design flawed in any way.

Same goes to the master volume icon, I think it should stay at the original.

@Nymbius
Copy link
Author

Nymbius commented Oct 19, 2018

The cpu leds were "cut" on the left side meanwhile on the right they were round. My "fix" made the both sides round :
image
(I made the latters bold since they looked better to me that way. But it isnt as big of a problem as the "cut".)

As for the master volume I would want a poll to see what people think about my change.

@RebeccaDeField
Copy link
Contributor

@Nymbius Making the left side of the CPU graph flush was a stylistic choice, but I don't think making it rounded on both sides is a problem (unless @Umcaruje's opinion differs).

Bold text is also okay but what you posted is fuzzy and less clear in comparison which is caused by aliasing problems. IIRC that text was custom made and pixel perfect (path, not font) so I'm not sure what you did to get the bold effect but it would have to be edited manually with nodes and not stretched.

@PhysSong
Copy link
Member

I think we may also fix the LFO graph issue by tweaking the drawing code.

int LFO_GRAPH_W = s_lfoGraph->width() - 6; // substract border
int LFO_GRAPH_H = s_lfoGraph->height() - 6; // substract border
int graph_x_base = LFO_GRAPH_X + 3;
int graph_y_base = LFO_GRAPH_Y + 3 + LFO_GRAPH_H / 2;

A similar issue exists in the classic theme as well...

@tresf
Copy link
Member

tresf commented Jan 6, 2019

@RebeccaDeField can you please fix and merge these small changes? I feel there's a lot of dialog for a very quick change. :)

@zonkmachine
Copy link
Contributor

zonkmachine commented Jan 7, 2019

There seem to be a 1 pixel glitch on the left side of the graph too.

before
lfograph3

after
lfograph

after with this tweak of the snippet @PhysSong linked to above.

-       int LFO_GRAPH_W = s_lfoGraph->width() - 6;      // substract border
+       int LFO_GRAPH_W = s_lfoGraph->width() - 3;      // substract border
        int LFO_GRAPH_H = s_lfoGraph->height() - 6;     // substract border
-       int graph_x_base = LFO_GRAPH_X + 3;
+       int graph_x_base = LFO_GRAPH_X + 2;
        int graph_y_base = LFO_GRAPH_Y + 3 + LFO_GRAPH_H / 2;

@RebeccaDeField
Copy link
Contributor

@tresf trying to explain changes when I don't have a computer in condition to make them on tends to end with a lot of words and not so much action. ^^;

This should be a simple fix, I'll take care of it as soon as I have availability to do so.

tresf added 2 commits January 7, 2019 13:09
Per @zonkmachine's feedback, fix 1 pixel glitch on the left side of the graph
Per @zonkmachine/@Physong's feedback
@tresf
Copy link
Member

tresf commented Jan 7, 2019

I just commited the recommended changes to move this along. Please test (and if possible, merge) at your earliest convenience.

@tresf
Copy link
Member

tresf commented Jan 7, 2019

I'll take care of it as soon as I have availability to do so.

No worries, we can tackle the small stuff.

@zonkmachine
Copy link
Contributor

I just commited the recommended changes to move this along.

Then you need to also revert data/themes/default/lfo_graph.png

@tresf
Copy link
Member

tresf commented Jan 7, 2019

Then you need to also revert data/themes/default/lfo_graph.png

Done.

@zonkmachine
Copy link
Contributor

I don't see that things are moving along here fast enough that this will reach 1.2 . I suggest pushing 3db03e0 and fb2b0a4 to stable-1.2 directly and then closing this PR. You can continue the discussion of details of the gui/theme in the original issue, #4597 and take the time you need. Then issue a new PR against master instead.

tresf added a commit that referenced this pull request Jan 13, 2019
Closes #4597, supersedes #4613
@tresf
Copy link
Member

tresf commented Jan 13, 2019

I suggest pushing 3db03e0 and fb2b0a4 to stable-1.2 directly and then closing this PR.

Done. I don't necessarily disagree with @Nymbius's opinion on the volume slider, but since I'm not the creator of this theme, I'm going to patch the bug and move along. @Nymbius thanks for your contributions!

@tresf tresf closed this Jan 13, 2019
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants