koreader / crengine

This is the KOReader CREngine fork. It cross-pollinates with the official CoolReader repository at https://github.com/buggins/coolreader, in case you were looking for that one.
69 stars 44 forks source link

Incorrect font rendering in a specific book #516

Open efermi opened 1 year ago

efermi commented 1 year ago

In the linked book an embedded font is being used to implement drop caps. In the web and book render modes they aren't visible at all, while in the flat and legacy modes they are visible but the font rendering seems incorrect. As the book looks like it has been authored with Apple tools I checked it in Foliate which also uses the WebKit engine for rendering. So here is what I assume a reference look of the document in Foliate: Screenshot_20230324_000929-foliate Web mode: Screenshot_20230324_000618-ko-web-mode Flat mode: Screenshot_20230324_000822-ko-flat-mode The file: The Ruby in the Smoke.zip I've tried to extract the font and embed it in a minimal test document created in calibre's ebook-editor and it looked right in KOReader, so probably the font itself isn't the problem, but some combination of the document layout and engine quirks is to blame?

poire-z commented 1 year ago

crengine is much more limited in what/how it can handles things vs. regular web browser engines, so we'll often meet something that's not handled well. And when that happens you can try to investigate the HTML and the CSS, selecting text around and View HTML, and long pressing on elements (new, in current release).

For this float, we see there the publisher styles, in yellow: image

I did copy that yellow bit into Book style tweak, and tried to workaround these, cancelling some and making them more normal. I tried a few things, and restoring line-height: 0% to 100% or inherit (in green above) makes it somehow work better (minus the little shift above):

image

We may do bad with edgy things (like negative margins in %, or a line-height of 0, and/or floats and inline-boxes), and it may be hard to fix - so sometimes, it's easier to let that to style tweaks and leave the pleasure of fixing its own stuff to the user :)

(But naively, I'd say what we do is somehow expected with a line-height of 0 :) and nothing being displayed may be because of clipping, dunno. I also have no idea why all this styling and what it's supposed to achieve.)

efermi commented 1 year ago

The styling there is certainly bizarre, but could it be handled in a way that doesn't make text disappear? Or is this the quirk of WebKit rendering and shouldn't be worked around in the engine? And what about the font, compare it to the first picture, it's a combination of thick and thin strokes, but the engine draws it in nearly uniform lines.

poire-z commented 1 year ago

The styling there is certainly bizarre, but could it be handled in a way that doesn't make text disappear? Or is this the quirk of WebKit rendering and shouldn't be worked around in the engine?

Dunno, would need investigating.

This is how some (older) version of Calibre renders it: image

The bottom part is me having added:

<div style='font-family:"Romanesque";'>O T P</div>
<div style='font-family:"Romanesque"; font-size:500%'>O T P</div>
<div style='font-family:"Romanesque"; font-size:500%; font-weight: bold'>O T P</div>

And this is how koreader/crengine renders that other part: image

And what about the font, compare it to the first picture, it's a combination of thick and thin strokes, but the engine draws it in nearly uniform lines.

That may be because on this, we may do better than webengines - and here, better happens to make it worse :) In the CSS, there is this:

@font-face{
font-family:"Romanesque";
src:url(../fonts/Romanesque_Normal.ttf);
font-weight:bold;
font-style:normal;
}

so, this defines a font named "Romanesque", and says that when requesting it for an element with font-weight:bold; , it should use ../fonts/Romanesque_Normal.ttf - and it doesn't say what ttf file to use when we request it for an element not bold. So, other engines may just use the only ttf they know about as-is - while crengine will try to make the only font it knows about (for bold) thinner (so, synthetic normal), so, here, messing with the stroke widths... We already met that (about italic and fake italic) at https://github.com/koreader/koreader/issues/10040#issuecomment-1399068812.

Add font-weight: bold to the style tweak shown in previous post, and you'll get it ok.

Frenzie commented 1 year ago

I tried a few things, and restoring line-height: 0% to 100% or inherit (in green above) makes it somehow work better (minus the little shift above):

I think strictly speaking you may be supposed to see a variety of things drawn over each other with line-height 0, so without looking closer I suspect seeing nothing could be wrong. That doesn't mean that which happens in Foliate is right of course; I'd expect something closer to what your old Calibre screenshot shows really.

efermi commented 1 year ago

Thank you for the extensive and clear explanations, feel free to close the issue as I tend to agree with your approach.

Frenzie commented 1 year ago

Sidenote, but my Foliate 2.6.4 doesn't render the font: image

My calibre 6.13 also renders it like this: image

Get rid of the weird line-height thing and: image

poire-z commented 1 year ago

The styling there is certainly bizarre, but could it be handled in a way that doesn't make text disappear? Or is this the quirk of WebKit rendering and shouldn't be worked around in the engine?

Investigated a bit why it would not be displayed - and its drawing is actually skipped. Floats are quite complicated to render, we have a floatBox wrapper, that we make sure it has always a height >= 1px so it is drawn, that's the little grey bar in here, that would get highlighted in if you select text from before to after:

image

Under some conditions of line-height and the negative top/bottom margins, the inner content (so, actually, a paragraph with the letter "O") may be positionned fully outside this little 1px-height box - and this is a condition for its drawing to be skipped. If it would just intercept with this little box, it would be drawn - and that is why when tweaking line-height and margins, we may get it to be displayed.

If I tweak the check for drawing outside content:

@@ -9609,7 +9610,9 @@ void DrawDocument( LVDrawBuf & drawbuf, ldomNode * enode, int x0, int y0, int dx
         int height = fmt.getHeight();
         int top_overflow = fmt.getTopOverflow();
         int bottom_overflow = fmt.getBottomOverflow();
-        if ( (doc_y + height + bottom_overflow <= 0 || doc_y - top_overflow >= 0 + dy) ) {
+        printf("%d + %d + %d = %d || %d\n", doc_y, height, bottom_overflow, doc_y + height + bottom_overflow, doc_y - top_overflow - dy);
+        if ( enode->getParentNode() && enode->getParentNode()->isFloatingBox() ) { printf("goingon\n");} else
+        if ( (doc_y + height + bottom_overflow < 0 || doc_y - top_overflow >= 0 + dy) ) {
             // We don't have to draw this node.
             // Except TR which may have cells with rowspan>1, and even though
             // this TR is out of range, it must draw a rowspan>1 cell, so it

we get the above picture, which is not even nice, as the float is only 1px, so it's overflowing content override the text. Negative margins probably, which is something we are quite limited at handling, mostly because it's not worth the pain getting into it :) I'm happy enough we can draw corectly "dense" floats :)

I'm not sure the above fix would be the right thing in other cases, so I won't touch anything. I don't like missing content - but at least, in the current situation we see a hole and know something is missing :)

efermi commented 1 year ago

Sidenote, but my Foliate 2.6.4 doesn't render the font

@Frenzie There is a checkbox in the options to use publisher font. @poire-z Thank you for illuminating the arcane complexities of the web renderer :)

Frenzie commented 1 year ago

@poire-z Yes, something in that general direction is the rendering I expected to see with line-height: 0 in this scenario.