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

[GSoC'21] Layers Feature #343

Merged
merged 52 commits into from
Aug 3, 2021
Merged

Conversation

Sov-trotter
Copy link
Member

@Sov-trotter Sov-trotter commented Jun 9, 2021

PR Checklist

If you are contributing to Javis.jl, please make sure you are able to check off each item on this list:

  • Did I update CHANGELOG.md with whatever changes/features I added with this PR?
  • Did I make sure to only change the part of the file where I introduced a new change/feature?
  • Did I cover all corner cases to be close to 100% test coverage (if applicable)?
  • Did I properly add Javis dependencies to the Project.toml + set an upper bound of the dependency (if applicable)?
  • Did I properly add test dependencies to the test directory (if applicable)?
  • Did I check relevant tutorials that may be affected by changes in this PR?
  • Did I clearly articulate why this PR was made the way it was and how it was made?

Link to relevant issue(s)
Closes #75

How did you address these issues with this PR? What methods did you use?
So this is a new approach towards having layers in Javis. Thanks to the ideas of @TheCedarPrince we have something that focuses on Actions rather than objects.

We realised that treating a layer as an object had certain limitations. So why not treat each layer as a drawing. This way we can apply actions on the objects in the layer and also apply actions to the layer(which is a Drawing).
One drawback of this is that since a layer is no longer an object, the actions are applied to it as a whole(ie. to the matrix of the layer's drawing). But its much better than having the layer as an object(as explored here #341 ).

So rendering is now a three step process:

for each frame
    if layers exist
         # render the layers as drawings, compute their actions and save their image matrices and settings based 
               on the computes actions
      
         # create an empty drawing(same size as the main video) and apply the actions on each layer and 
           place the layer image matrices on the empty drawing
     end

     # finally render all the independent objects on the main drawing and merge the drawing from step two with 
           the main drawing
end

test script

TODO'S :

@Sov-trotter Sov-trotter mentioned this pull request Jun 9, 2021
14 tasks
@Wikunia
Copy link
Member

Wikunia commented Jun 9, 2021

Thanks for the update @Sov-trotter
Will have a deeper look into this once you have some test cases 😉 and after work (in 2 hours)
Seems like currently only scaling and translation is supported, right?
That is probably 95% of use cases but Is it hard to support all kinds of actions though?
As an example one might want to rotate it or change the background color/opacity of the layer itself.

@Sov-trotter
Copy link
Member Author

Sov-trotter commented Jun 9, 2021

So not all actions are supported, but we can support the ones defined here https://juliagraphics.github.io/Luxor.jl/dev/images/#Transforming-images

I am planning to do rotation and opacity abit later? They are easy to implement.

@Wikunia
Copy link
Member

Wikunia commented Jun 9, 2021

Oh because each layer is kinda it's own drawing, right?
One could potentially set a background color and all that kind of stuff though in it as well, right?
But that's fine at a later stage. Anyway will check back later 😊

@Wikunia
Copy link
Member

Wikunia commented Jun 9, 2021

Can we keep the act! function name from before also for layers instead of creating a new one by just using multiple dispatch?
In a way that we keep the possibility of translating several layers together maybe?

As reference of your test script: https://gist.github.com/Sov-trotter/f8b8a7a188c4d0782e48fb2f32a9b48e#file-test_layers-jl-L95

@Sov-trotter
Copy link
Member Author

Sov-trotter commented Jun 9, 2021

Yeah sure. Also I think that this is the right time to start talking about the syntax. @TheCedarPrince I would like to hear your opinion too.
Like for the layer syntax @TheCedarPrince suggested something like,

L1 = @layer(frame, width, length) begin
        .......
        ........
end

But this will be quite difficult to parse and generalize.

A simpler macro syntax can be

L1 = @layer 1:70 500 500 Point(10, 20) begin
object1 = .....
act!(object1...)
...
...
end

They are almost similar except for the comma separators and braces.

Also I have introduced a new flag viz. PUSH_TO_LAYER which will determine if Objects are to be pushed to the video or layer. This also allows one to define a custom background for the layer. Since the ground object defined in the begin ...end block will be pushed to the layer and rendered.

Updated the test script https://gist.github.com/Sov-trotter/f8b8a7a188c4d0782e48fb2f32a9b48e

@Wikunia
Copy link
Member

Wikunia commented Jun 10, 2021

Interesting process with the macro. I like that way. We just always need to make sure that both things work such that we can move objects later into a layer which might sometimes be better for readability.
Can I use this at this stage already for a recreation of my YT video? 😉
Would love to experiment a little bit with it when writing my blog post

@Sov-trotter
Copy link
Member Author

Sov-trotter commented Jun 10, 2021

Yeah. Sure. I'll start with the tests once @TheCedarPrince gives his views on the macro syntax.

I think trying to recreate the video using layers will put the feature to a good test!
Though there are some ifs and buts, I'll try to list them here:

  • Layers for now will work only on objects that are supposed to be animated independently.
    eg: I can't use the objects defined within the layer out side with other objects eg:(in actions like anim_rotate_around). However defining actions for independent objects based on a layer should work.
orange_ball = Object(1:70, (args...)->object(O, "orange"), Point(100,0))
act!(orange_ball, Action(anim_rotate_around(2π, 0.0, layer1)))
  • Layer has it's own CRS. Eg: one can't define an object at Point(100, 100) in the layer, and expect it to appear on the global canvas at the same point. It depends on the layer size and layer positioning.
  • One needs to be careful with the positioning of independent objects and layers, since layers are placed ON the global canvas, they are always on top of other objects.
  • .....

@Wikunia
Copy link
Member

Wikunia commented Jun 10, 2021

One thing that I'm not sure about:
If I create a layer with the size 1000x1000 with a 1000x1000 video and then scale the layer down: does it actually scale the layer or does it only scale the objects in it such that the size is still 1000x1000 ?
Regarding your third point then: Will objects be visible when they are on the main canvas once the layer is scaled or not.
Hope you understand what I mean

@Sov-trotter
Copy link
Member Author

Sov-trotter commented Jun 10, 2021

If I create a layer with the size 1000x1000 with a 1000x1000 video and then scale the layer down: does it actually scale the layer or does it only scale the objects in it such that the size is still 1000x1000 ?

No it actually scales the layer down. Also the objects no longer exist and all the layer based actions are applied to the image matrix of the layer.

Will objects be visible when they are on the main canvas once the layer is scaled or not.

Yeah they will be

layer_test

@codecov
Copy link

codecov bot commented Jun 10, 2021

Codecov Report

Merging #343 (21b9cd6) into master (013fd5c) will increase coverage by 0.39%.
The diff coverage is 97.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #343      +/-   ##
==========================================
+ Coverage   95.58%   95.98%   +0.39%     
==========================================
  Files          23       27       +4     
  Lines        1201     1369     +168     
==========================================
+ Hits         1148     1314     +166     
- Misses         53       55       +2     
Impacted Files Coverage Δ
src/morphs.jl 96.46% <ø> (ø)
src/structs/Action.jl 100.00% <ø> (ø)
src/structs/Livestream.jl 100.00% <ø> (ø)
src/structs/PlutoViewer.jl 100.00% <ø> (ø)
src/layers.jl 84.37% <84.37%> (ø)
src/Javis.jl 96.34% <100.00%> (+2.45%) ⬆️
src/action_animations.jl 100.00% <100.00%> (ø)
src/javis_viewer.jl 91.20% <100.00%> (ø)
src/luxor_overrides.jl 95.83% <100.00%> (+4.92%) ⬆️
src/structs/Layer.jl 100.00% <100.00%> (ø)
... and 9 more

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 013fd5c...21b9cd6. Read the comment docs.

Copy link
Member

@Wikunia Wikunia left a comment

Choose a reason for hiding this comment

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

Just a quick review for now. Will try to find some time to really test it out. Sorry I'm a bit slow these days.

src/Javis.jl Outdated Show resolved Hide resolved
src/Javis.jl Outdated Show resolved Hide resolved
src/Javis.jl Outdated Show resolved Hide resolved
src/structs/Layer.jl Outdated Show resolved Hide resolved
src/structs/Layer.jl Outdated Show resolved Hide resolved
@Wikunia
Copy link
Member

Wikunia commented Jul 19, 2021

Make sure to add something to the changelog for this PR @Sov-trotter
I'll work on a small tutorial for layers now.

@Wikunia
Copy link
Member

Wikunia commented Jul 19, 2021

It seems like anim_rotate_around is not supported for layers? Is there a reason for that?

@Wikunia
Copy link
Member

Wikunia commented Jul 19, 2021

That's what I currently have 😂
Maybe I can add some scaling to it as well just to show the different animations one can do with layers.
Chess board example

@Sov-trotter
Copy link
Member Author

Sov-trotter commented Jul 19, 2021

It seems like anim_rotate_around is not supported for layers?

Oh shoot I forgot that. Adding it in a while. Though one thing I have realized is that we might need a better algorithm for anim_rotate_around in the future.
layer_test

If you see this gif, it also rotates the layer(the contents of the layer are also rotated), and I am not sure that's what rotating around(revolution) means. What I mean to say is that the layer contents should not rotate, just that the black layer should move around a point.
This is not a javis issue though. The fact that Luxor's rotate rotates the object too(layer in this case). We didn't notice it before since objects have solid color.

So the new algorithm, should NOT USE rotate() but simply translate the objects/layers to move around(in a circle).
So for eg: we can draw a circle around a point, get the points of a circle and then move the layer to those points sequentially.

That's what I currently have

This looks super good. I really loved the color change.

@Wikunia
Copy link
Member

Wikunia commented Jul 19, 2021

Oh so the assumption is normally more that we want to rotate the layer around a point while the layer itself should not rotate.
Which would also apply to anim_rotate_around for non round things like rectangles, right?

@Sov-trotter
Copy link
Member Author

Yeah.

@Wikunia
Copy link
Member

Wikunia commented Jul 20, 2021

Current code for the chess board:

using Colors
using Javis

function ground(args...)
    background("white") # canvas background
    sethue("black") # pen color
end

function ground_chess_board(args...)
    background("black") # canvas background
    sethue("white") # pen color
end

function chess_square(i,j,square_size, light, dark)
    translate(-4*square_size, -4*square_size)
    translate((i-1)*square_size, (j-1)*square_size)
    if (i+j) % 2 == 1
        sethue(dark)
    else 
        sethue(light)
    end
    rect(O, square_size, square_size, :fill)
end

myvideo = Video(1000, 1000)

Background(1:300, ground)

default_light = RGB(1.0, 1.0, 1.0)
default_dark = RGB(2/256, 144/256, 53/256)

new_light = RGB(0.8, 0.8, 0.8)
new_dark = RGB(204/256, 136/256, 0.0)

chess_board = @JLayer 1:300 500 500 O begin 
    Background(1:300, ground_chess_board)
    square_size = 60
    squares = [Object(1:300, (args...; light=default_light, dark=default_dark) -> chess_square(i,j,square_size, light, dark)) for i=1:8, j=1:8]
    for col in squares 
        act!(col, Action(20:80, change(:light, default_light => new_light)))
        act!(col, Action(20:80, change(:dark, default_dark => new_dark)))

        act!(col, Action(240:300, change(:light, new_light => default_light)))
        act!(col, Action(240:300, change(:dark, new_dark => default_dark)))
    end
end


act!(chess_board, Action(20:80, anim_rotate(1.0*π)))
act!(chess_board, Action(100:120, anim_translate(Point(-100, -100))))
act!(chess_board, Action(130:190, anim_rotate(-1.0*π)))
act!(chess_board, Action(200:220, anim_translate(Point(-100, -100), O)))
act!(chess_board, Action(240:300, anim_rotate(1.0*π)))

render(myvideo; pathname = "chess_board.gif")

@Sov-trotter
Copy link
Member Author

Sov-trotter commented Jul 30, 2021

This should be good to go now @TheCedarPrince @Wikunia

Copy link
Member

@Wikunia Wikunia left a comment

Choose a reason for hiding this comment

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

Oh forgot to publish my review. Sorry. Otherwise looks very solid

src/action_animations.jl Outdated Show resolved Hide resolved
src/layers.jl Outdated Show resolved Hide resolved
@TheCedarPrince TheCedarPrince self-requested a review August 3, 2021 02:46
Copy link
Member

@TheCedarPrince TheCedarPrince left a comment

Choose a reason for hiding this comment

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

@Sov-trotter and @Wikunia - this is ready for merging.

@TheCedarPrince
Copy link
Member

Just address @Wikunia 's last two points and we can merge.

@Sov-trotter
Copy link
Member Author

I can't understand what happened here? Any ideas @Wikunia

@Wikunia
Copy link
Member

Wikunia commented Aug 3, 2021

I think you understood it 😉 You don't want to return from inside of the quote

@Wikunia Wikunia merged commit 16695ed into JuliaAnimators:master Aug 3, 2021
@Wikunia
Copy link
Member

Wikunia commented Aug 3, 2021

Thanks a lot for your wonderful work @Sov-trotter
If there is anything else we want to change we shall create an issue about. 😉

@Wikunia Wikunia mentioned this pull request Aug 3, 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.

combine actions in a layer
3 participants