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.

Sign up for GitHub

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

Jump to bottom

pretty: prefer shrinking instead of thinning text, and make SPIR-V ops/enums more distinct. #21

Merged
merged 5 commits into from
Mar 12, 2023
Merged

pretty: prefer shrinking instead of thinning text, and make SPIR-V ops/enums more distinct. #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8830fc7
print: add `font-size` styling and use it for the comment style.
eddyb Feb 16, 2023
9175409
print: shrink `foo.` prefix sizes and use `spv.` and/or orange for SP…
eddyb Feb 16, 2023
74e13fb
spv/print: `Enum.(.A)` -> `Enum.A`, `Enum.(.A | .B)` -> `Enum.{A, B}`.
eddyb Feb 16, 2023
eb26401
print/pretty: force-disable Dark Reader to fix `htmlpreview.github.io`.
eddyb Mar 12, 2023
948bab2
Update CHANGELOG.
eddyb Mar 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased] - ReleaseDate

### Changed 🛠
- [PR#21](https://github.com/EmbarkStudios/spirt/pull/21) tweaked pretty-printing
styles around de-emphasis (shrinking instead of thinning, for width savings),
and SPIR-V ops/enums (via de-emphasized `spv.` prefix and distinct orange color)

## [0.1.0] - 2022-12-16
*Initial public release of SPIR-T for minimal Rust-GPU integration.*
### Added ⭐
Expand Down
135 changes: 100 additions & 35 deletions src/print/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ impl Printer<'_> {
fn comment_style(&self) -> pretty::Styles {
pretty::Styles {
color_opacity: Some(0.3),
thickness: Some(-3),
size: Some(-4),
..pretty::Styles::color(pretty::palettes::simple::DARK_GRAY)
}
}
Expand All @@ -968,12 +968,13 @@ impl Printer<'_> {
..pretty::Styles::color(pretty::palettes::simple::MAGENTA)
}
}
fn spv_operand_kind_name_style(&self) -> pretty::Styles {
// HACK(eddyb) effectively this is almost always redundant.
fn spv_base_style(&self) -> pretty::Styles {
pretty::Styles::color(pretty::palettes::simple::ORANGE)
}
fn spv_op_style(&self) -> pretty::Styles {
pretty::Styles {
color_opacity: Some(0.4),
thickness: Some(-3),
..self.declarative_keyword_style()
thickness: Some(3),
..self.spv_base_style()
}
}
fn spv_enumerand_name_style(&self) -> pretty::Styles {
Expand All @@ -987,6 +988,18 @@ impl Printer<'_> {
..Default::default()
}
}

/// Compute a suitable style for an unintrusive `foo.` "namespace prefix",
/// from a more typical style (by shrinking and/or reducing visibility).
fn demote_style_for_namespace_prefix(&self, mut style: pretty::Styles) -> pretty::Styles {
// NOTE(eddyb) this was `opacity: Some(0.4)` + `thickness: Some(-3)`,
// but thinner text ended up being more annoying to read while still
// using up too much real-estate (compared to decreasing the size).
style.color_opacity = Some(style.color_opacity.unwrap_or(1.0) * 0.6);
// FIXME(eddyb) maybe this could be more uniform with a different unit.
style.size = Some(style.size.map_or(-4, |size| size - 1));
style
}
}

