Display ROI #40

jiviteshjain · 2020-07-20T19:36:33Z

Adds the `display_roi` functionality.

Adds inset_roi that enlarges and displays a single ROI inside the image itself.

Some considerations:

Does not follow the usual x (first coordinate) refers to columns and y (second coordinate) refers to rows convention, because ROIs are more likely to be interpreted in terms of pixel indices, I assumed. Hence also does not use Point to denote the diagonal points of the rectangle so as to avoid confusion. Let me know if I assumed wrong.
Natively supports drawing borders with a different type of colorant, even though draw does not, because highlighting ROIs in color on gray images is fairly common.

The second commit e71f317 adds a function signature that could support showing up to 4 different ROIs inside the image along the 4 corners, let me know if something like that would be desirable.

Example

using Images, ImageDraw, TestImages
img = testimage("cameraman")
inset_roi(img, ((170, 170), (200, 300)), 2, ImageDraw.bottom_left, RGB(0, 1, 1))

@johnnychen94

coveralls · 2020-07-20T19:40:17Z

Coverage decreased (-30.4%) to 66.486% when pulling d627648 on MLH-Fellowship:roi into 1b444e8 on JuliaImages:master.

johnnychen94

It would be more convenient to make scale, align and color optional. With the thickness feature implemented #31, we could also expose a border_width keyword.

