Help implementing LayoutImage::render() (with external image rendering support)

[poire-z]

Hi @sammycage , thank you for this nice library, I'm having fun and for now quite some success integrating it in our EPUB reading application: https://github.com/koreader/koreader/issues/9298#issuecomment-1214427853 We have some good text rendering stack and image drawing stuff, so I'm trying to extend LunaSVG to support <text> and <tspan> and <image> by having it call back into our existing EPUB library. I've been succesful with text and tspan, but I'm a bit stuck with images. I'm requesting your help :)

I added an imagelement.cpp that builds a LayoutImage object, by just parsing x/y/width/height and href and setting them as attributes of the LayoutImage. I'm currently stuck in LayoutImage::render() - and I have a hard time getting into your RenderState/Canvas/plutovg stuff :/ (understanding in which coordinates they work in, which handle translation & rotation, etc...) It looks to me that I can't use canvas->blend() to have any kind of rotation, and that my only solution is to use ->setTexture() and ->fill() - and that I should use newState.beginGroup/endGroup() if I want any clipping to be ensured - but I can't have it all to work nicely (and when it seems to look good, resizing makes it bad again :/) I hope that for you, there's an easy and obvious solution to my problem, and that you can share your intuition. How would you go from this (that works) to get this horizontal rectangle to be properly drawn with all this <image> and its parents transformations & clips ?:

    auto img = Canvas::create(0,  0, width, height);
    if ( external_context_helper->draw_image_func ) {
        external_context_helper->draw_image_func(external_context_helper, url.c_str(),
                                    img->data(), width, height);
        // this properly fills the img canvas with the requested image content scaled to that canvas
    }

I'm sharing my current mess, if you need more context - but I'm more interested in your from-scratch-approach to solving this problem :)

void LayoutImage::render(RenderState& state) const
{
    if(visibility == Visibility::Hidden)
        return;

    /* This at least displays something
    BlendInfo info{clipper, masker, opacity, Rect::Invalid};
    RenderState newState(this, state.mode());
    // auto transform = this->transform;
    //transform.translate(x, y);
    newState.transform = transform * state.transform;
    newState.transform.translate(x, y);
    //newState.transform.translate(100, 100);
    //newState.transform.scale(2, 2);
    // newState.beginGroup(state, info);
    int w = static_cast<int>(ceil(width));
    int h = static_cast<int>(ceil(height));
    newState.canvas = Canvas::create(0, 0, width, height);

    bool drawn = false;
    if ( external_context_helper->draw_image_func ) {
        drawn = external_context_helper->draw_image_func(external_context_helper, url.c_str(),
                                    newState.canvas->data(), width, height);
    }
    if ( !drawn)
        return;
    Path path;
    path.rect(x, y, width, height, 0, 0);
    // XXX not good with TextureType::Tiled, not much with TextureType::Plain
    state.canvas->setTexture(newState.canvas.get(), TextureType::Tiled, state.transform);
    state.canvas->fill(path, newState.transform, WindRule::NonZero, BlendMode::Src_Over, opacity);

    // newState.endGroup(state, info);
    */
    BlendInfo info{clipper, masker, opacity, Rect::Invalid};
    RenderState newState(this, state.mode());
    // auto transform = this->transform;
    //transform.translate(x, y);
    newState.transform = transform * state.transform;
//    newState.transform.translate(x, y);
    //newState.transform.translate(100, 100);
    //newState.transform.scale(2, 2);
//    newState.beginGroup(state, info);
    int w = static_cast<int>(ceil(width));
    int h = static_cast<int>(ceil(height));

    auto box = newState.transform.map(strokeBoundingBox());
    newState.canvas = Canvas::create(box);
    auto img = Canvas::create(0,  0, width, height);

    bool drawn = false;
    if ( external_context_helper->draw_image_func ) {
        drawn = external_context_helper->draw_image_func(external_context_helper, url.c_str(),
                                    img->data(), width, height);
    }
    if ( !drawn)
        return;
    Path path;
    path.rect(x, y, width, height, 0, 0);
    // XXX not good with TextureType::Tiled, not much with TextureType::Plain
    // newState.canvas->setTexture(img.get(), TextureType::Tiled, newState.transform);
    newState.canvas->setTexture(img.get(), TextureType::Tiled, state.transform); // mostly ok, but may be by chance
    newState.canvas->fill(path, newState.transform, WindRule::NonZero, BlendMode::Src, opacity);
    state.canvas->blend(newState.canvas.get(), BlendMode::Src_Over, 1.0);

 //   newState.endGroup(state, info);
}

