Add drop shadow to capture tools
- A naive and basic implementation for #313
https://github.com/user-attachments/assets/bfd97faf-8644-407c-9617-0ac3c714319e
Issues:
- [ ] slow rendering with many images
- [ ] maybe bad design or code location
I am new to Qt and C++, come up with the shadow rendering using ChatGPT, still searching for a better way to improve, suggestions welcome!
Thanks for the PR and showing interest in Flameshot.
I like the idea of configurable toggle in the sidebar. As for code change in this PR, I'm not the best person to review this, but my gut feeling tells me that 34 changed files (most of which are process -> doProcess) sounds excessive for such PR. I think this needs a little bit of clean-up, but I believe @borgmanJeremy is the better person to review this and give you feedback.
Hello I agree with @mmahmoudian that there is no reason to refactor so many files for this PR.
TBH I have not looked into the qt documentation for adding shadows, but I would recommend starting there over chatGPT. I think there probably is a more efficient way to add the shadow without all the copies (which is probably what causes the lag). The nice thing about LLM's is they are good at giving you a place to start, so now that you have a starting point its best to reference the Qt documentation rather than trying to refine with an LLM (at least in my experience). Then you will have a deeper understanding of how to fix it, rather than guess and checking chatGPT.
@mmahmoudian @borgmanJeremy Thanks for the feedbacks! I will learn and research more in the docs about how all this works to refactor this into a more sensible PR.
@jonathan-f-silva thanks for pushing this forward. I follow your commits closely and I'm grateful for the passion you are showing. I hope you are also enjoying the learning process 😊
@mmahmoudian Hi again! Thanks for the comments, it is challenging but I am enjoying figuring things bit by bit! I did a couple of changes, hopefully for the better.
09fdfc2fd3b66bffbf0e2f36e2296c5e4141993e Simplifying doProcess
-
I inverted the renaming places, now the child tool classes do not change and just changed the function calls in two places in
CaptureWidgettodoProcessto check if it needs to draw the shadow before the main draw. -
Still unsure if exists a better place/way to call
drawDropShadow.
fd363c9c775fda490e0dd734016bd4c11bc413c3 Refactoring drawDropShadow
-
I tried some methods on how to make a blurred shadow using QPixmap (some on https://stackoverflow.com/questions/3903223/qt4-how-to-blur-qpixmap-image) but still was slow, probably because the many
QPixmapandQpaintercreated for each draw. -
So in this commit I used just some repeated and translated
process()calls to draw the shadows using the main painter with no additionalQPixmapandQpainter, a little hard-coded and hacky, but works quite fast. -
Still using the
QGraphicsDropShadowEffecton the text widget, though. Not sure if it is okay/needed, or is there a better way.- On updating the branch to newest master that is using Qt6, a bug appeared on the text cursor, is not showing properly on writing the text. Still looking to understand why this is happening (I am on Windows 11 right now).
-
Excluded the shadow for the marker, invert and pixelate tools.
🎞️ Preview recorded before the master branch update
https://github.com/user-attachments/assets/a53ff1d5-0c99-4753-9ad9-b03bb9420c39
🎞️ Updated preview (with the text cursor bug)
https://github.com/user-attachments/assets/aea47dac-9b4f-4bf0-bafc-6484f0eb578f
So, what do you think? Is it going to the right direction? Thanks again for your time taking a look at this! Looking forward to your feedback!
@jonathan-f-silva Thanks for the update and the videos. They look very smooth and great to me.
but still was slow, probably because the many QPixmap and Qpainter created for each draw.
I think you are right about the reason, but I haven't tried it so my guess is as good as yours. But there can be a workaround:
You can apply the blur after the object is fully drawn. For example while an arrow is being drawn, you can show the rigid shadow (same as what you showed in the videos), and when the user releases the mouse and the arrow shape is final, you can apply the blur effect. The only time this might cause some performance issue is if the user quickly changes the size/thickness of an object and you update the blur every time they scroll. If that causes some issue, you can make it so that when the user selects an object, the shadow converts to this rigid form, and they can change the size and move it around, and when the object deselects, the blur is getting applied.
I'm sure @borgmanJeremy has a lot of experience in such optimizations and he can correct me and give better solutions.
I will try to build this PR later and see how it behaves on my computer.
- Added rendering with
QGraphicsDropShadowEffect, now usingQPixmapCacheto cache the pixmap renderings after the shape is finished. - Also noticed some rendering issues when using the text tool:
- https://github.com/flameshot-org/flameshot/issues/4062
QTextEditwithQGraphicsDropShadowEffectcaret overlap issue
🎞️ Preview so far
https://github.com/user-attachments/assets/c551ac2d-ac0b-47a7-8904-45096d47acd2
@jonathan-f-silva Thanks. It is very much improved and is not more snappy and fast.
I approved it just to trigger the CIs to run. It seems the Clang format failed. I believe the easiest way to solve this is by running:
clang-format -i $(git ls-files "*.cpp" "*.h")
@borgmanJeremy is this still the approved way? Because on my own branch this causes a lot of file change that I haven't touched.
@mmahmoudian yes thats still the process, but new clang-format versions are not apply the same changes as older ones. I think the config file needs to be updated.
I've started using distrobox to set up a small container specifically for clang-format. I'm using ubuntu 22.04, but the CI is set up for ubuntu-latest so I suspect 22.04 and 24.04 would also work fine. Once i enter the distrbox I run the following command:
find src/ -iname '*.h' -o -iname '*.cpp' | xargs clang-format -i
@mmahmoudian Thanks! I used your command and seems that changed some formatting on some other files too, like src/widgets/capture/capturetoolbutton.cpp, is this okay? If not, I can revert these changes.
It should only have formatted the lines you editted. What OS are you developing on? It may also have a version of clang-format that is too new and you may need to run it in a container. I'll open an issue to address this.
@borgmanJeremy I am using Windows 11 24H2 as my main OS. It was the newer clang-format indeed, tried on git bash, Arch WSL and the Ubuntu 24.04, but just in the Ubuntu 22.04 seems to work as intended.
EDIT: it still changed some formatting on some one-liner functions on src/tools/capturetool.h though.
I love this addition! it works perfectly for me. how hard would it be to add an "outline" as an additional option.