scale: we can infer scale so that maximum(size(img) ./ size(resize(img_roi, scale)) == 2, or prod(size(img)) / prod(size(resize(img_roi, scale))) == 6 (or 8)
align: we can infer the align to be the first option that doesn't introduce overlap.
color: it depends on personal preferences, but I guess most people won't be sensitive to whatever color we make as a default value.

Hence also does not use Point to denote the diagonal points of the rectangle so as to avoid confusion. Let me know if I assumed wrong.

I was always confused that Point(x, y) represents (second_dim, first_dim) 😕 We can file an issue for further discussion on whether we should change this.

IMO, It is less confusing to make roi::Union{NTuple{2, <:AbstractUnitRange}, CartesianIndices{2}}, how do you think?

johnnychen94 · 2020-07-21T02:49:27Z

src/roi.jl

+function inset_roi(
+    img::AbstractArray{T, 2},
+    roi::NTuple{2, NTuple{2, <:Int}},
+    scale::R,
+    align::AlignOptions,
+    color::C
+) where {T<:Colorant, R<:Real, C<:Colorant}


If roi is the top-left corner of the image, and we set align = top_left, there can be some overlap here.

This won't be a big issue as people are using it in a try-and-see manner.

It would be more convenient to make align optional.

johnnychen94 · 2020-07-21T02:50:21Z

src/roi.jl

+function inset_roi(
+    img::AbstractArray{T, 2},
+    roi::NTuple{2, NTuple{2, <:Int}},
+    scale::R,


need to ensure scale is positive

jiviteshjain · 2020-07-23T11:10:56Z

inset_roi now tries to place the ROI view automatically so as to not overlap with the actual ROI. The scale is inferred as well to occupy at most 1/6th of the image's area and not exceed the image's size.

Some considerations:

Currently, scale can be auto-determined to be <1 for large ROIs. That should probably not be the case, right?
The display_roi function that was supposed to display up to 4 ROIs would become unwieldy if we have to prevent overlap between 4 ROIs and their 4 views. Because only one ROI is commonly shown inside the image (multiple can be shown outside) I suggest we drop that functionality.
(https://github.com/JuliaImages/ImageDraw.jl/pull/40/files#diff-4d5d98e77f1b9de23c3c82b0f9509dc4R125-R131)

@johnnychen94

jiviteshjain · 2020-07-23T11:18:27Z

It would be more convenient to make scale, align and color optional. With the thickness feature implemented #31, we could also expose a border_width keyword.

scale: we can infer scale so that maximum(size(img) ./ size(resize(img_roi, scale)) == 2, or prod(size(img)) / prod(size(resize(img_roi, scale))) == 6 (or 8)

align: we can infer the align to be the first option that doesn't introduce overlap.

color: it depends on personal preferences, but I guess most people won't be sensitive to whatever color we make as a default value.

These arguments are now optional. Is #31 ready to merge, or should I wait before rebasing?

I was always confused that Point(x, y) represents (second_dim, first_dim) We can file an issue for further discussion on whether we should change this.

I guess that is to keep x as the "horizontal" dimension and y as the "vertical". Can't say it is intuitive for me when thinking about arrays, but it may be in a more mathematical context.

IMO, It is less confusing to make roi::Union{NTuple{2, <:AbstractUnitRange}, CartesianIndices{2}}, how do you think?

Yes, done!

jiviteshjain · 2020-07-23T11:27:47Z

What happened to the tests? Error while compiling Images doesn't make sense. I haven't changed anything in the tests yet, and they all passed locally 😨

johnnychen94

Appropriate usage of CartesianIndex helps make codes simpler than you'd imagine, so I recommend you to take a look at this blog. And perhaps another example that I just wrote down yesterday: JuliaImages/ImageFiltering.jl#179

I've suggested some changes to your code so that you can use it as a start.

These arguments are now optional. Is #31 ready to merge, or should I wait before rebasing?

We don't need to wait for #31 given that it's inactive for a long time. Also, I don't think that code is nicely coded in Julia -- the API style. I've opened an issue #42 so that we can discuss this there.

johnnychen94 · 2020-07-23T11:27:54Z

src/roi.jl

+    if typeof(roi) <: CartesianIndices{2}
+        sx = first(roi)[1]
+        sy = first(roi)[2]
+
+        ex = last(roi)[1]
+        ey = last(roi)[2]
+    else
+        sx = first(first(roi))
+        ex = last(first(roi))
+
+        sy = first(last(roi))
+        ey = last(last(roi))
+
+    end
+
+    if sx >= ex || sy >= ey
+        throw(ArgumentError("Region of interest must be an increasing range"))
+    end
+
+    if !checkindex(Bool, axes(img, 1), sx) ||
+       !checkindex(Bool, axes(img, 1), ex) ||
+       !checkindex(Bool, axes(img, 2), sy) ||
+       !checkindex(Bool, axes(img, 2), ey)
+        throw(ArgumentError("Region of interest lies out of image"))
+    end


CartesianIndex helps handle N-D case in a generic way:

R = CartesianIndices(img) roi = CartesianIndices(roi) I_first, I_last = first(roi), last(roi) (I_first in R && I_last in R) || throw(ArgumentError("Region of interest lies out of image"))

We don't need to check sx >= ex || sy >= ey because cases like -2:-1:-5 is not a AbstractUnitRange

johnnychen94 · 2020-07-23T12:33:27Z

What happened to the tests? Error while compiling Images doesn't make sense. I haven't changed anything in the tests yet, and they all passed locally 😨

Ref: JuliaGraphics/ColorTypes.jl#209

You can temporarily disable the nightly test to continue this:

ImageDraw.jl/.github/workflows/UnitTest.yml

Line 18 in 1b444e8

julia-version: ['1.0', '1', 'nightly']

kimikage · 2020-07-24T00:06:39Z

Maybe fixed in ColorTypes v0.10.7.

ashwanirathee · 2021-03-24T04:45:03Z

I was always confused that Point(x, y) represents (second_dim, first_dim) We can file an issue for further discussion on whether we should change this.

Yeah,I had quite a lot of trouble with this too 😓 🤣 for rectangle one.I read this part just now,so we maybe need some changes in #52 and ofcourse on how the data is handled.CartesianIndex + Point,it gets eerily hard to makes sense of it..

jiviteshjain added 2 commits July 21, 2020 00:50

display ROI: add inset_roi

ae412c1

display ROI: add function signature for multiple inset rois

e71f317

jiviteshjain marked this pull request as draft July 20, 2020 19:36

johnnychen94 reviewed Jul 21, 2020

View reviewed changes

jiviteshjain added 2 commits July 23, 2020 15:34

display roi: default arguments

a52e89b

display roi: change type of roi argument

d627648

johnnychen94 reviewed Jul 23, 2020

View reviewed changes

johnnychen94 mentioned this pull request Jul 14, 2022

RectanglePoints confuses me #69

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display ROI #40

Display ROI #40

jiviteshjain commented Jul 20, 2020

coveralls commented Jul 20, 2020 •

edited

Loading

johnnychen94 left a comment

johnnychen94 Jul 21, 2020

jiviteshjain Jul 23, 2020

johnnychen94 Jul 21, 2020

jiviteshjain Jul 23, 2020

jiviteshjain commented Jul 23, 2020 •

edited

Loading

jiviteshjain commented Jul 23, 2020

jiviteshjain commented Jul 23, 2020

johnnychen94 left a comment •

edited

Loading

johnnychen94 Jul 23, 2020

johnnychen94 commented Jul 23, 2020

kimikage commented Jul 24, 2020

ashwanirathee commented Mar 24, 2021

Display ROI #40

Are you sure you want to change the base?

Display ROI #40

Conversation

jiviteshjain commented Jul 20, 2020

Adds the display_roi functionality.

Example

coveralls commented Jul 20, 2020 • edited Loading

johnnychen94 left a comment

Choose a reason for hiding this comment

johnnychen94 Jul 21, 2020

Choose a reason for hiding this comment

jiviteshjain Jul 23, 2020

Choose a reason for hiding this comment

johnnychen94 Jul 21, 2020

Choose a reason for hiding this comment

jiviteshjain Jul 23, 2020

Choose a reason for hiding this comment

jiviteshjain commented Jul 23, 2020 • edited Loading

jiviteshjain commented Jul 23, 2020

jiviteshjain commented Jul 23, 2020

johnnychen94 left a comment • edited Loading

Choose a reason for hiding this comment

johnnychen94 Jul 23, 2020

Choose a reason for hiding this comment

johnnychen94 commented Jul 23, 2020

kimikage commented Jul 24, 2020

ashwanirathee commented Mar 24, 2021

Adds the `display_roi` functionality.

coveralls commented Jul 20, 2020 •

edited

Loading

jiviteshjain commented Jul 23, 2020 •

edited

Loading

johnnychen94 left a comment •

edited

Loading