-
-
Notifications
You must be signed in to change notification settings - Fork 994
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
Redesign of EQ plugin #3030
Redesign of EQ plugin #3030
Conversation
a6e6b9f
to
40f2ac7
Compare
done
can you make a rough mockup?
I want to wait for @LocoMatt to match the design to his "Dynamics Processor" design. |
Thank you 👏
When muted reds and yellows overlap they can have a tendency to look a bit muddy, whereas combos like purple+blue, green+blue might look really well together.
Sounds good 👍 |
@BaraMGB LocoMatt probably won't be contributing anymore, so can you please proceed with the redesign. The current waveshaper redesign idea looks like this: #3097 (comment) so you could adjust the grid to that |
I think it looks a bit odd to have a separate frame around each band, at least when there's so much contrast between the bands and the background. |
@BaraMGB do we do antialising on the FFT graph? looks a bit jagged to me. Also I think you should update the faders so they reflect the new fader redesign. |
@Umcaruje Summed up everything I was going to say 😉 |
@Spekular which frame do you mean? The gray rectangle under the Knobs and faders? There are so many controls in this plugin. I think the eyes needs some guide to see which things belong together. |
@BaraMGB yeah the knobs and faders. I agree they should be outlined/separated somehow, but it feels "busy" to me to have them stand out so much from the flat background. I think if the contrast between colors in the bg and foreground were lower it might be better. That being said, nobody else has mentioned it so it could be just me. |
That looks really good imo! |
That looks good to me, too. But, @Spekular, is this a solution for your objections? |
@BaraMGB yeah, somehow I mind the borders less now. |
@BaraMGB I like @IvanMaldonado's idea as well 👍 |
Okay, then. I'll adjust the layout. |
@RebeccaDeField @Umcaruje @Spekular @IvanMaldonado opinions? |
Looks great imo :) I've got no objections. |
@BaraMGB this is looking fantastic! It does seems that for some reason, the design is using an older version of the fader leds. |
@RebeccaDeField umcaruje is working on this. ;) |
@RebeccaDeField I'm on that, going to make some fresh fader graphics for the eq. |
Ninja'd lol |
I think it's ready, finally. |
It looks amazing. I think it looked just a bit better with a gradual change on the fader leds. haven't seen it in motion though. Merge anytime. :) |
I finally figured how these fader peaks works. There were two little bugs. And for the shelving filters I choose new values so they are more useful I think. Actually they aren't useful because we have the analyzing graph. But we have the faders so they should work useful in any way. If there are no objections anymore this can be merged. |
@BaraMGB since the weird value happening in the faders are gone now, could you try switching them out to render in dB and see how it goes? I'm willing to make new graphics if you make the switch, when I finish my schoolwork. |
Oh and Just to mention it, I don't know what you exactly changed in the shelving filters, but they don't usually have a resonance, at least not in the EQ's I've used. The problem is when you accidentaly tweak the resonance it starts really getting a huge spike in the curve and I'm not sure many people could find it useful, so maybe the resonance knob's range could be reduced for the shelving filters, so there's a more fine-tuned approach? |
plugins/Eq/EqEffect.cpp
Outdated
peakBand( 0, | ||
m_eqControls.m_lowShelfFreqModel.value(), fft , samplerate ); | ||
peakBand( m_eqControls.m_lowShelfFreqModel.value() | ||
- ( m_eqControls.m_lowShelfFreqModel.value() * m_eqControls.m_lowShelfResModel.value() * 0.5 ), |
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.
Freq.value * (2 - Res.value) / 2
To avoid one call to Freq.value ;)
plugins/Eq/EqEffect.cpp
Outdated
@@ -397,7 +399,9 @@ void EqEffect::setBandPeaks(EqAnalyser *fft, int samplerate ) | |||
|
|||
m_eqControls.m_highShelfPeakL = m_eqControls.m_highShelfPeakR = | |||
peakBand( m_eqControls.m_highShelfFreqModel.value(), | |||
samplerate * 0.5 , fft, samplerate ); | |||
m_eqControls.m_highShelfFreqModel.value() | |||
+ ( m_eqControls.m_highShelfFreqModel.value() * m_eqControls.m_highShelfResModel.value() * 0.5 ), |
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.
Same here:
Freq.value * (2 + Res.value) / 2
plugins/Eq/EqCurve.cpp
Outdated
|
||
painter->drawPixmap( -12, -12, PLUGIN_NAME::getIconPixmap( "handlehover" ) ); | ||
QPixmap hover = PLUGIN_NAME::getIconPixmap( "handlehover" ); | ||
painter->drawPixmap( 0 - ( hover.width() / 2) - 1, 0 - ( hover.height() / 2 ), hover ); |
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.
painter->drawPixmap( - hover.width() / 2 - 1, - hover.height() / 2, hover );
Why not removing the 0
?
plugins/Eq/EqCurve.cpp
Outdated
painter->drawPixmap( -12, -12, circlePixmap ); | ||
// graphics for the handles | ||
loadPixmap(); | ||
painter->drawPixmap( 0 - ( m_circlePixmap.width() / 2 ) - 1 , 0 - ( m_circlePixmap.height() / 2 ), m_circlePixmap ); |
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.
painter->drawPixmap( - m_circlePixmap.width() / 2 - 1, - m_circlePixmap.height() / 2, m_circlePixmap );
Why not removing the 0
?
plugins/Eq/EqCurve.cpp
Outdated
@@ -49,15 +49,15 @@ EqHandle::EqHandle( int num, int x, int y ) : | |||
|
|||
QRectF EqHandle::boundingRect() const | |||
{ | |||
return QRectF( -11, -11, 23, 23 ); | |||
return QRectF( 0 - m_circlePixmap.width() / 2, 0 - m_circlePixmap.height() / 2, m_circlePixmap.width(), m_circlePixmap.height() ); |
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.
return QRectF( - m_circlePixmap.width() / 2, - m_circlePixmap.height() / 2, m_circlePixmap.width(), m_circlePixmap.height() );
Why not removing the 0
?
plugins/Eq/EqControlsDialog.cpp
Outdated
@@ -161,79 +131,66 @@ EqControlsDialog::EqControlsDialog( EqControls *controls ) : | |||
QObject::connect( m_parameterWidget->getBandModels( i )->active, SIGNAL( dataChanged() ), m_parameterWidget , SLOT ( updateHandle() ) ); | |||
|
|||
m_parameterWidget->changeHandle( i ); | |||
distance = distance + 44; |
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.
distance += 44;
Easier to read IMHO.
Same on line 91.
* redesign of EQ plugin * correct some symbols * set pixelPerUnitHeight to float * change textcolor of inactive handles to white * revert changes in styles.css * change grid color/adjust Fontsize of info box / change analizing color * range changed (20 - 20000 Hz) / display width increased * Update the EQ faders and remove unused files * Increase transparency of the graph * corrects the filter fader peak * makes fader peak display more useful * Implement @zapashcanon's suggestions
@Umcaruje @RebeccaDeField
suggestions?