-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[GUI] Add handy ti.imscale(img, w, h) for scaling image #1735
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1735 +/- ##
==========================================
- Coverage 43.45% 43.35% -0.11%
==========================================
Files 44 44
Lines 6015 6053 +38
Branches 1042 1045 +3
==========================================
+ Hits 2614 2624 +10
- Misses 3250 3275 +25
- Partials 151 154 +3
Continue to review full report at Codecov.
|
docs/gui.rst
Outdated
@@ -522,3 +522,14 @@ Image I/O | |||
This function will create an instance of ``ti.GUI`` and show the input image on the screen. | |||
|
|||
It has the same logic as ``ti.imwrite`` for different datatypes. | |||
|
|||
.. function:: ti.imscale(img, w=512, h=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. function:: ti.imscale(img, w=512, h=None): | |
.. function:: ti.imscale(img, w=512, h=512): |
I would suggest to use a meaningful number in the parameter h
, because it can guide our users better even though when h
is None
it will be the same as w
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about ti.imscale(img, w=512, h=w)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! I love that ;)
Co-authored-by: Xudong Feng <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! Given you are passing in an actual image size, I think we should use imresize
instead of imscale
. The latter hints the users to pass in ratios (instead of pixels).
u, v = (img.shape[0] - 1) / (w - 1), (img.shape[1] - 1) / (h - 1) | ||
x = np.clip(np.arange(w) * u, 0, img.shape[0] - 1).astype(np.int32) | ||
y = np.clip(np.arange(h) * v, 0, img.shape[1] - 1).astype(np.int32) | ||
return img[tuple(np.meshgrid(x, y))].swapaxes(0, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is swapaxes
necessary here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
np.meshgrid
is row-major (y, x), while our image is column major (x, y).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK sounds good. It's still very confusing to me, we should enforce correctness via a test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can now verify them by clicking the four endpoint of window of game_of_life.py
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much. Please rename all imscale
to imresize
.
Please also add a test.
u, v = (img.shape[0] - 1) / (w - 1), (img.shape[1] - 1) / (h - 1) | ||
x = np.clip(np.arange(w) * u, 0, img.shape[0] - 1).astype(np.int32) | ||
y = np.clip(np.arange(h) * v, 0, img.shape[1] - 1).astype(np.int32) | ||
return img[tuple(np.meshgrid(x, y))].swapaxes(0, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK sounds good. It's still very confusing to me, we should enforce correctness via a test.
Co-authored-by: Yuanming Hu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. LGTM now.
Related issue = #
[Click here for the format server]
When I was debugging MGPCG, I realized that a small resolution is fast but hardly-visible. So it would be great if I could scale that small image up.