And this a sample with the various bad stuff I get (mis-rotation, black content in some case, bad positionnning, unwanted tiling/bad sizing :) Firefox | KOReader

Any thoughts ?

---

Btw, unrelated, but trying SVGs samples from https://commons.wikimedia.org/wiki/User:JoKalliauer/SVG_test_suites/resvg_Issues_details, I got LunaSVG (or at least, my use of it, but I got the crash without my added stuff) to crash with this image: https://upload.wikimedia.org/wikipedia/commons/8/87/Flag-map_of_the_world.svg

#0  0x00007ffff7ccfce1 in raise () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00007ffff7cb9537 in abort () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x00007ffff7cb940f in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#3  0x00007ffff7cc8662 in __assert_fail () from /lib/x86_64-linux-gnu/libc.so.6
#4  0x00007fffe8789dfd in ft_stroke_border_export (border=border@entry=0x555556d31ef0, outline=outline@entry=0x5555594f3370)
    at lunasvg/3rdparty/software/sw_ft_stroker.c:666
#5  0x00007fffe878ae7e in SW_FT_Stroker_ExportBorder (stroker=stroker@entry=0x555556d31e80, border=border@entry=SW_FT_STROKER_BORDER_LEFT, outline=outline@entry=0x5555594f3370)
    at lunasvg/3rdparty/software/sw_ft_stroker.c:1744
#6  0x00007fffe878ae9a in SW_FT_Stroker_Export (stroker=0x555556d31e80, outline=outline@entry=0x5555594f3370)
    at lunasvg/3rdparty/software/sw_ft_stroker.c:1752
#7  0x00007fffe8790342 in plutovg_rle_rasterize (rle=<optimized out>, path=<optimized out>, matrix=matrix@entry=0x555555e4fd70, clip=clip@entry=0x55555898dc20,
    stroke=stroke@entry=0x555555e4fda8, winding=winding@entry=plutovg_fill_rule_non_zero)
    at lunasvg/3rdparty/plutovg/plutovg-rle.c:257
#8  0x00007fffe878bb32 in plutovg_stroke_preserve (pluto=pluto@entry=0x55555898dbf0)
    at lunasvg/3rdparty/plutovg/plutovg.c:472
#9  0x00007fffe878bb61 in plutovg_stroke (pluto=0x55555898dbf0)
    at lunasvg/3rdparty/plutovg/plutovg.c:431
#10 0x00007fffe87792bc in lunasvg::Canvas::stroke (this=0x555557982150, path=..., transform=..., width=0.5, cap=lunasvg::LineCap::Butt, join=lunasvg::LineJoin::Miter,
    miterlimit=3.9828999999999999, dash=..., mode=lunasvg::BlendMode::Src_Over, opacity=1)
    at lunasvg/source/canvas.cpp:126
#11 0x00007fffe8773cb6 in lunasvg::StrokeData::stroke (this=this@entry=0x555556731958, state=..., path=...)
    at lunasvg/source/layoutcontext.cpp:344
#12 0x00007fffe87757b1 in lunasvg::LayoutShape::render (this=0x5555567318b0, state=...)
    at lunasvg/source/layoutcontext.cpp:409
#13 0x00007fffe877368b in lunasvg::LayoutContainer::renderChildren (this=this@entry=0x55555673a3a0, state=...)
    at lunasvg/source/layoutcontext.cpp:88
#14 0x00007fffe8775405 in lunasvg::LayoutGroup::render (this=0x55555673a3a0, state=...)
    at lunasvg/source/layoutcontext.cpp:179
#15 0x00007fffe877368b in lunasvg::LayoutContainer::renderChildren (this=this@entry=0x55555996d4c0, state=...)
    at lunasvg/source/layoutcontext.cpp:88
#16 0x00007fffe8775405 in lunasvg::LayoutGroup::render (this=0x55555996d4c0, state=...)
    at lunasvg/source/layoutcontext.cpp:179
#17 0x00007fffe877368b in lunasvg::LayoutContainer::renderChildren (this=this@entry=0x55555670db30, state=...)
    at lunasvg/source/layoutcontext.cpp:88
#18 0x00007fffe8775405 in lunasvg::LayoutGroup::render (this=0x55555670db30, state=...)
    at lunasvg/source/layoutcontext.cpp:179
#19 0x00007fffe877368b in lunasvg::LayoutContainer::renderChildren (this=this@entry=0x55555670da70, state=...)
    at lunasvg/source/layoutcontext.cpp:88
#20 0x00007fffe8775405 in lunasvg::LayoutGroup::render (this=0x55555670da70, state=...)
    at lunasvg/source/layoutcontext.cpp:179
