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

print: shorten names like type2/func3/etc. to T2/F3/etc. #39

Merged
merged 1 commit into from
Jun 14, 2023

Conversation

eddyb
Copy link
Contributor

@eddyb eddyb commented Jun 14, 2023

This is something I realized while looking at the output of #36, where keywords like type or const were overly verbose.

I've kept the keywords but only on definitions, so e.g. type T5 = ... is referred to as T5.

Comparison (using Kajiya's assets/shaders/ircache/raster_origins_vs.hlsl, compiled by DXC):

Before
(click for complete pretty HTML example)
After
(click for complete pretty HTML example)
image image
image image

@eddyb eddyb force-pushed the print-shorter-names branch from afeecc1 to a99d2a5 Compare June 14, 2023 04:35
@eddyb eddyb force-pushed the print-shorter-names branch from a99d2a5 to 852853c Compare June 14, 2023 05:13
@eddyb eddyb marked this pull request as ready for review June 14, 2023 05:14
@eddyb eddyb enabled auto-merge June 14, 2023 05:14
@eddyb eddyb added this pull request to the merge queue Jun 14, 2023
Merged via the queue into main with commit 5a27d6d Jun 14, 2023
@eddyb eddyb deleted the print-shorter-names branch June 14, 2023 05:20
eddyb added a commit that referenced this pull request Jun 18, 2023
…guous. (#36)

I've also made the `T1` style use subscripts (so more like `T₁`),
because I wasn't happy with the aesthetics of having digits exactly as
large as the prefix.

The reason I haven't done the subscript change earlier is e.g. `v5`
already looks great, and `v₅` and `T₅` still looks different because the
`T` is larger than the `v` - the missing trick was to make the
subscripts larger when the prefix is uppercase, which seems to balance
things out.

Comparison (using Kajiya's
`assets/shaders/ircache/raster_origins_vs.hlsl`, compiled by DXC, like
#39):

|[**Before**](https://htmlpreview.github.io/?https://gist.github.com/eddyb/22a1468ce5341e822e95204a91406897/raw/0-before-spirt%252336-raster_origins_vs.hlsl.structured.spirt.html)<br><sub>(click
for complete pretty HTML
example)</sub>|[**After**](https://htmlpreview.github.io/?https://gist.github.com/eddyb/22a1468ce5341e822e95204a91406897/raw/1-after-spirt%252336-raster_origins_vs.hlsl.structured.spirt.html)<br><sub>(click
for complete pretty HTML example)</sub>|
|-|-|

|![image](https://github.com/EmbarkStudios/spirt/assets/77424/c6a6c00d-1e80-49e8-a87d-d45c4287cdd2)|![image](https://github.com/EmbarkStudios/spirt/assets/77424/d595c9d2-5098-42b4-90dc-4df25d890195)|

|![image](https://github.com/EmbarkStudios/spirt/assets/77424/6ef0ae99-eaea-4453-b804-13eed6afd8d4)|![image](https://github.com/EmbarkStudios/spirt/assets/77424/acd6224f-f1b5-4c72-a8f8-980418447279)|
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.

1 participant