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

Adaptive Dark colorscheme for Stmt HTML. Ability to programmatically export conceptual stmt files. #8327

Merged
merged 4 commits into from
Jul 16, 2024

Conversation

mcourteaux
Copy link
Contributor

@mcourteaux mcourteaux commented Jun 26, 2024

Colorscheme based on gruvbox-material.

stmt-html.small.mp4

Also improved assembly syntax highlighting (while reducing one HTTP fetch, as the highlight rules are now embedded in the generated file.).

Copy link
Contributor

@steven-johnson steven-johnson left a comment

Choose a reason for hiding this comment

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

I'm not going to reject this, but let the record show that IMHO a light color scheme is preferable to a dark one; is there a way to have this say "prefer the system setting"?

/** Write out a conceptual representation of lowered code, before any parallel loop
* get factored out into separate functions, or GPU loops are offloaded to kernel code.r
* Useful for analyzing and debugging scheduling. Can emit html or plain text. */
void compile_to_conceptual_stmt(const std::string &filename,
Copy link
Contributor

Choose a reason for hiding this comment

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

This new API call should be added to the Python bindings as well

@mcourteaux
Copy link
Contributor Author

I'm not going to reject this, but let the record show that IMHO a light color scheme is preferable to a dark one; is there a way to have this say "prefer the system setting"?

It follows your system preferences. It's one of those CSS media queries. You'll see in the diff.

@mcourteaux mcourteaux changed the title Dark colorscheme for Stmt HTML. Ability to programmatically export conceptual stmt files. Adaptive Dark colorscheme for Stmt HTML. Ability to programmatically export conceptual stmt files. Jun 26, 2024
@alexreinking
Copy link
Member

It follows your system preferences. It's one of those CSS media queries. You'll see in the diff.

Can it be made into a checkbox that either enables or disables dark mode? It can use the system preference by default, but I don't want to have to change my whole system preference just to get the page to switch palates.

@mcourteaux
Copy link
Contributor Author

Can it be made into a checkbox that either enables or disables dark mode?

Fair request; working on it.

@mcourteaux
Copy link
Contributor Author

mcourteaux commented Jun 27, 2024

Updated: added a toolbar with various settings. Browser persists those settings automatically (no cookie stuff). Color theme can be set to auto, or manual modes. Auto mode also live switches with the system preferences. @alexreinking @steven-johnson See video.

stmt-html.small.mp4

@steven-johnson Added those python bindings AFAICT. Not tested that tho. Just a copy-paste-and-modify.

@alexreinking
Copy link
Member

Looks great!

@mcourteaux
Copy link
Contributor Author

@steven-johnson Did this OSX buildbot fail twice on exactly the same test? This thing doesn't seem to fail on other things? It's surprising that this PR would be related but I'm not ruling anything out here. Do you have more knowledge on this failure? Please see #8332 regarding this as well.

@steven-johnson
Copy link
Contributor

@steven-johnson Did this OSX buildbot fail twice on exactly the same test? This thing doesn't seem to fail on other things? It's surprising that this PR would be related but I'm not ruling anything out here. Do you have more knowledge on this failure? Please see #8332 regarding this as well.

Not sure, let's retry again

@mcourteaux
Copy link
Contributor Author

Failure does seem unrelated...

2024-06-28 17:09:27.935 generator_aot_gpu_multi_context_threaded[91557:5585717] Metal API Validation Enabled
Assertion failed: (buf2_result.all_equal(6)), function run_kernels_on_thread, file gpu_multi_context_threaded_aottest.cpp, line 237.
473/474 Test #631: generator_aotcpp_gpu_multi_context_threaded ................Subprocess aborted***Exception: 32.16 sec
2024-06-28 17:09:27.944 generator_aotcpp_gpu_multi_context_threaded[91558:5585726] Metal API Validation Enabled
Assertion failed: (buf1_result.all_equal(val + 2)), function run_kernels_on_thread, file gpu_multi_context_threaded_aottest.cpp, line 236.

Can this be merged, or should something change?

@mcourteaux
Copy link
Contributor Author

@steven-johnson Can you have a look if this is okay? Side question: I'm guessing many folks are taking vacations and activity here is lower because of that?

@steven-johnson steven-johnson self-requested a review July 16, 2024 15:34
@steven-johnson steven-johnson merged commit b741d9c into halide:main Jul 16, 2024
18 of 19 checks passed
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.

3 participants