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

[GUI] [mac] Support fast_gui on macOS #1981

Merged
merged 2 commits into from
Oct 23, 2020
Merged

[GUI] [mac] Support fast_gui on macOS #1981

merged 2 commits into from
Oct 23, 2020

Conversation

k-ye
Copy link
Member

@k-ye k-ye commented Oct 21, 2020

Sorry for the delay! Fixes #1976

Related issue = #1976

[Click here for the format server]


@codecov
Copy link

codecov bot commented Oct 21, 2020

Codecov Report

Merging #1981 into master will decrease coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1981      +/-   ##
==========================================
- Coverage   43.43%   43.41%   -0.03%     
==========================================
  Files          45       45              
  Lines        6264     6268       +4     
  Branches     1108     1109       +1     
==========================================
  Hits         2721     2721              
- Misses       3369     3373       +4     
  Partials      174      174              
Impacted Files Coverage Δ
python/taichi/lang/meta.py 60.75% <0.00%> (-3.25%) ⬇️

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 523c68a...456b7ef. Read the comment docs.

Copy link
Collaborator

@archibate archibate left a comment

Choose a reason for hiding this comment

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

🚀 LGTMig + 2 nits!

@@ -16,13 +16,24 @@ def tensor_to_ext_arr(tensor: ti.template(), arr: ti.ext_arr()):


@ti.kernel
def vector_to_fast_image(img: ti.template(), out: ti.ext_arr()):
def vector_to_fast_image(img: ti.template(), small_endian: ti.template(),
out: ti.ext_arr()):
# FIXME: Why is ``for i, j in img:`` slower than:
for i, j in ti.ndrange(*img.shape):
u, v, w = min(255, max(0, int(img[i, img.shape[1] - 1 - j] * 255)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Possible my fault, use r, g, b instead of u, v, w for clarity?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, fixed

# We use i32 for |out| since OpenGL and Metal doesn't support u8 types
# TODO: treat Cocoa and Big-endian machines, with XOR logic
out[j * img.shape[0] + i] = w + (v << 8) + (u << 16)
if ti.static(small_endian):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if ti.static(small_endian):
if ti.static(ti.get_os_name() != 'osx'):

instead of passing a template argument?
Btw, can you confirm if all OS X machines are big-endian (and also all Win and Linux X11 are little-endian)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

can you confirm if all OS X machines are big-endian

Given that the non-fast way is using big-endian and we don't hear users complain, I think this is safe. Let's just roll it out and see what happens, shall we?

@k-ye k-ye requested a review from archibate October 22, 2020 10:07
Copy link
Collaborator

@archibate archibate left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@k-ye k-ye merged commit 4cba15a into taichi-dev:master Oct 23, 2020
@k-ye k-ye deleted the fast_gui branch October 23, 2020 10:25
@yuanming-hu yuanming-hu mentioned this pull request Oct 24, 2020
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.

ti.GUI(fast_gui=True) keeps showing black screen on Mac(it functions on Windows)
2 participants