#21 0x00007fffe877368b in lunasvg::LayoutContainer::renderChildren (this=this@entry=0x55555994efc0, state=...)
    at lunasvg/source/layoutcontext.cpp:88
#22 0x00007fffe877529d in lunasvg::LayoutSymbol::render (this=0x55555994efc0, state=...)
    at lunasvg/source/layoutcontext.cpp:159
#23 0x00007fffe8761648 in lunasvg::Document::render (this=this@entry=0x5555599e4390, bitmap=..., matrix=...)
    at lunasvg/source/lunasvg.cpp:378
#24 0x00007fffe8761871 in lunasvg::Document::renderToBitmap (this=0x5555599e4390, width=562, height=281, backgroundColor=backgroundColor@entry=0)
    at lunasvg/source/lunasvg.cpp:403
#25 0x00007fffe8a3d424 in LVSvgImageSource::DecodeFromBuffer (this=this@entry=0x55555694e640,

[sammycage]

Great... Can you send me the full source code? Also, since lunasvg is getting more attention, I'll start working on it.

[poire-z]

The full source code would be all of koreader :) and some ugly work in progress on its rendering engine https://github.com/koreader/crengine What I can share:

• my current patch on top of your current LunaSVG: lunasvg-koreader20220822.patch.txt
• the addition to our image decoding code https://github.com/koreader/crengine/blob/master/crengine/src/lvimg.cpp that uses lunasvg, just so you can see how I use the external_context_helper (meaning both a context and a helper): lvimg.cpp.txt

There are other changes to our text rendering stack to output path d= for each glyph, that depends on much of it (and it using FreeType, Harfbuzz), that I could share if you're wishing to build koreader - but I guess you don't want to be involved in that :)

My idea was to patch at minima LunaSVG, and put my stuff in files names x* so there's minimal conflict with your future additions.

Also, since lunasvg is getting more attention, I'll start working on it.

I wouldn't dare propose a PR with my stuff, because I'm not very good with C++ and your code base looks very neat and tidy, full of proper const and std:: stuff I'm not confortable with :)

But if you're going to add support for text and images, I guess you might not want to include a full font management and text rendering stack (meaning needing Freetype and Harfbuzz) and image decoding libraries (gif, jpg, png...) - so, it would be nice if you could go with something like my initial idea of delegating this to some external helpers - you could provide a simple implementations (a single font text stack linking against harfbuzz would be easy) - that would allow external users of LunaSVG to provide something more complex.

Hoping you still can give me some hints on how to go with LayoutImage - I'm quite frustrated and impatient having that working :) Thanks.

[sammycage]

Hoping you still can give me some hints on how to go with LayoutImage

Sure... I'll give you some tips before the weekend.

[sammycage]

@poire-z

void LayoutImage::render(RenderState& state) const
{
    if(visibility == Visibility::Hidden)
        return;

    BlendInfo info{clipper, masker, opacity, Rect::Invalid};
    RenderState newState(this, state.mode());
    newState.transform = transform * state.transform;
    newState.beginGroup(state, info);

    Rect srcRect(0, 0, image->width(), image->height());
    Rect dstRect(x, y, width, height);
    // preserveAspectRatio.transformRect(srcRect, dstRect);

    auto scalex = srcRect.w / dstRect.w;
    auto scaley = srcRect.h / dstRect.h;

    Transform transform(scalex, 0, 0, scaley, srcRect.x, srcRect.y);
    Path path;
    path.rect(dstRect.x, dstRect.y, dstRect.w, dstRect.h, 0, 0);

    newState.canvas->setTexture(image.get(), TextureType::Plain, transform);
    newState.canvas->fill(path, newState.transform, WindRule::NonZero, BlendMode::Src_Over, 1.0);

    newState.endGroup(state, info);
}

[sammycage]

Fixed : https://upload.wikimedia.org/wikipedia/commons/8/87/Flag-map_of_the_world.svg

[poire-z]

Thank you ! This works wonderfully with all my test cases !

(Trying to understand it, and map what needed to be changed in my attemps, it comes down to this:

-    newState.canvas->setTexture(img.get(), TextureType::Tiled, state.transform); // mostly ok, but may be by chance - I also tried using newState.transform :)
+    newState.canvas->setTexture(img.get(), TextureType::Plain, Transform{1,0,0,1,0,0});

which wasn't much :) but I couldn't have find it alone :) these transformations and their maths are quite over my head...) But your code takes care of x and y that I was ignoring for the moment, so I'll use it !

You also took care of any scaling in case the image was not the requested size - it would be in my case - but may be I shouldn't scale it myself as then I could uncomment your preserveAspectRatio line and have lunasvg ensure it and that it's centered and not distorted - would that be enough? I'll have to try that.

I tried requesting half the final size from my draw_image_func, and it looks these:

    auto scalex = srcRect.w / dstRect.w;
    auto scaley = srcRect.h / dstRect.h;

should be inverted: scalex = dstRect.w / srcRect.w.

Also, do I need more/different than that for these:

Rect LayoutImage::map(const Rect& rect) const
{
    return transform.map(rect);
}

const Rect& LayoutImage::fillBoundingBox() const
{
    if(m_fillBoundingBox.valid())
        return m_fillBoundingBox;

    m_fillBoundingBox = Rect{x, y, width, height};
    return m_fillBoundingBox;
}

const Rect& LayoutImage::strokeBoundingBox() const
{
    if(m_strokeBoundingBox.valid())
        return m_strokeBoundingBox;

    m_strokeBoundingBox = Rect{x, y, width, height};
    return m_strokeBoundingBox;
}

I think I'm quite done on the lunasvg side with the patch I posted above and your addition - and I can continue working on the code on the EPUB/HTML side.

Any thoughts on the other stuff you may have read in that patch?:

• my StyledElement::font_size() to compute the inherited font size
• handling units in em with a ref font size of 16 (the more correct alternative would be to use more often value(element, mode) instead of value(max_for_percent) to get the element and its font size for computing em, but this is expensive, and a lot of changes, so I'm fine not having it perfect)
• forcing a final size from the outside to have nice rasterization without needing any downscaling (we force images to fit in the page/screen) (in SVGElement::layoutDocument())

Again, thank you for this, and the crash fix (which makes me confident in LunaSVG to not crash our engine - none of the other 60 images from that wikimedia test suite did cause a crash).

[sammycage]

forcing a final size from the outside to have nice rasterization without needing any downscaling

I don't understand.. Can you please give me an example <3

[poire-z]

Ok :) (lots of things I had to fight with and try to understand, trying to remember that one...)

Let's say we have a SVG image that announces itself to be 1000x500. It has some CSS (on the HTML side) that sets it max-width: 95%, max-height: 95% so it fits in the page. Let's work with max-width: 400px; max-height: 400px, ie: my test svg has:

<svg xmlns:svg="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" class="imageobject" version="1.1" id="Layer_1" width="43.7841739130435em" height="17.9907826086957em" viewBox="0 0 503.518 206.894" xml:space="preserve" preserveAspectRatio="xMaxYMax meet" style="max-width: 400px; max-height: 400px; border: 1px solid blue">

On my side, I compute the final size of the image, taking into account the native image size and the CSS properties. The algo for ensuring max/min-width/height, when both max-width and max-height are specified, doesnt say the aspect ratio has to be kept. So, I end up with say a target image size of 400x400. I was then naively using: bitmap = doc->renderToBitmap(400, 400, 0x00000000); but this would distort the image to make it be 400x400, ie.: or (depending on if I scale it or not, a bit confused right now trying to get the previous behaviour I was seeing :) but anyway, these 2 renderings are the only 2 things I managed to get without my trick).

