Skip to content

[fontir] Tweak glyph ir logging#1589

Merged
cmyr merged 1 commit into
mainfrom
glyph-logging-tweaks
Aug 8, 2025
Merged

[fontir] Tweak glyph ir logging#1589
cmyr merged 1 commit into
mainfrom
glyph-logging-tweaks

Conversation

@cmyr

@cmyr cmyr commented Aug 8, 2025

Copy link
Copy Markdown
Member

Tiny fixups to try and make some of these things more visible, as I try to debug a funny component flattening issue.

JMM

Comment thread fontir/src/glyph.rs Outdated
}
trace!(
"Flatten {} {:?}",
log::debug!(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this should stay trace, and probably the flattening non-export components of should also be trace?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What is your heuristic for making that determination?

my issue with trace is that if you turn it on our logging is incredibly verbose, and it's very hard to find anything. These operations don't happen that often, and I often want to see if they happen because they end up being a common source of glyf/gvar difference.

@rsheeter rsheeter Aug 8, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's not a hard rule but broadly I would expect:

  1. Debug is safe to turn on globally. It'll be noisy but not completely unusable
  2. Trace is too noisy to turn on globally, it's very detailed and you shoudl turn it on only for the module you are troubleshooting (e.g. export RUST_LOG=warn,fontir::glyph=debug)
    • Aside: Invalid kern side seems to be very common, to the point I wonder if it's really a warning

Are you turning trace on globally or for a specific module?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm basically not turning trace on at all 😅

Comment thread fontir/src/glyph.rs Outdated
trace!(
"Flatten {} {:?}",
log::debug!(
"flattening {} (components: {:?})",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: capitalization of the first word of the log message is inconsistent. I think mostly we capitalize so Flattening

@cmyr cmyr force-pushed the glyph-logging-tweaks branch from 7d5b545 to b564845 Compare August 8, 2025 16:45
@cmyr cmyr force-pushed the glyph-logging-tweaks branch from b564845 to 68d370f Compare August 8, 2025 17:38
@cmyr cmyr added this pull request to the merge queue Aug 8, 2025
Merged via the queue into main with commit 145e5fd Aug 8, 2025
12 checks passed
@cmyr cmyr deleted the glyph-logging-tweaks branch August 8, 2025 17:44
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.

2 participants