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

Canvas creation #32

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Conversation

Codyk12
Copy link
Contributor

@Codyk12 Codyk12 commented Jul 2, 2019

Added a canvas creation function. Allows for creating images with a specified background. Currently Solid and a striped background

Copy link
Member

@timholy timholy left a comment

Choose a reason for hiding this comment

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

@Codyk12, sorry about the lack of reviews here, I for one missed this when you submitted it.

Creates an image of with the given colortype and size.
Defaults to a black solid image
"""
generatecanvas(colortype::Type, x::Int, y::Int) = zeros(colortype, (x,y))
Copy link
Member

Choose a reason for hiding this comment

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

Since the work is being done by zeros, why do we effectively have to create an alias to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I have done this a different way. We can scrap this merge request. We can stick a stripped function in when needed.


Paints the entire given image a solid color
"""
function draw!(img::AbstractArray{T,2}, b::SolidBackground) where {T<:Colorant}
Copy link
Member

Choose a reason for hiding this comment

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

Again seems redundant with standard names:

julia> A = fill(colorant"red", 2, 2)
2×2 Array{RGB{N0f8},2} with eltype RGB{Normed{UInt8,8}}:
 RGB{N0f8}(1.0,0.0,0.0)  RGB{N0f8}(1.0,0.0,0.0)
 RGB{N0f8}(1.0,0.0,0.0)  RGB{N0f8}(1.0,0.0,0.0)

julia> fill!(A, colorant"blue")
2×2 Array{RGB{N0f8},2} with eltype RGB{Normed{UInt8,8}}:
 RGB{N0f8}(0.0,0.0,1.0)  RGB{N0f8}(0.0,0.0,1.0)
 RGB{N0f8}(0.0,0.0,1.0)  RGB{N0f8}(0.0,0.0,1.0)


Layers colors onto a given image to set a "background"
"""
function draw!(img::AbstractArray{T,2}, b::StripedBackground) where {T<:Colorant}
Copy link
Member

Choose a reason for hiding this comment

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

This one is interesting and adds novel functionality. However, you seem to implicitly assume that img is an all-zeros array, in which case maybe this shouldn't take img as an argument? Should we have a function called stripes?

Copy link
Member

Choose a reason for hiding this comment

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

Also worth asking: how often do you use angles different from 0 and π/2? If your stripes are all rectilinear then there are easier implementations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was more trying to make it completely general. A stripes function might be preferable and not dealing with backgrounds. Its just fit better into the flow of the module I am making.

cnt += 1
elseif img[i,j] == c
break
elseif img[i,j] in b.colors[1:e-1]
Copy link
Member

Choose a reason for hiding this comment

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

Slow. How about an auxiliary array that indicates whether a given pixel has already been handled?

src/core.jl Show resolved Hide resolved
src/core.jl Show resolved Hide resolved
src/cross.jl Show resolved Hide resolved
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.

2 participants