search

flameshot-org / flameshot

Powerful yet simple to use screenshot software :desktop_computer: :camera_flash:
https://flameshot.org
GNU General Public License v3.0
24.22k stars 1.55k forks source link

[FEAT] added randomize pixels function to pixelate tool #3368

Open kurayami07734 opened 9 months ago

kurayami07734 commented 9 months ago

Fixes #3289

Concerns

Pixelate tool's look changes completely. It now looks like white noise

Changes

Results of cursory test

before

lorem before

after

lorem after

mmahmoudian commented 9 months ago

@kurayami07734 Thanks for the great work. Is it possible to:

  1. change the pixel size in accordance to the tool size that the user is using?
  2. considering that its a drastic change, can you add an item in the settings that allow the user to toggle this on/off? This is good for privacy, but it now looks like an extra opaque layer. This may cause backlash from users.
kurayami07734 commented 9 months ago

change the pixel size in accordance to the tool size that the user is using?

I did not find where exactly the pixel size is being set

considering that its a drastic change, can you add an item in the settings that allow the user to toggle this on/off? This is good for privacy, but it now looks like an extra opaque layer. This may cause backlash from users.

I tried to add a setting for this in my latest commit. I followed the example of other settings already present. I am some how getting a segfault now

QLayout: Attempting to add QLayout "" to SidePanelWidget "", which already has a layout

Edit

I've narrowed it down to QAbstractButton::setChecked(bool) I am probably not using the connect function properly

kurayami07734 commented 9 months ago

Hey guys, you probably missed this PR

veracioux commented 8 months ago

@kurayami07734 I'm reviewing your PR now. Sorry about the wait.

nxmndr commented 2 months ago

Is it not the same issue as #2439, #3323, #3242 etc ?

The current blur tool can be set at max size by scrolling and I think the effect is very satisfying. I'd rather have it than these little dots (no offense). It may not be clear enough that the scroll can be used for size though.

kurayami07734 commented 2 months ago

Is it not the same issue as #2439, #3323, #3242 etc ?

The current blur tool can be set at max size by scrolling and I think the effect is very satisfying. I'd rather have it than these little dots (no offense). It may not be clear enough that the scroll can be used for size though.

I've randomized the pixel using std::random_shuffle, so it looks like white noise a bit.

Also, can I run flameshot from wsl? I'm this error flameshot: error: Unable to connect via DBus

nxmndr commented 2 months ago

Ah my bad it will be a separate toggle/option. Cool.

AlexFolland commented 1 week ago

What's the status of reviewing this? The only conversation in the latest review looks resolved. I'm specifically referring to the need to add an initRandomizePixels call to the GeneralConf constructor, which was done. Is this ready for merging?