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

Clip: Use QPainter opacity #637

Closed
wants to merge 2 commits into from
Closed

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented Feb 18, 2021

Hey, I loves me a good bit-banging as much as the next guy.

(OK, that's a lie, I completely hate it and feel like, if we're fiddling with the raw, individual bits of an image, we're way too low-level and should be using better APIs or support classes.)

But I especially can't countenance openshot::Clip() manually applying alpha transparency in a for() loop, immediately before it uses QPainter to draw the resulting image... when QPainter has a .setOpacity() method!!

...So let's just use that, why don't we?

Edit: Also, I finally figured out what that one Codacy message was complaining about, with the source_image in Clip::apply_keyframes. (We were double-encapsulating the waveform QImage in a std::shared_ptr.) Fixed.

@codecov
Copy link

codecov bot commented Feb 18, 2021

Codecov Report

Merging #637 (d91a8a3) into develop (0d522e1) will increase coverage by 0.04%.
The diff coverage is 70.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #637      +/-   ##
===========================================
+ Coverage    51.85%   51.90%   +0.04%     
===========================================
  Files          151      151              
  Lines        12332    12329       -3     
===========================================
+ Hits          6395     6399       +4     
+ Misses        5937     5930       -7     
Impacted Files Coverage Δ
src/Clip.cpp 44.52% <57.14%> (+0.63%) ⬆️
src/QtImageReader.cpp 74.19% <100.00%> (+0.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d522e1...d85984f. Read the comment docs.

@github-actions
Copy link

Merge conflicts have been detected on this PR, please resolve.

@github-actions github-actions bot added the conflicts A PR with unresolved merge conflicts label Feb 25, 2021
@ferdnyc
Copy link
Contributor Author

ferdnyc commented Feb 28, 2021

This code has changed so much in the wake of #638 that I can't even begin to reconcile it.

@ferdnyc ferdnyc closed this Feb 28, 2021
@ferdnyc ferdnyc deleted the clip-opacity branch February 28, 2021 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflicts A PR with unresolved merge conflicts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant