Background images enhancements, fix FB2 footnotes alignment #353

poire-z · 2020-07-05T16:13:09Z

`(Upstream) HyphMan cleanup`

Relevant stuff by @virxkane picked from https://github.com/virxkane/coolreader/commit/e461e58c6cc69458204852eed9225e0ad1fa0d6d

`Rendering methods: remove erm_runin`

`Text formatting: simplify 'display: run-in' handling`

Simplify it so this obsolete stuff used to support for FB2 footnotes (see #329) is more localized and does not bleed on too much code.
This also fixes an issue with alignment since #347 cleaning. Will allow closing koreader/koreader#6344.

For reference, FB2 footnotes could be handled with floats (but display: run-in has the benefit of blending in the paragraph, so for Arabic RTL FB2, they would be displayed at start, on the right - unlike with floats):

body[name="notes"] section title,
body[name="comments"] section title {
    float: left;
    margin: 0;
    margin-right: 0.5em; /* spacing after footnote number */
    font-size: inherit;
    font-weight: bold;
    text-align: start; /* counteract default of center with regular title */
    page-break-before: auto; /* counteract default of always with regular title */
    page-break-inside: auto;
    page-break-after: auto;
}
body[name="notes"] section title p,
body[name="comments"] section title p,
body[name="notes"] section title ~ p,
body[name="comments"] section title ~ p {
    text-indent: 0;
}

But we'll keep using display: run-in (with floats, they would not work in legacy and flat rendering modes)

`Simplify background image drawing`

`CSS: adds support for "background-size" property`

`CSS: support background:url("data:image/png;base64,...)`

Some fix and improvement to CSS background images.
Will allow closing koreader/koreader#6345 (comment) .

`Fix elements cancelling inherited "white-space: pre"`

Will allow closing koreader/koreader#6346 (comment) .

`initNodeStyle(): skip some possibly costly validation`

Some optimisation, mostly only noticable with large TXT documents. See koreader/koreader#3646 (comment) .

This change is

Includes some small refactoring and changes needed for CoolReader Android build.

Not really needed, and avoid some checks.

Get rid of LTEXT_RUNIN_FLAG, and instead fetch the alignment flags from the next node early. Mostly only used with FB2 footnotes to bring two block nodes together. This fixes FB2 footnotes lost alignment since 49f8250 and 28ae22c.

For better support of the image original colors and transparency.

Including keywords "contain" and "cover". Remove parsed but unimplemented "background-attachment", which would anyway not make much sense to support: we behave as "background-attachment: scroll".

Just a matter of accepting this when parsing "background:" and "background-image:". What's been added to support <img src="data:image/png;base64,...> by c35d1b8 will then handle it when drawing background.

All elements (except PRE-like ones) should start with "white-space: inherit" and not "white-space: normal", so they don't cancel "white-space: pre" when they are children of a <PRE>. Only the root node is set to "white-space: normal", thal all children will inherit from (except when they get a speficied white-space: property value).

Frenzie · 2020-07-05T16:42:57Z

crengine/include/lvstyles.h

@@ -278,7 +278,6 @@ enum lvdom_element_render_method
    erm_block,         ///< render as block element (render as containing other elements)
    erm_final,         ///< final element: render the whole it's content as single render block
    erm_inline,        ///< inline element
-    erm_runin,         ///< run-in (used as a solution to inline FB2 footnotes)


There's a Norse mythological bird named something like Runin — I can't quite think of the name right now. I didn't immediately realize it said run-in. :-)

Muninn, (one of) Odin's raven (Memory, IIRC).

Oh right, thanks. https://en.wikipedia.org/wiki/Huginn_and_Muninn

Frenzie · 2020-07-05T16:45:23Z

crengine/src/lvrend.cpp

+                if ( bg_w.type == css_val_unspecified && bg_h.type == css_val_unspecified ) {
+                    if ( bg_w.value == css_generic_contain && bg_h.value == css_generic_contain ) {
+                        // Image should be fully contained in container (no crop)
+                        int scale_w = 1024 * container_w / img_w;


It's reverted below when applied.
This is just to avoid int scale_w = 300 / 400 to be int 0 with int arithmetic.
Dunno if there's a proper way to do that :) @NiLuJe ?

Short of storing the scale as a proper float (which would imply also casting one of the DIV operands to float), not really.

See also https://en.wikipedia.org/wiki/Fixed-point_arithmetic & http://www.sunshine2k.de/articles/coding/fp/sunfp.html for more complex trickery (e.g., 16.16 is how FreeType does arithmetics).

So, I guess that we'll probably never have images or container larger than 65536px, so I guess I could use 65536 = 2^16 as the multiplier (instead of 1024 = 2^10), and we'll fit in a 32-bits int. Right ?

int scale_w = (container_w<<16) / img_w [...] new_h = (img_h * scale_w) >> 16;

Or there's some risk I don't see?

I mean, unless it's a really hot code path, I'd probably jump the gun and use a float, nothing we run on lacks an FPU.

i.e., float scale_w = container_w / (float) img_w;.

When going the other way around (i.e. int smthg = scale * int_somthg), the implicit cast will truncate by default, so you may need to ifloorf/iceilf/iroundf (which are C99 standard, and/or GCC/Clang builtins that return an int instead of a float).

Well, given that with rgrep float crengine/, I only see my stuff for supporting CSS floats, I'd say crengine is fully fixed-point arithmetic ! So, not going to introduce the first float variable :)
Ok, so it will be:

unsigned int scale_w = (container_w<<16) / img_w [...] new_h = (img_h * scale_w) >> 16;

See the overflow warnings above w/ 65536 ;).

OK, so, let's say the max image or contained width or height we'll meet will be 65535px :)

echo $(( (2 ** 16) * (2**15) )) 2147483648

or should I use <<15 or <<14 to be safer ?

Oh my, but in new_h = (img_h * scale_w) >> 16 , (img_h * scale_w) will overflow.
So, I guess it should be img_h * (scale_w >> 16) - but then I can't figure if it's useful or if the >>16 serves nothing :/

OK, letting the 1024 as-is. I'll rethink that when I have to - or never :)
And letting it as signed int - dunno if in some edge cases, I can have negative container widths...

Frenzie · 2020-07-05T16:47:01Z

crengine/src/lvtinydom.cpp

@@ -16124,11 +16124,16 @@ void ldomNode::initNodeStyle()
        {
            ldomNode * parent = getParentNode();

+            /* This has never triggered over the years, so trust we don't need it.
+             * This might also improves quite a bit TXT documents handling (where the main


also improves or might also improve ^_^

virxkane and others added 7 commits July 5, 2020 17:56

(Upstream) HyphMan cleanup

f64d682

Includes some small refactoring and changes needed for CoolReader Android build.

Rendering methods: remove erm_runin

e68bfee

Not really needed, and avoid some checks.

Simplify background image drawing

10ac343

For better support of the image original colors and transparency.

CSS: adds support for "background-size" property

53cd73c

Including keywords "contain" and "cover". Remove parsed but unimplemented "background-attachment", which would anyway not make much sense to support: we behave as "background-attachment: scroll".

CSS: support background:url("data:image/png;base64,...)

56b5ec0

Just a matter of accepting this when parsing "background:" and "background-image:". What's been added to support <img src="data:image/png;base64,...> by c35d1b8 will then handle it when drawing background.

poire-z changed the title ~~Bgimage runin~~ Background images enhancement, fix FB2 footnotes alignment Jul 5, 2020

poire-z changed the title ~~Background images enhancement, fix FB2 footnotes alignment~~ Background images enhancements, fix FB2 footnotes alignment Jul 5, 2020

This was referenced Jul 5, 2020

Question: fb2 in-page footnotes justified koreader/koreader#6344

Closed

PNG Image Displayed as black box koreader/koreader#6345

Closed

Formulas are not formatted koreader/koreader#6346

Closed

Frenzie approved these changes Jul 5, 2020

View reviewed changes

initNodeStyle(): skip some possibly costly validation

546b96d

poire-z force-pushed the bgimage_runin branch from 3fea520 to 546b96d Compare July 5, 2020 17:32

poire-z merged commit b4ff178 into koreader:master Jul 5, 2020

poire-z deleted the bgimage_runin branch July 5, 2020 19:01

This was referenced Jul 5, 2020

bump crengine: memory fixes, some speedup, background image enhancements koreader/koreader-base#1135

Merged

bump crengine: memory fixes, speedup, background image enhancements koreader/koreader#6350

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Background images enhancements, fix FB2 footnotes alignment #353

Background images enhancements, fix FB2 footnotes alignment #353

poire-z commented Jul 5, 2020 •

edited

Loading

Frenzie Jul 5, 2020

NiLuJe Jul 5, 2020 •

edited

Loading

Frenzie Jul 5, 2020

Frenzie Jul 5, 2020

poire-z Jul 5, 2020

NiLuJe Jul 5, 2020 •

edited

Loading

poire-z Jul 5, 2020

NiLuJe Jul 5, 2020 •

edited

Loading

poire-z Jul 5, 2020

NiLuJe Jul 5, 2020 •

edited

Loading

poire-z Jul 5, 2020

poire-z Jul 5, 2020 •

edited

Loading

poire-z Jul 5, 2020 •

edited

Loading

Frenzie Jul 5, 2020

Background images enhancements, fix FB2 footnotes alignment #353

Background images enhancements, fix FB2 footnotes alignment #353

Conversation

poire-z commented Jul 5, 2020 • edited Loading

(Upstream) HyphMan cleanup

Rendering methods: remove erm_runin

Text formatting: simplify 'display: run-in' handling

Simplify background image drawing

CSS: adds support for "background-size" property

CSS: support background:url("data:image/png;base64,...)

Fix elements cancelling inherited "white-space: pre"

initNodeStyle(): skip some possibly costly validation

Choose a reason for hiding this comment

NiLuJe Jul 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NiLuJe Jul 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NiLuJe Jul 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NiLuJe Jul 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

poire-z Jul 5, 2020 • edited Loading

Choose a reason for hiding this comment

poire-z Jul 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

poire-z commented Jul 5, 2020 •

edited

Loading

`(Upstream) HyphMan cleanup`

`Rendering methods: remove erm_runin`

`Text formatting: simplify 'display: run-in' handling`

`Simplify background image drawing`

`CSS: adds support for "background-size" property`

`CSS: support background:url("data:image/png;base64,...)`

`Fix elements cancelling inherited "white-space: pre"`

`initNodeStyle(): skip some possibly costly validation`

NiLuJe Jul 5, 2020 •

edited

Loading

NiLuJe Jul 5, 2020 •

edited

Loading

NiLuJe Jul 5, 2020 •

edited

Loading

NiLuJe Jul 5, 2020 •

edited

Loading

poire-z Jul 5, 2020 •

edited

Loading

poire-z Jul 5, 2020 •

edited

Loading