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

Fix Mask/Transitions for Global Timeline Effects #675

Merged
merged 2 commits into from May 20, 2021

Conversation

jonoomph
Copy link
Member

Refactor of global timeline effects, to address a regression with global/timeline Mask/Transitions no longer working correctly. This was caused by an optimization that broke the general behavior of the global transitions.

…bal/timeline Mask/Transitions no longer working correctly. This was caused by an optimization that broke the general behavior of the global transitions.
@codecov
Copy link

codecov bot commented May 18, 2021

Codecov Report

Merging #675 (a505f87) into develop (6a004ed) will increase coverage by 0.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #675      +/-   ##
===========================================
+ Coverage    52.38%   52.39%   +0.01%     
===========================================
  Files          151      151              
  Lines        12345    12367      +22     
===========================================
+ Hits          6467     6480      +13     
- Misses        5878     5887       +9     
Impacted Files Coverage Δ
src/Clip.h 75.00% <ø> (ø)
src/Timeline.h 63.63% <ø> (ø)
src/Clip.cpp 42.06% <46.15%> (-0.17%) ⬇️
src/Timeline.cpp 41.70% <90.90%> (+0.09%) ⬆️
src/Settings.h 50.00% <0.00%> (ø)
src/Settings.cpp 100.00% <0.00%> (ø)
tests/Settings.cpp 100.00% <0.00%> (ø)

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 6a004ed...a505f87. Read the comment docs.

@ferdnyc
Copy link
Contributor

ferdnyc commented May 19, 2021

@jonoomph It might be a good idea to make the new GetFrame signature a protected method (and friend class the callers that need access). Right now it's ending up in the Python API, and I don't even want to think about how that works when passing a pointer to a struct as one of the arguments.

@ferdnyc
Copy link
Contributor

ferdnyc commented May 19, 2021

Huh! Well, apparently it works fine. SWIG, you so crazy.

>>> import openshot
>>> c = openshot.Clip("/var/tmp/pix_scale.mp4")
>>> c.Open()
>>> st = openshot.TimelineInfoStruct()
>>> st.timeline_frame_number = 27
>>> st.is_top_clip = True
>>> f = c.GetFrame(openshot.Frame(), 1, st)

The returned f is a perfectly valid 1px × 1px (due to the default-constructed background_frame) instance of openshot.Frame.

So, I guess it's dealer's choice whether the new GetFrame should be part of the public API, or if it's really meant for TimelineBase internal use only.

src/Timeline.cpp Outdated Show resolved Hide resolved
…foStruct, we already have this data in scope (on the background frame instance)
@jonoomph jonoomph merged commit b09416c into develop May 20, 2021
@jonoomph jonoomph changed the title WIP: Fix Mask/Transitions for Global Timeline Effects Fix Mask/Transitions for Global Timeline Effects May 20, 2021
@jonoomph jonoomph deleted the fix-transition-regression branch May 20, 2021 18:48
@jonoomph
Copy link
Member Author

Thanks for the feedback @ferdnyc! Merged!

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.

None yet

2 participants