-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add JPEG Quality Option #3285
Add JPEG Quality Option #3285
Conversation
src/config/generalconf.cpp
Outdated
{ | ||
auto* tobox = new QHBoxLayout(); | ||
|
||
int timeout = |
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.
Is the "timeout" variable name intentional? If not, I suggest changing it to something more fitting to improve code readability
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.
It was not. I copied and pasted code from a similar configuration item to insure consistent formatting and placement and forgot to change that int. Will fix later today.
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.
The fix has been pushed.
@mdphillips375 Thanks for the PR and the time you have invested in Flameshot. Let's wait for other devs to also go through the code and approve it. Cheers. |
Looks good |
Add JPEG quality option Signed-off-by: Mike Phillips <mdphillips375@gmail.com> Revert "Add JPEG quality option" This reverts commit 046497d.
c382f7a
to
8de1e96
Compare
@mdphillips375 thanks for handling the comments. I set it to run the Actions. One question though: the range is 0-100. What does 0 and 100 mean? Is 100 without any compression? And what is 0? |
Looking through the sources to libjpeg, I see several references to 100 being lossless. It certainly has the visual appearance of being lossless. I can say that 0 is pretty bad for most things I would personally use this software for. A screenshot of text is not legible. It will turn a photograph into an image resembling a low resolution 8-bit (or lower) display, which could actually be desirable in some cases (retro gaming look). Qt5 defaults to setting the quality to 75 if you do not pass a quality parameter, which is the case in the current release, so I set the default to 75. As for a size comparison of the 100 setting, I took a screenshot test I did with Quality = 100, converted to uncompressed TIF, and PNG with maximum compression using GIMP. It is certainly not uncompressed, although not as small as PNG's lossless compression. |
@panpuchkov @veracioux do you see any issues to be discussed here or do you have any objections? |
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.
Please also merge master
into this branch, so that this PR can be tested against a CI/CD pipeline that has had some fixes applied. There is also a conflict that should be resolved.
@mdphillips375 Thanks for the changes. Please also run clang-format as described here. We require this on all PRs before merge. |
Fixed. Sorry about that. |
* fixes flameshot-org#3188 (flameshot-org#3254) Add JPEG quality option Signed-off-by: Mike Phillips <mdphillips375@gmail.com> Revert "Add JPEG quality option" This reverts commit 046497d. * Add JPEG Quality Option * Fix local variable name * JPEG Quality: Add .ini example and minor fixes. * JPEG Quality: Add .ini example and minor fixes. * Fix Formatting --------- Co-authored-by: Mehrad Mahmoudian <m.mahmoudian@gmail.com> (cherry picked from commit c97c3aa)
JPEG is a lossy compression algorithm that trades quality for file size. This update adds an option to the general options which allows the user to set the quality and passes that on in the places where there is a possibility to write an image (either to a file or buffer) in JPEG format. The changes to the calls to QPixmap::save() and QImageWriter::setQuality() are wrapped in if statements to check the format so that these changes do not impact (the probably more common) PNG format, which also uses the same parameter differently (a PNG with maximum compression is still lossless).
This change addresses issue #3216 and #506 by giving the user the choice of a higher quality, but larger file.