Skip to content
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 a tool to take an inverted screenshot #1849

Merged
merged 16 commits into from
Sep 1, 2021

Conversation

Cr4ckC4t
Copy link
Contributor

Why invert colors?

When using flameshot in dark environments on a regular basis it would be very useful to invert the colors in the capture, making the screenshot white(ish) background and dark text. Thus, enhancing the usability of those screenshots in reports and prints without having to use additional tools or workarounds.

Also, it's been a feature request for quite a while (#689).

How does it work?

Basically, I copied the savetool and made an inverttool, that inverses all pixels before saving the screenshot. The actual consists of four lines:

    QPixmap inverted = context.selectedScreenshotArea();
    QImage img = inverted.toImage();
    img.invertPixels();
    inverted.convertFromImage(img);

However, it turned out that flameshot requires to update quite a few places in order to add a tool. Without much of a documentation I tried to stay close to how the savetool is setup.

Before merging

I'd advise that someone with a way better understanding of this project looks at my modifications in order to spot potentially missing or even unnecessary code for this new tool to be integrated.

Example

See the above mentioned issue for a video demo. Here's a screenshot of the newly added button.
invert-example

Usage

After choosing the area and drawing markers, text, arrows etc. - use Ctrl+i or the Invert button to save the screenshot with inverted colors.

Notes

This isn't a perfect solution. People might want to invert colors while still drawing stuff or see a review of the inversion before saving the capture. Additionally, more filters could be added maybe (comment by @mmahmoudian). However, my Qt knowledge isn't by far good enough to be able to help with those things.

From my point of view, this Invert tool is a good solution for the time being and could still be extended at a later point.


Alright, that's it - let me know if I missed something or should update anything.

@borgmanJeremy
Copy link
Contributor

What do you think about changing it so a rectangular region is inverted instead of the entire section? I think that will allow it to be more flexible

@Cr4ckC4t
Copy link
Contributor Author

That's an interesting idea. For my workflow, saving the inverted screenshot is good enough but I can see how that would make the tool more flexible.

Currently I have no idea how to do that but I will try and see if I can implement your suggestion and report back here.

@borgmanJeremy
Copy link
Contributor

I'm happy to provide guidance if you have questions or want help! Maybe start by looking at the pixelate tool, this will be similar.

@Cr4ckC4t
Copy link
Contributor Author

Cr4ckC4t commented Aug 28, 2021

Haha, that was my thought exactly. (Thanks for the offer.)

So I came up with this:

meow.mp4

Does that match your idea?

It's basically just a stripped down version of pixelate with inversion instead of bluring.


If this is the preferred way let me know and I'll clean up the code before pushing it again.

@borgmanJeremy
Copy link
Contributor

Perfect!

@Cr4ckC4t
Copy link
Contributor Author

So I'm unsure about one thing. The pixelate tool used a scaled selection by querying the pixel-aspect ratio of the screen before copying the screen area. However, the normal rectangle tool and selection tool don't do that. I've left that out for now and used the basic selection area (point().first to point().second) which seems to work just fine. I don't know if the scaled selection maybe is necessary for different screen resolutions or if that was just for pixelating- so maybe check that (inverttool.cpp).

Other than that, I've updated the butten order of the invert tool (next to pixelate). While doing that I also noticed that the numbering tool is probably supposed to be with the other drawing tools as well, so I positioned it next to the inverter too (let me know if that's not wanted).

To match the other drawing tools I also updated the description and the shortcut (now just i).

Here's what it looks like now:
invert-example

@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" aria-hidden="true" focusable="false" role="img" width="1025" height="1024" preserveAspectRatio="xMidYMid meet" style="transform: rotate(360deg);"><rect id="backgroundrect" width="100%" height="100%" x="0" y="0" fill="none" stroke="none"/><g class="currentLayer" style=""><title>Layer 1</title><path d="M896.428 1024h-768q-53 0-90.5-37.5T.428 896V128q0-53 37.5-90.5t90.5-37.5h768q53 0 90.5 37.5t37.5 90.5v768q0 53-37.5 90.5t-90.5 37.5zm0-768q0-53-38-91l-120 121q-44-45-102.5-69.5t-123.5-24.5q-87 0-160.5 43t-116.5 116.5t-43 160.5q0 65 24.5 123.5t69.5 102.5l-121 121q38 37 91 37h512q53 0 90.5-37.5t37.5-90.5V256zm-384 576q-133 0-226-94l452-452q45 44 69.5 102.5t24.5 123.5q0 87-43 160.5T672.928 789t-160.5 43z" fill="#000000" id="svg_1" class="selected" fill-opacity="1"/></g></svg>
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you design this icon? If yes, can you add a license to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not. I don't actually remember the site I got it from.

Instead, I've seen that flameshot already uses Material Design icons so I updated the icon to this one (black and white). The current license file covers this already if I'm not mistaken.

c420842b

@@ -1303,6 +1303,19 @@ You may need to escape the &apos;#&apos; sign as in &apos;\#FFF&apos;</source>
<translation>Guarda la captura</translation>
</message>
</context>
<context>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please adjust the PR so the *.ts files are not committed. I like to create these in a dedicated commit to help with merging from the Namecheap Fork.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✅ Changes reverted in 973aef74 and de41c958.

void InvertTool::drawSearchArea(QPainter& painter, const QPixmap& pixmap)
{
Q_UNUSED(pixmap)
Q_UNUSED(painter)
Copy link
Contributor

Choose a reason for hiding this comment

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

Reference the implementation in the Pixelate tool for this. Currently since you marked Q_UNUSED(painter), this new feature does not tie into the "movable" feature. IE I cannot move the selection once placed.

If you duplicate the implementation that is in the pixelate feature that will resolve this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✅ Done in 9889eadf, although I didn't fully understand. I was able to move the screen selection without those lines too.

Regardless, it's now the same as in the pixelate tool.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, what I meant was I could not move the inverted box the way other objects are moveable. This should fix it.

@Creteil
Copy link

Creteil commented Sep 1, 2021

Hi all,

Sorry for my ignorance, but from where I can fetch this source code to have access to this latest features commits ?

@Creteil
Copy link

Creteil commented Sep 1, 2021

Look like it must appear after « change request » approval in the master branch...

True ?

If so, sorry to bogus on the question...

@Cr4ckC4t
Copy link
Contributor Author

Cr4ckC4t commented Sep 1, 2021

@Creteil you could clone my fork from here: https://github.com/Cr4ckC4t/flameshot/tree/invert (branch: invert). Building this flameshot allows you to use the invert tool but you obviously won't get the continous updates and merges from the original repository.

However, maybe this gets merged soon (thanks to @borgmanJeremy for the quick responses and reviews) - so I'd say keep an eye on the original flameshot:master.

Copy link
Contributor

@borgmanJeremy borgmanJeremy left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for the contribution!

@borgmanJeremy borgmanJeremy merged commit df20c7e into flameshot-org:master Sep 1, 2021
@Cr4ckC4t Cr4ckC4t mentioned this pull request Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants