-
Notifications
You must be signed in to change notification settings - Fork 47
Enhanced Visibility Controls #1683
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
base: develop
Are you sure you want to change the base?
Enhanced Visibility Controls #1683
Conversation
Introduce enabled/disabled/phantom visibility handling across staff lines, lyrics, notes, clefs, custos, and NABC, with shared helpers and documentation updates.
Keep NABC/above-lines elements independent of note visibility, adjust spacing when notes/lines are hidden, and document the separation.
|
For some elements, there isn't going to be any difference between ETA: I'm thinking especially of all the options like |
| \fi | ||
| }% | ||
|
|
||
| % Helpers to control text rendering mode (LuaTeX) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these ever used? I didn't see any place where they were.
| \ifgre@showclef% | ||
| \gre@skip@temp@four = \gre@space@skip@afterclefnospace\relax% | ||
| \hbox{\gre@typeclef{#1}{#2}{0}{#3}{#4}{#5}{#6}{#7}\gre@hskip\gre@skip@temp@four}% | ||
| \ifnum\gre@clef@phantomwrapper>0\relax |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If \gre@clef@phantomwrapper is 0, then \gre@applyphantomwrapper does nothing, so can't you just call \gre@applyphantomwrapper in all cases?
|
What do you think about merging |
I've did a deeper review and realized the only commands I'll remove Does it sound better for you, @davidweichiang ? |
|
My opinions: Should remain visible/invisible:
I think all the options (enabled, disabled, phantom, hphantom, and vphantom) make sense:
The following have zero width anyway, so hphantom is the same as disabled, and vphantom is the same as phantom. But enabled, disabled, and phantom make sense.
|
| \gre@dimen@temp@five=\dimexpr(\gre@dimen@staffheight % | ||
| + \gre@space@dimen@spacebeneathtext % | ||
| + \gre@space@dimen@spacelinestext)\relax% | ||
| % If notes/lines are hidden with hphantom or disabled, bring NABC/ALT closer to the lyrics line |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be cleaner to do
temp5 = beneathtext+spacelinestext
if notes or lines are enabled or phantom or vphantom then temp5 += staffheight
instead of adding and then subtracting staffheight?
Summary
This pull request introduces comprehensive visibility controls for GregorioTeX elements, including support for phantom rendering modes, and decouples NABC and above-lines-text visibility from note visibility.
Changes Overview
This PR consists of two main commits:
1. Add Unified Visibility Controls (edd97f4)
Introduces a unified visibility control system across multiple GregorioTeX elements:
enabled,disabled,phantom,hphantom,vphantom\gre@processvisibilityand\gre@applyphantomwrapperfor consistent handlingvisible/invisibleoptions are deprecated in favor of the new systemKey features:
hphantom: preserves horizontal spacing onlyvphantom: preserves vertical spacing onlyphantom: preserves both horizontal and vertical spacing2. Decouple NABC and Above-Lines Visibility (b43b12a)
Makes NABC and above-lines-text elements independent from note visibility:
\gre@notes@rendernabcmacro for NABC-only rendering when notes are disabledKey features:
Backward Compatibility
visible/invisibleoptions remain functional but emit deprecation warningsenabled/disabledoptionsResolves