impl<'a> Printer<'a> {
Expand All @@ -998,6 +1011,19 @@ impl<'a> Printer<'a> {
pretty::join_space(":", [ty.print(self)])
}

/// Pretty-print a SPIR-V `opcode`'s name, prefixed by `"spv."`.
fn pretty_spv_opcode(
&self,
opcode_name_style: pretty::Styles,
opcode: spv::spec::Opcode,
) -> pretty::Fragment {
pretty::Fragment::new([
self.demote_style_for_namespace_prefix(self.spv_base_style())
.apply("spv."),
opcode_name_style.apply(opcode.name()),
])
}

/// Pretty-print a single SPIR-V operand from only immediates, potentially
/// composed of an enumerand with parameters (which consumes more immediates).
fn pretty_spv_operand_from_imms(
Expand All @@ -1010,14 +1036,32 @@ impl<'a> Printer<'a> {
.tokens
.into_iter()
.map(|token| match token {
spv::print::Token::Error(s) => self.error_style().apply(s),
spv::print::Token::Error(s) => self.error_style().apply(s).into(),
spv::print::Token::Punctuation(s) => s.into(),
spv::print::Token::OperandKindName(s) => {
self.spv_operand_kind_name_style().apply(s)
spv::print::Token::OperandKindNamespacePrefix(s) => {
pretty::Fragment::new([
// HACK(eddyb) double-demote to end up with `spv.A.B`,
// with increasing size from `spv.` to `A.` to `B`.
self.demote_style_for_namespace_prefix(
self.demote_style_for_namespace_prefix(self.spv_base_style()),
)
.apply("spv."),
// FIXME(eddyb) avoid the cost of allocating here.
self.demote_style_for_namespace_prefix(
self.declarative_keyword_style(),
)
.apply(format!("{s}.")),
])
}
spv::print::Token::EnumerandName(s) => {
self.spv_enumerand_name_style().apply(s).into()
}
spv::print::Token::NumericLiteral(s) => {
self.numeric_literal_style().apply(s).into()
}
spv::print::Token::StringLiteral(s) => {
self.string_literal_style().apply(s).into()
}
spv::print::Token::EnumerandName(s) => self.spv_enumerand_name_style().apply(s),
spv::print::Token::NumericLiteral(s) => self.numeric_literal_style().apply(s),
spv::print::Token::StringLiteral(s) => self.string_literal_style().apply(s),
spv::print::Token::Id(_) => unreachable!(),
}),
)
Expand All @@ -1039,7 +1083,7 @@ impl<'a> Printer<'a> {
/// [`Print::print`] method, instead of a closure, as `print_id`).
///
/// Immediate operands are wrapped in angle brackets, while `ID` operands are
/// wrapped in parentheses, e.g.: `OpFoo<Bar, 123, "baz">(v1, v2)`.
/// wrapped in parentheses, e.g.: `spv.OpFoo<Bar, 123, "baz">(v1, v2)`.
///
/// This should be used everywhere a SPIR-V instruction needs to be printed,
/// to ensure consistency across all such situations.
Expand All @@ -1054,7 +1098,7 @@ impl<'a> Printer<'a> {
) -> pretty::Fragment {
// Split operands into "angle brackets" (immediates) and "parens" (IDs),
// with compound operands (i.e. enumerand with ID parameter) using both,
// e.g: `OpFoo<Bar(/* #0 */)>(/* #0 */ v123)`.
// e.g: `spv.OpFoo<Bar(/* #0 */)>(/* #0 */ v123)`.
let mut next_extra_idx: usize = 0;
let mut paren_operands = SmallVec::<[_; 16]>::new();
let mut angle_bracket_operands =
Expand All @@ -1067,19 +1111,33 @@ impl<'a> Printer<'a> {
// FIXME(eddyb) deduplicate the `Token` match with `pretty_spv_operand_from_imms`.
Some(pretty::Fragment::new(operand.tokens.into_iter().map(
|token| match token {
spv::print::Token::Error(s) => self.error_style().apply(s),
spv::print::Token::Error(s) => self.error_style().apply(s).into(),
spv::print::Token::Punctuation(s) => s.into(),
spv::print::Token::OperandKindName(s) => {
self.spv_operand_kind_name_style().apply(s)
spv::print::Token::OperandKindNamespacePrefix(s) => {
pretty::Fragment::new([
// HACK(eddyb) double-demote to end up with `spv.A.B`,
// with increasing size from `spv.` to `A.` to `B`.
self.demote_style_for_namespace_prefix(
self.demote_style_for_namespace_prefix(
self.spv_base_style(),
),
)
.apply("spv."),
// FIXME(eddyb) avoid the cost of allocating here.
self.demote_style_for_namespace_prefix(
self.declarative_keyword_style(),
)
.apply(format!("{s}.")),
])
}
spv::print::Token::EnumerandName(s) => {
self.spv_enumerand_name_style().apply(s)
self.spv_enumerand_name_style().apply(s).into()
}
spv::print::Token::NumericLiteral(s) => {
self.numeric_literal_style().apply(s)
self.numeric_literal_style().apply(s).into()
}
spv::print::Token::StringLiteral(s) => {
self.string_literal_style().apply(s)
self.string_literal_style().apply(s).into()
}
spv::print::Token::Id(id) => {
let comment = self
Expand All @@ -1092,7 +1150,7 @@ impl<'a> Printer<'a> {
});
paren_operands.push(pretty::join_space(comment.clone(), [id]));

comment
comment.into()
}
},
)))
Expand All @@ -1101,8 +1159,8 @@ impl<'a> Printer<'a> {
.peekable();

// Put together all the pieces, angle-bracketed operands then parenthesized
// ones, e.g.: `OpFoo<Bar, 123, "baz">(v1, v2)` (with either group optional).
let mut out = spv_inst_name_style.apply(opcode.name()).into();
// ones, e.g.: `spv.OpFoo<Bar, 123, "baz">(v1, v2)` (with either group optional).
let mut out = self.pretty_spv_opcode(spv_inst_name_style, opcode);

if angle_bracket_operands.peek().is_some() {
out = pretty::Fragment::new([
Expand Down Expand Up @@ -1615,7 +1673,7 @@ impl Print for ExportKey {
struct ImplicitTargetId;

printer.pretty_spv_inst(
printer.declarative_keyword_style(),
printer.spv_op_style(),
wk.OpEntryPoint,
imms,
&[ImplicitTargetId],
Expand Down Expand Up @@ -1865,7 +1923,7 @@ impl Print for TypeDef {
} else {
match *ctor {
TypeCtor::SpvInst(spv::Inst { opcode, ref imms }) => printer.pretty_spv_inst(
printer.declarative_keyword_style(),
printer.spv_op_style(),
opcode,
imms,
ctor_args,
Expand All @@ -1876,9 +1934,9 @@ impl Print for TypeDef {
None,
),
TypeCtor::SpvStringLiteralForExtInst => pretty::Fragment::new([
printer.error_style().apply("type_of"),
printer.error_style().apply("type_of").into(),
"(".into(),
printer.declarative_keyword_style().apply("OpString"),
printer.pretty_spv_opcode(printer.spv_op_style(), wk.OpString),
")".into(),
]),
}
Expand Down Expand Up @@ -2020,19 +2078,20 @@ impl Print for ConstDef {
pretty::Fragment::new(["&".into(), gv.print(printer)])
}
ConstCtor::SpvInst(spv::Inst { opcode, ref imms }) => printer.pretty_spv_inst(
printer.declarative_keyword_style(),
printer.spv_op_style(),
opcode,
imms,
ctor_args,
Print::print,
Some(*ty),
),
ConstCtor::SpvStringLiteralForExtInst(s) => pretty::Fragment::new([
printer.declarative_keyword_style().apply("OpString"),
printer.pretty_spv_opcode(printer.spv_op_style(), wk.OpString),
"<".into(),
printer
.string_literal_style()
.apply(format!("{:?}", &printer.cx[s])),
.apply(format!("{:?}", &printer.cx[s]))
.into(),
">".into(),
]),
}),
Expand Down Expand Up @@ -2475,7 +2534,7 @@ impl Print for DataInstDef {
return AttrsAndDef {
attrs,
def_without_name: printer.pretty_spv_inst(
printer.declarative_keyword_style(),
printer.spv_op_style(),
opcode,
imms,
inputs,
Expand All @@ -2485,18 +2544,24 @@ impl Print for DataInstDef {
};
}
DataInstKind::SpvExtInst { ext_set, inst } => {
let wk = &spv::spec::Spec::get().well_known;

// FIXME(eddyb) should this be rendered more compactly?
pretty::Fragment::new([
"(".into(),
printer.declarative_keyword_style().apply("OpExtInstImport"),
printer.pretty_spv_opcode(printer.spv_op_style(), wk.OpExtInstImport),
"<".into(),
printer
.string_literal_style()
.apply(format!("{:?}", &printer.cx[ext_set])),
.apply(format!("{:?}", &printer.cx[ext_set]))
.into(),
">).".into(),
printer.declarative_keyword_style().apply("OpExtInst"),
printer.pretty_spv_opcode(printer.spv_op_style(), wk.OpExtInst),
"<".into(),
printer.numeric_literal_style().apply(format!("{inst}")),
printer
.numeric_literal_style()
.apply(format!("{inst}"))
.into(),
">".into(),
])
}
Expand Down
32 changes: 28 additions & 4 deletions src/print/pretty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ pub struct Styles {
/// For HTML output, each unit is equivalent to `±100` in CSS `font-weight`.
pub thickness: Option<i8>,

/// `0` corresponds to the default, with positive values meaning larger,
/// and negative values smaller text, respectively.
///
/// For HTML output, each unit is equivalent to `±0.1em` in CSS `font-size`.
pub size: Option<i8>,

pub subscript: bool,
pub superscript: bool,
}
Expand All @@ -90,12 +96,16 @@ pub mod palettes {
/// Minimalist palette, chosen to work with both light and dark backgrounds.
pub mod simple {
pub const DARK_GRAY: [u8; 3] = [0x44, 0x44, 0x44];

pub const RED: [u8; 3] = [0xcc, 0x55, 0x55];
pub const GREEN: [u8; 3] = [0x44, 0x99, 0x44];
pub const BLUE: [u8; 3] = [0x44, 0x66, 0xcc];

pub const YELLOW: [u8; 3] = [0xcc, 0x99, 0x44];
pub const MAGENTA: [u8; 3] = [0xcc, 0x44, 0xcc];
pub const CYAN: [u8; 3] = [0x44, 0x99, 0xcc];

pub const ORANGE: [u8; 3] = [0xcc, 0x77, 0x55];
}
}

Expand Down Expand Up @@ -176,8 +186,19 @@ impl HtmlSnippet {
var params = new URLSearchParams(document.location.search);
var dark = params.has("dark"), light = params.has("light");
if(dark || light) {
if(dark && !light)
if(dark && !light) {
document.documentElement.classList.add("simple-dark-theme");

// HACK(eddyb) forcefully disable Dark Reader, for two reasons:
// - its own detection of websites with built-in dark themes
// (https://github.com/darkreader/darkreader/pull/7995)
// isn't on by default, and the combination is jarring
// - it interacts badly with whole-document-replacement
// (as used by htmlpreview.github.io)
document.documentElement.removeAttribute('data-darkreader-scheme');
document.querySelectorAll('style.darkreader')
.forEach(style => style.disabled = true);
}
} else if(matchMedia("(prefers-color-scheme: dark)").matches) {
// FIXME(eddyb) also use media queries in CSS directly, to ensure dark mode
// still works with JS disabled (sadly that likely requires CSS duplication).
Expand All @@ -188,9 +209,7 @@ impl HtmlSnippet {

<style>
/* HACK(eddyb) `[data-darkreader-scheme="dark"]` is for detecting Dark Reader,
as its own automatic detection of websites with built-in dark themes
(https://github.com/darkreader/darkreader/pull/7995) isn't on by default,
and the result is jarring when both dark modes combine. */
to avoid transient interactions (see also comment in the `<script>`). */

html.simple-dark-theme:not([data-darkreader-scheme="dark"]) {
background: #16181a;
Expand Down Expand Up @@ -302,6 +321,7 @@ impl FragmentPostLayout {
color,
color_opacity,
thickness,
size,
subscript: _,
superscript: _,
} = *styles;
Expand All @@ -325,6 +345,10 @@ impl FragmentPostLayout {
write!(css_style, "font-weight:{};", 500 + (thickness as i32) * 100)
.unwrap();
}
if let Some(size) = size {
write!(css_style, "font-size:{}em;", 1.0 + (size as f64) * 0.1)
.unwrap();
}
if !css_style.is_empty() {
push_attr("style", &css_style);
}
Expand Down
Loading