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.

Sign up for GitHub

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

Jump to bottom

Add Functionality to Easily Add Images to J-Objects #388

Open
TheCedarPrince opened this issue Aug 11, 2021 · 6 comments · May be fixed by #399
Open

Add Functionality to Easily Add Images to J-Objects #388

TheCedarPrince opened this issue Aug 11, 2021 · 6 comments · May be fixed by #399
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@TheCedarPrince
Copy link
Member

TheCedarPrince commented Aug 11, 2021 •
edited
Loading

Is your feature request related to a problem? Please explain.
After some fun experiments today, I realized that we do not have a way to easily add images to J-Objects!

Describe the solution you'd like

Here is the syntax I created for a function call:

JCircle(dog_positions[i], 27; img = readpng("tiny_doggo.png"))

Then, here is what I changed in base Javis to the JCircle function:

JCircle(
    center::Point,
    radius::Real;
    color = "black",
    linewidth = 1,
    action = :stroke,
    img = nothing,
) =
    (
        args...;
        center = center,
        radius = radius,
        color = color,
        linewidth = linewidth,
        action = action,
        img = img,
    ) -> _JCircle(center, radius, color, linewidth, action, img)

and here is the internal method:

function _JCircle(center, radius, color, linewidth, action, img)
    sethue(color)
    setline(linewidth)

    if !isnothing(img)
        circle(center, radius, :stroke)
        circle(center, radius, :clip)
        placeimage(img, center, centered = true)
    else
        circle(center, radius, action)
    end

    return center
end

Describe alternatives you've considered

I considered writing an external function outside of Javis to do this, but it felt like too much work for such a small new feature addition. Then, I thought about creating a new base Javis function. That didn't make much sense as it is not much different from the base JCircle J-Object implementation. So, I added it to the base Javis JCircle definition.

What do you think @Wikunia and @Sov-trotter ? I tested out the above changes and none of the tests we had in place failed using this new functionality.
@TheCedarPrince TheCedarPrince added enhancement New feature or request good first issue Good for newcomers labels Aug 11, 2021
@Sov-trotter
Copy link
Member

Sov-trotter commented Aug 11, 2021 •
edited
Loading

Would it be better if we create a separate JImage and also a JCircle?
Putting images into JObjects(JCircle JRect etc.) will clip the images and having a JImage would simply place the full image?

Another thought unrelated to this issue:
Eg: Manim has many many MObjects. In our case we can create provide many other handy yet complex shapes using JShape in base Javis.

@Wikunia
Copy link
Member

Wikunia commented Aug 11, 2021

Yes I think that makes sense. Having the functionality for all shapes shouldn't be too complicated as well. We probably want to restructure some parts to avoid writing the same code over and over.

Creating more functionality by using JShape internally sounds like a good idea too

@TheCedarPrince
Copy link
Member Author

I like the idea of a JImage syntax. What are you thinking about the syntax looking like? I was imagining it a bit differently like this:

JImage(O, 30, star)

Where the function is defined like:

function JImage(center::Point, radius::Real, clipshape, img)
	clipshape(center, radius, :clip)
        placeimage(img, center, centered = true)
end

When you say that JImage simply places the first image, are you thinking about a syntax like this:

JImage(O, 30, JCircle)

Or what were you imagining/envisioning @Sov-trotter ?

@Sov-trotter
Copy link
Member

Sov-trotter commented Aug 12, 2021 •
edited
Loading

function JImage(center::Point, radius::Real, clipshape, img)
clipshape(center, radius, :clip)
placeimage(img, center, centered = true)
end

I will be hard to generalize JImage for every shape(in case we want to clip), as we would need to write lots of multiple dispatch, all of it doing the asme thing inside, so that code can sit within respective shapes.
So JImage should simply place the full image. The problem I see with this is that we will be having different functions to place images which can get confusing but specifying it in the docs should be good.

eg:

function JImage(pos::Point, img)
    placeimage(img, center, centered = true)
end

@Wikunia
Copy link
Member

Wikunia commented Aug 12, 2021

We might want to have a vector of those convenience methods in the future. That way we could say
[JCircle(O,50;action=:clip), JImage(filename, width, height)]

@TheCedarPrince TheCedarPrince linked a pull request Aug 14, 2021 that will close this issue
Open
7 tasks
@TheCedarPrince
Copy link
Member Author

Created a potential implementation of ideas here! Feel free to review that PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

JImage Implementation for J-Objects JuliaAnimators/Javis.jl
3 participants
@Wikunia @TheCedarPrince @Sov-trotter