By having me providing a forced width/height (my exact target size) and LayoutDocument "hack"/override the width/height/viewport, I can get LunaSVG to give me this, ensuring preserveAspectRatio="xMaxYMax meet" (the image is too tall, haven't thought much if I would need to do better, having the full image and the aspect ratio kept was good enough for me :)

Anyway, I figure I have some added constraints with using SVG images in an HTML context and screen pages, and having to make them fit in a page - that you/LunaSVG itself maybe shouldn't have to worry about (?).

[poire-z]

Fixed : https://upload.wikimedia.org/wikipedia/commons/8/87/Flag-map_of_the_world.svg

This image did not crash when I had it sized small (and this is done rather quickly, less than 2 seconds):

refName svgs/testsuite/67.Flag-map_of_the_world.svg
found 1 in _urlImageMap svgs/testsuite/67.Flag-map_of_the_world.svg
lvtextfm draw image 0 2400 1200
DrawCallback (scaleble: 1) 2400 1200 > 562 281
LVSvgImageSource::DecodeFromBuffer target from DrawCallback 562 281
SVG: parsing...
LVSvgImageSource::DecodeFromBuffer 562 281
(lunasvg working)
LVSvgImageSource::Decode res 1

But when having it sized a bit larger, I get another crash after 2 minutes:

refName svgs/testsuite/67.Flag-map_of_the_world.svg
found 1 in _urlImageMap svgs/testsuite/67.Flag-map_of_the_world.svg
lvtextfm draw image 0 2400 1200
DrawCallback (scaleble: 1) 2400 1200 > 1328 664
LVSvgImageSource::DecodeFromBuffer target from DrawCallback 1328 664
SVG: parsing...
LVSvgImageSource::DecodeFromBuffer 1328 664

Thread 1 "luajit" received signal SIGSEGV, Segmentation fault.
#0  gray_find_cell (worker=worker@entry=0x7fffffffac70)
    at lunasvg/3rdparty/software/sw_ft_raster.c:427
#1  0x00007fffe8798a17 in gray_record_cell (worker=worker@entry=0x7fffffffac70)
    at lunasvg/3rdparty/software/sw_ft_raster.c:452
#2  0x00007fffe879908b in gray_set_cell (worker=worker@entry=0x7fffffffac70, ex=ex@entry=8, ey=-4294967296, ey@entry=-4294967291)
    at lunasvg/3rdparty/software/sw_ft_raster.c:487
#3  0x00007fffe879938f in gray_render_line (worker=0x7fffffffac70, to_x=-8589932480, to_y=1420)
    at lunasvg/3rdparty/software/sw_ft_raster.c:623
#4  0x00007fffe8799825 in gray_line_to (to=<optimized out>, worker=<optimized out>)
    at lunasvg/3rdparty/software/sw_ft_raster.c:829
#5  0x00007fffe87986c4 in SW_FT_Outline_Decompose (outline=outline@entry=0x7fffffffb380, func_interface=func_interface@entry=0x7fffe88713a0 <func_interface>, user=user@entry=0x7fffffffac70)
    at lunasvg/3rdparty/software/sw_ft_raster.c:1097
#6  0x00007fffe8798a79 in gray_convert_glyph_inner (worker=worker@entry=0x7fffffffac70)
    at lunasvg/3rdparty/software/sw_ft_raster.c:1209
#7  0x00007fffe8798c90 in gray_convert_glyph (worker=worker@entry=0x7fffffffac70)
    at lunasvg/3rdparty/software/sw_ft_raster.c:1298
#8  0x00007fffe8798f7e in gray_raster_render (raster=<optimized out>, params=0x7fffffffbd50)
    at lunasvg/3rdparty/software/sw_ft_raster.c:1382
#9  0x00007fffe87a1258 in plutovg_rle_rasterize (rle=<optimized out>, path=<optimized out>, matrix=matrix@entry=0x555555e0a030, clip=clip@entry=0x555555e3d490,
    stroke=stroke@entry=0x555555e0a068, winding=winding@entry=plutovg_fill_rule_non_zero)
    at lunasvg/3rdparty/plutovg/plutovg-rle.c:262
#10 0x00007fffe879ca55 in plutovg_stroke_preserve (pluto=pluto@entry=0x555555e3d460)
    at lunasvg/3rdparty/plutovg/plutovg.c:472
#11 0x00007fffe879ca84 in plutovg_stroke (pluto=0x555555e3d460)
    at lunasvg/3rdparty/plutovg/plutovg.c:431
#12 0x00007fffe878a20e in lunasvg::Canvas::stroke (this=0x555556241fe0, path=..., transform=..., width=0.61043000000000003, cap=lunasvg::LineCap::Butt, join=lunasvg::LineJoin::Miter,
    miterlimit=4, dash=..., mode=lunasvg::BlendMode::Src_Over, opacity=1)
    at lunasvg/source/canvas.cpp:126
#13 0x00007fffe8784d20 in lunasvg::StrokeData::stroke (this=this@entry=0x5555577d7c28, state=..., path=...)
    at lunasvg/source/layoutcontext.cpp:344
#14 0x00007fffe878681b in lunasvg::LayoutShape::render (this=0x5555577d7b80, state=...)
    at lunasvg/source/layoutcontext.cpp:409

[sammycage]

Anyway, I figure I have some added constraints with using SVG images in an HTML context and screen pages, and having to make them fit in a page - that you/LunaSVG itself maybe shouldn't have to worry about (?).

Wow

But when having it sized a bit larger, I get another crash after 2 minutes:

I will try to fix the crash

[poire-z]

Closing, as I got my help :) Thank you.

[poire-z]

