font-variant display

[roger64]

• KOReader version:2021.06
• Device:Kindle PW3

On this attached scrambled ePub3, I have used open-type "font-variant:small-caps" for the two following styles Heading and Subtitle (see page in chapitre 4.xhtml).

I also used "font-variant:normal" for a span within these styles (see styles.css).

Code:

.Heading span {
  display: block;
  margin-top: 0.25em;
  font-size: 0.85em;
  font-style: italic;
  font-variant: normal;
  color: #585858;
}
.Subtitle span {
  display: block;
  margin-top: 0.25em;
  font-size: 0.9em;
  font-family: LinLibertineG30;
  font-style: italic;
  font-variant: normal;
}

The resulting display with Calibre and Sigil are OK (see screenshot)
With Koreader, I get italic with small-caps which is not intended.

scrambled.zip

[poire-z]

#5821 (comment)
The way we handle font-variant (all values from the various font-variant-* properties mapped into a single 32bits bitmap, OR'ing all values from every declaration that apply to a node) and its inheritance (OR'ing the value we got for a node with the value of its parent), has many shortcomings vs. how it's supposed to work according to the specs. Too different for me to even think about compiling the differences...

But looking at the code, it seems our/my handling of none and normal is limited to just cancelling previous values in a same property (ie: font-variant: small-caps normal oldstyle-num will only result in oldstyle-num), which feels a bit useless.
https://github.com/koreader/crengine/blob/844be50324fb7ddd7849bca04a191803f264f237/crengine/src/lvstsheet.cpp#L2284-L2389

We could fix your use-case by:

• having normal/none reset previously applied (from lower specificity CSS declarations) font-variant
• when a node ends up with having been applied font-variant: normal (and nothing else), don't inherit anything from its parent.

Feels a bit better than previous case, although with our implementation shortcomings, it's a bit hard to guess what other cases it might break :) but there are some.
(Like if in that book, you would have set body { font-variant: oldstyle-nums }, numbers in your .Heading span won't be oldstyle anymore.)

--- a/crengine/src/lvrend.cpp
+++ b/crengine/src/lvrend.cpp
@@ -10076,34 +10076,40 @@ void setNodeStyle( ldomNode * enode, css_style_ref_t parent_style, LVFontRef par

     // font_features (font-variant/font-feature-settings)
     // The specs say a font-variant resets the ones that would be
     // inherited (as inheritance always does).
     // But, as we store in a single bitmap the values from multiple
     // properties (font-variant, font-variant-caps...), we drift from
     // the specs by OR'ing the ones sets by style on this node with
     // the ones inherited from parents (so we can use style-tweaks
     // like body { font-variant: oldstyle-nums; } without that being
     // reset by a lower H1 { font-variant: small-caps; }.
     // Note that we don't handle the !important bit whether it's set
     // for this node or the parent (if it's set on the parent, we
     // could decide to = instead of |=), as it's not clear whether
     // it's better or not: we'll see.
     // (Note that we can use * { font-variant: normal !important; } to
     // stop any font-variant without !important from being applied.)
-    pstyle->font_features.value |= parent_style->font_features.value;
-    pstyle->font_features.type = css_val_unspecified;
+    // There is one case where we don't inherit: when styles had this
+    // node ending up being (css_val_unspecified, 0), which can only
+    // happen with "font-variant(-*): normal/none", that might be
+    // used to prevent some upper font-variant to be inherited.
+    if ( pstyle->font_features.type == css_val_inherited || pstyle->font_features.value != 0 ) {
+        pstyle->font_features.value |= parent_style->font_features.value;
+        pstyle->font_features.type = css_val_unspecified;
+    }

--- a/crengine/src/lvstsheet.cpp
+++ b/crengine/src/lvstsheet.cpp
@@ -3185,7 +3185,16 @@ void LVCssDeclaration::apply( css_style_rec_t * style )
         case cssd_font_features:
             // We want to 'OR' the bitmap from any declaration that is to be applied to this node
             // (while still ensuring !important).
-            style->ApplyAsBitmapOr( read_length(p), &style->font_features, imp_bit_font_features, is_important );
+            {
+                css_length_t font_features = read_length(p);
+                if ( font_features.value == 0 && font_features.type == css_val_unspecified ) {
+                    // except if "font-variant: normal/none", which resets all previously set bits
+                    style->Apply( font_features, &style->font_features, imp_bit_font_features, is_important );
+                }
+                else {
+                    style->ApplyAsBitmapOr( font_features, &style->font_features, imp_bit_font_features, is_important );
+                }
+            }
             break;
         case cssd_text_indent:
             style->Apply( read_length(p), &style->text_indent, imp_bit_text_indent, is_important );

Thoughts?

[roger64]

Thank you for your explanations and work. I am sorry I cannot help you much on this.

With my basic knowledge, I had hoped that selecting "font-variant: normal" would just cancel the "font-variant:small-caps" value used with the parent element (Heading or Subtitle style).

I also thought that the property used in this book, i.e. "font-variant-numeric: oldstyle-nums" belonged to a distinct property and was unrelated (or unconnected) to the one above.

[poire-z]

I also thought that the property used in this book, i.e. "font-variant-numeric: oldstyle-nums" belonged to a distinct property and was unrelated (or unconnected) to the one above.

^ That's from the CSS2 specs.
In the latest specs, font-variant is a shorthand property, and can have the keywords of all the others.
https://developer.mozilla.org/en-US/docs/Web/CSS/font-variant
And font-variant:normal cancels them all (exept some ligatures, than none does additionnally cancel).

font-variant-caps: normal should disable small-caps only.
font-variant: normal should disable small-caps but also oldstyle-nums if set by font-variant-numeric: oldstyle-nums or inherited from something set with this.

We were limited at the time by the number of properties we can support (64) - but since then, we killed this limitation. So, we would now have room to support all these as distinct properties.

But it's still really complex: are font-variant-caps: petite-caps; small-caps mutually exclusive? I think so.
Are font-variant-numeric: oldstyle-nums, slashed-zero, diagonal-fractions mutually exclusive? I don't think so.
So, lots of little things to handle manually... for some so rarely used property (except for small-caps and oldstyle-nums it seems).

Not really keen on reworking all this :) but we can tweak the current really-limited implemenation to make it better with the most classic use cases,

[roger64]

OK. I was not aware of this change.

Then, if Koreader makes do as you said,

"(Like if in that book, you would have set body { font-variant: oldstyle-nums }, numbers in your .Heading span won't be oldstyle anymore.)"

it would really not be a problem is the span is not oldstyle anymore. The main thing is that this span becomes just "normal" and not "small-caps".