Skip to content

Commit

Permalink
increase max of new gaussian blur to 100
Browse files Browse the repository at this point in the history
  • Loading branch information
@ddennedy
ddennedy committed Mar 6, 2022
1 parent ae7a642 commit 0bc292d
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/qml/filters/blur_gaussian/meta_av.qml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ Metadata {
gangedProperties: ['av.sigmaV']
isCurve: true
minimum: 0
maximum: 50
maximum: 100
}
]
}
Expand Down
4 changes: 2 additions & 2 deletions src/qml/filters/blur_gaussian/ui_av.qml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Shotcut.KeyframableFilter {
property string amountH: 'av.sigma'
property string amountV: 'av.sigmaV'
property double amountSliderMin: 0
property double amountSliderMax: 50
property double amountSliderMax: 100
property double amountSliderDefault: 4

keyframableParameters: [amountH, amountV]
Expand Down Expand Up @@ -94,7 +94,7 @@ Shotcut.KeyframableFilter {
maximumValue: amountSliderMax
stepSize: .1
decimals: 1
suffix: ' '
suffix: ' %'
onValueChanged: {
updateFilter(amountH, amountSlider.value, amountKeyframesButton, getPosition())
updateFilter(amountV, amountSlider.value, amountKeyframesButton, getPosition())
Expand Down

4 comments on commit 0bc292d

@bmatherly
Copy link
Member

Choose a reason for hiding this comment

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

I had avoided this because the "useful" range of the value is 1-7. Maybe someone would want to go as high as 10. But above that, everything is kind of the same. There is a risk that people will complain that the slider is not useful because there is no precision between the values people probably want (below 4).

The Frei0r implementation uses a log scale internally so that the useful range was closer to 1-50. For example, 20 in the old filter is approximately equal to 1 in the new filter. I originally implemented a log scale conversion in the qml code for the FFmpeg version. But then the values in the slider do not match the values in the keyframes graph when you only scale them in the controls UI.

I appreciate the conformity of having all the blur filters range from 0-100%. But I do not know if that is more important than having a useful range for the slider control.

@ddennedy
Copy link
Member Author

Choose a reason for hiding this comment

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

DRM pointed out to me that it does not go to 100 and get nearly as blurry like the old one. I agreed. There is a usefulness to extreme blurring to turn an image or video into a non-detailed background.

@bmatherly
Copy link
Member

Choose a reason for hiding this comment

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

OK. I do not feel strongly about it, but be aware that the scale does not work the same as the old one - even if it has the same range. Maybe I will implement a log scale in the future (the UI can still go from 0-100 if that is important)

Also, the parameter does have a unit - it is "sigma" (or the square of the standard deviation). I am thinking about this currently because the new box blur filter will have parameters that map to the underlying blur radius in pixels. So I wonder if I should scale that to some kind of percentage of image size in the underlying filter (as I did in the pillar_echo filter). I will submit a pull request and we can discuss there.

All that to say that I am not sure if we want to abstract all these blur filters to "percent" since it means different things for different filters and some users might want to know things like "what is the blur radius for this parameter setting?" or "how do I achieve a sigma of 4?".

These are just comments. I do not feel strongly about them.

@ddennedy
Copy link
Member Author

Choose a reason for hiding this comment

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

the scale does not work the same as the old one

I know.

Maybe I will implement a log scale in the future

The curves in Keyframes does not know about a non-linear function that would be in the parameters UI.

Also, the parameter does have a unit - it is "sigma"

I know, and a percentage is abstract as well but better than nothing.

the new box blur filter will have parameters that map to the underlying blur radius in pixels

Anything pixel-based is sensitive to the resolution including preview scaling and perhaps should be avoided if possible.

"how do I achieve a sigma of 4?"

What is sigma? Even I do not know, and I do not want to know. It is abstract too.

Please sign in to comment.