@sammycage : I think you could add proper support for units/lengths in em/ex to LunaSVG (even if it doesn't support text and font, there is a default value for font-size of 16, and font-size can be specified on elements and has some effect on lengths) with something like the following (cleaned up manually from my larger current patch, so it may need small tweaks to apply):

diff --git a/source/element.h b/source/element.h
index f94f927..9740569 100644
--- a/source/element.h
+++ b/source/element.h
@@ -48,7 +52,8 @@ enum class PropertyId
     Fill_Opacity,
     Fill_Rule,
+    Font_Size,
     Fx,
     Fy,
@@ -141,4 +155,5 @@ public:
     virtual bool isPaint() const { return false; }
     virtual bool isGeometry() const { return false; }
+    virtual bool isStyled() const { return false; }
     virtual void layout(LayoutContext*, LayoutContainer*) const;
     virtual std::unique_ptr<Node> clone() const = 0;
diff --git a/source/parser.cpp b/source/parser.cpp
index cda095a..8acbe57 100644
--- a/source/parser.cpp
+++ b/source/parser.cpp
@@ -1101,4 +1110,5 @@ static const std::map<std::string, PropertyId> csspropertymap = {
     {"fill-opacity", PropertyId::Fill_Opacity},
     {"fill-rule", PropertyId::Fill_Rule},
+    {"font-size", PropertyId::Font_Size},
     {"marker-end", PropertyId::Marker_End},
     {"marker-mid", PropertyId::Marker_Mid},
diff --git a/source/property.cpp b/source/property.cpp
index 97ddf37..c887ca4 100644
--- a/source/property.cpp
+++ b/source/property.cpp
@@ -561,5 +561,5 @@ Length::Length(double value, LengthUnits units)
 static const double dpi = 96.0;

-double Length::value(double max) const
+double Length::value(double max, double font_size) const
 {
     switch(m_units) {
@@ -579,4 +579,12 @@ double Length::value(double max) const
     case LengthUnits::Percent:
         return m_value * max / 100.0;
+    case LengthUnits::Em:
+        // Most usage is via LengthContext::valueForLength(), which gets provided
+        // the element and will find its computed inherited font size.
+        // Otherwise, use the default value font_size=16 (defined in property.h),
+        // which is sparsely documented, but seems to be what Firefox uses.
+        return m_value * font_size;
+    case LengthUnits::Ex:
+        return m_value * font_size / 2;
     default:
         break;
@@ -598,4 +606,11 @@ double Length::value(const Element* element, LengthMode mode) const
         return m_value * max / 100.0;
     }
+    if(m_units == LengthUnits::Em || m_units == LengthUnits::Ex)
+    {
+        if ( element->isStyled() ) {
+            auto font_size = (static_cast<const StyledElement*>(element))->font_size();
+            return value(1.0, font_size);
+        }
+    }

     return value(1.0);
diff --git a/source/property.h b/source/property.h
index 52a07e5..d886fe1 100644
--- a/source/property.h
+++ b/source/property.h
@@ -272,5 +272,5 @@ public:
     Length(double value, LengthUnits units);

-    double value(double max) const;
+    double value(double max, double font_size=16) const;
     double value(const Element* element, LengthMode mode) const;

@@ -278,4 +278,5 @@ public:
     bool isZero() const { return m_value == 0.0; }
     bool isRelative() const { return m_units == LengthUnits::Percent || m_units == LengthUnits::Em || m_units == LengthUnits::Ex; }
+    LengthUnits getUnits() const { return m_units; }

     static const Length Unknown;
diff --git a/source/styledelement.cpp b/source/styledelement.cpp
index bfb5ad8..1c49878 100644
--- a/source/styledelement.cpp
+++ b/source/styledelement.cpp
@@ -9,4 +9,33 @@ StyledElement::StyledElement(ElementId id)
 }

+double StyledElement::font_size() const
+{
+    // font-size can't just be find() among ancestors. If it is in relative units,
+    // it should scale ancestors' font sizes.
+    double factor = 1.0;
+    auto element = (Element*)this;
+    do {
+        auto& value = element->get(PropertyId::Font_Size);
+        if (!value.empty()) {
+            Length size = Parser::parseLength(value, ForbidNegativeLengths, Length::Unknown);
+            if ( size.isValid() ) {
+                LengthUnits units = size.getUnits();
+                if ( units == LengthUnits::Percent ) {
+                    factor *= size.value(1.0);
+                }
+                else if ( units == LengthUnits::Em || units == LengthUnits::Ex ) {
+                    factor *= size.value(1.0, 1.0);
+                }
+                else { // Absolute units: apply factor as made up to here
+                    return size.value(1.0) * factor;
+                }
+            }
+        }
+        element = element->parent;
+    } while(element);
+    // No absolute value met in ancestors: use the default SVG font size of 16 as the reference
+    return 16 * factor;
+}
+
 Paint StyledElement::fill() const
 {
diff --git a/source/styledelement.h b/source/styledelement.h
index 1257548..d3ee608 100644
--- a/source/styledelement.h
+++ b/source/styledelement.h
@@ -11,4 +11,8 @@ public:
     StyledElement(ElementId id);

+    bool isStyled() const { return true; }
+
+    double font_size() const;
+
     Paint fill() const;
     Paint stroke() const;

Feel free to pick it or rewrite it in a neater way :) This seems to match what Firefox does when I throw various <line x1="2em" x2="3ex"...>, and it's better than having coordinates and lengths computed to 0 and stuff not shown.

[sammycage]

Fixed : https://upload.wikimedia.org/wikipedia/commons/8/87/Flag-map_of_the_world.svg

This image did not crash when I had it sized small (and this is done rather quickly, less than 2 seconds):

refName svgs/testsuite/67.Flag-map_of_the_world.svg
found 1 in _urlImageMap svgs/testsuite/67.Flag-map_of_the_world.svg
lvtextfm draw image 0 2400 1200
DrawCallback (scaleble: 1) 2400 1200 > 562 281
LVSvgImageSource::DecodeFromBuffer target from DrawCallback 562 281
SVG: parsing...
LVSvgImageSource::DecodeFromBuffer 562 281
(lunasvg working)
LVSvgImageSource::Decode res 1

But when having it sized a bit larger, I get another crash after 2 minutes:

refName svgs/testsuite/67.Flag-map_of_the_world.svg
found 1 in _urlImageMap svgs/testsuite/67.Flag-map_of_the_world.svg
lvtextfm draw image 0 2400 1200
DrawCallback (scaleble: 1) 2400 1200 > 1328 664
LVSvgImageSource::DecodeFromBuffer target from DrawCallback 1328 664
SVG: parsing...
LVSvgImageSource::DecodeFromBuffer 1328 664

Thread 1 "luajit" received signal SIGSEGV, Segmentation fault.
#0  gray_find_cell (worker=worker@entry=0x7fffffffac70)
    at lunasvg/3rdparty/software/sw_ft_raster.c:427
#1  0x00007fffe8798a17 in gray_record_cell (worker=worker@entry=0x7fffffffac70)
    at lunasvg/3rdparty/software/sw_ft_raster.c:452
#2  0x00007fffe879908b in gray_set_cell (worker=worker@entry=0x7fffffffac70, ex=ex@entry=8, ey=-4294967296, ey@entry=-4294967291)
    at lunasvg/3rdparty/software/sw_ft_raster.c:487
#3  0x00007fffe879938f in gray_render_line (worker=0x7fffffffac70, to_x=-8589932480, to_y=1420)
    at lunasvg/3rdparty/software/sw_ft_raster.c:623
#4  0x00007fffe8799825 in gray_line_to (to=<optimized out>, worker=<optimized out>)
    at lunasvg/3rdparty/software/sw_ft_raster.c:829
#5  0x00007fffe87986c4 in SW_FT_Outline_Decompose (outline=outline@entry=0x7fffffffb380, func_interface=func_interface@entry=0x7fffe88713a0 <func_interface>, user=user@entry=0x7fffffffac70)
    at lunasvg/3rdparty/software/sw_ft_raster.c:1097
#6  0x00007fffe8798a79 in gray_convert_glyph_inner (worker=worker@entry=0x7fffffffac70)
    at lunasvg/3rdparty/software/sw_ft_raster.c:1209
#7  0x00007fffe8798c90 in gray_convert_glyph (worker=worker@entry=0x7fffffffac70)
    at lunasvg/3rdparty/software/sw_ft_raster.c:1298
#8  0x00007fffe8798f7e in gray_raster_render (raster=<optimized out>, params=0x7fffffffbd50)
    at lunasvg/3rdparty/software/sw_ft_raster.c:1382
#9  0x00007fffe87a1258 in plutovg_rle_rasterize (rle=<optimized out>, path=<optimized out>, matrix=matrix@entry=0x555555e0a030, clip=clip@entry=0x555555e3d490,
    stroke=stroke@entry=0x555555e0a068, winding=winding@entry=plutovg_fill_rule_non_zero)
    at lunasvg/3rdparty/plutovg/plutovg-rle.c:262
#10 0x00007fffe879ca55 in plutovg_stroke_preserve (pluto=pluto@entry=0x555555e3d460)
    at lunasvg/3rdparty/plutovg/plutovg.c:472
#11 0x00007fffe879ca84 in plutovg_stroke (pluto=0x555555e3d460)
    at lunasvg/3rdparty/plutovg/plutovg.c:431
#12 0x00007fffe878a20e in lunasvg::Canvas::stroke (this=0x555556241fe0, path=..., transform=..., width=0.61043000000000003, cap=lunasvg::LineCap::Butt, join=lunasvg::LineJoin::Miter,
    miterlimit=4, dash=..., mode=lunasvg::BlendMode::Src_Over, opacity=1)
    at lunasvg/source/canvas.cpp:126
#13 0x00007fffe8784d20 in lunasvg::StrokeData::stroke (this=this@entry=0x5555577d7c28, state=..., path=...)
    at lunasvg/source/layoutcontext.cpp:344
#14 0x00007fffe878681b in lunasvg::LayoutShape::render (this=0x5555577d7b80, state=...)
    at lunasvg/source/layoutcontext.cpp:409

Fixed

[poire-z]

Hi @sammycage ,

About the above stuff, I ended up with doing it that way: https://github.com/koreader/koreader-base/blob/c1d97b47abeca6eccd0c136e31749302cc84212f/thirdparty/lunasvg/xtended/ximageelement.cpp#L72-L137

I added some stuff in the middle of that highlighted code where I try to ensure preserveAspectRatio. When the image/texture is smaller, we should get it not filling the whole area. But (in some cases? I didn't notice that at the time, so it may not happen in all cases...), I may get:

or that is, the first or last pixels color is used to fill the area before or after, while it is expected this area should instead be blank/untouched.

If I would use TextureType::Tiled instead of Plain in newState.canvas->setTexture(image.get(), TextureType::Plain, transform);, I would get:

or

So, I believe my input drawn image is correct and does not carry these edge bleedings, and that it may be plutovg plain texture handling that does that bleeding. Is that its expected behaviour, or a bug ? Do you understand what may be happening ? Should I somehow ensure some mask/clipping on my side to avoid this bleeding? Or is my code just crappy and not the right way to go at ensuring preserveAspectRatio ? :/ (again, all this "transform" stuff is really white magic to me :) Thank you for any hint. (Related KOReader issue: https://github.com/koreader/koreader/issues/9766. This is not some regresssion, it happens with your August code and your current master.)

[poire-z]

I found the bit of code involved, which is doing some clamp(), so getting the first or last pixel from the texture when we are before or after it. I could solve my issue with:

--- a/3rdparty/plutovg/plutovg-blend.c
+++ b/3rdparty/plutovg/plutovg-blend.c
@@ -516,23 +516,32 @@ static void blend_transformed_argb(plutovg_surface_t* surface, plutovg_operator_

         int length = spans->len;
         const int coverage = (spans->coverage * texture->const_alpha) >> 8;
         while(length)
         {
             int l = plutovg_min(length, BUFFER_SIZE);
             const uint32_t* end = buffer + l;
             uint32_t* b = buffer;
             while(b < end)
             {
+                /*
                 int px = plutovg_clamp(x >> 16, 0, image_width - 1);
                 int py = plutovg_clamp(y >> 16, 0, image_height - 1);
                 *b = ((const uint32_t*)(texture->data + py * texture->stride))[px];
+                */
+                int px = x >> 16;
+                int py = y >> 16;
+                if ( px >= 0 && py >= 0 && px < image_width && py < image_height )
+                    *b = ((const uint32_t*)(texture->data + py * texture->stride))[px];
+                else
+                    *b = 0x00000000;
+

                 x += fdx;
                 y += fdy;
                 ++b;
             }

             func(target, l, buffer, coverage);
             target += l;
             length -= l;
         }

but I have no idea if the current behaviour with its clamp() is legitimate in other use cases... (And if such fixes are needed elsewhere - although I see no other plutovg_clamp() in this plutovg-blend.c.)

[sammycage]

Interesting... I will take a closer look at this

[poire-z]

Did you get a chance to take that closer look ? :) Or, if you feel the way plutovg handles that is the right way in its "plutovg project and aim" context, and that it is also just ok the way LunaSVG uses plutovg for now - as the issue is just with the way I use it in my little extensions to LunaSVG - do you feel using my above patch on my side could cause any issue with LunaSVG and its own use of plutovg (with stuff I haven't studied, like gradient, filling, patterns, clipping...) ?

I haven't noticed any problem with my test suite, but some feeling from you would reassure me I can go on with this patch. Thanks.

[sammycage]

do you feel using my above patch on my side could cause any issue with LunaSVG and its own use of plutovg (with stuff I haven't studied, like gradient, filling, patterns, clipping...) ?

No

I haven't noticed any problem with my test suite, but some feeling from you would reassure me I can go on with this patch.

Your patch is as clear as day.... You can go on with it.