Fix bounding box returned by resvg_get_image_bbox

[jermy]

This was using abs_bounding_box before which, amongst other things, didn't include the full boundary box for text. This change swaps to using abs_layer_bounding_box which will return a bounding box including stroke, filters and text.

This will only return true in the same circumstances that the function previously did - since abs_layer_bounding_box() returns a non-zero rect, it can potentially be invalid (width/height set to 1) - so check if there is any valid bounding box, then return the correct one.

Relates to/Fixes #823

[jermy]

I'm not completely happy with this needing to check bounding box first, although there seems no other way right now. Is there a particular reason layer_bounding_box and abs_layer_bounding_box need to be NonZeroRects?

[RazrFalcon]

layer_bounding_box is NonZeroRect because you're expected to create a bitmap from it. And there is no point in creating a zero width/height bitmap. Let's say you have a horizontal line without a stroke. What height does it have? Zero. Nothing to render. But Wx0 is still a valid object bbox in SVG terms. That's why we have two Rect types.

I don't think we need a check here. Just replace abs_bounding_box with abs_layer_bounding_box and we're good to go.

[jermy]

The issue for me is that it's a change in behaviour for the C API - previously if there was nothing to render, then resvg_get_image_bbox returns false. If I always return abs_layer_bounding_box then sometimes it will return a spuriously positioned rectangle with a size of 1x1 that would be misleading at best. Is there any other way to determine if this is a valid image or not that I could use?

[RazrFalcon]

What do you mean? .abs_bounding_box().to_non_zero_rect() has the same logic as .abs_layer_bounding_box(). I don't understand the problem.

If you're rendering an SVG without specifying its size, it would fallback to 100x100 for empty images anyway. I don't see how you would get 1x1 image.

[RazrFalcon]

Oh, I see. We return 0x0x1x1 for empty SVGs... that's fun. Then in C API you can simply do tree.root().has_children() instead of abs_bounding_box. That should do it. Or just call resvg_is_image_empty beforehand from your code.

You're definitely doing weird things with SVG and we're not testing such cases. resvg is designed for a basic SVG->PNG scenario. Every other use case is on you.

[jermy]

I think there are some cases where a possibly valid SVG returns true from has_children() but doesn't return a bounding box. I'll see if I can directly find a SVG that does that, since I don't know if there is a bug there. In this case, I don't believe it should have been empty, and don't believe all the elements should have zero size either (in your "Let's say you have a horizontal line without a stroke" scenario).

[RazrFalcon]

I think there are some cases where a possibly valid SVG returns true from has_children() but doesn't return a bounding box.

I cannot think of any cases like that.

The only way to know if there are anything to render is to call abs_layer_bounding_box() to begin with, but it currently returns 0x0x1x1 for empty images... abs_bounding_box() can be zero, but we still can render stroke and filters. That's why it's followed by to_non_zero_rect().

Basically, right now there are no API to check if "SVG without size has anything to render". Not in Rust nor in C API. The question itself is a bit abstract in SVG terms.

Also, you have to remember that you cannot use resvg_get_image_size when your SVG has no size. Because the size recovery algorithm will use either abs_bounding_box or fallback to default values for empty images (100x100px). And that's by the spec. So you're already trying to go agains the spec here.

SVG is a mess...

[RazrFalcon]

Basically, resvg is working correctly right now. Yes, you get clipped text, stroke and filters, but that's how it suppose to be by the spec. And you want to get results that are against the spec and resvg doesn't support such case right now.

[RazrFalcon]

Right now, the only issue with resvg is that an empty group has layer_bounding_box set to 0x0x1x1, which would be pretty hard to change.

And also we have to expose abs_layer_bounding_box to C API.

The main issue here is documentation, not the code...

[RazrFalcon]

I have pushed my solution. Hope it helps.

Once again, the main issue here is that you misunderstand how SVG without size behaves. And I don't blame you. It's not what anyone would expect.

More details: https://razrfalcon.github.io/notes-on-svg-parsing/auto-size.html Basically, do not try doing anything weird with SVG. It will never work as you would expect.

[jermy]

So this might be some other issue, but does provide invalid behaviour with your change.

To keep things simple, I've got a patch to your C example which looks like this:

--- a/crates/c-api/examples/cairo/example.c
+++ b/crates/c-api/examples/cairo/example.c
@@ -16,6 +16,7 @@ int main(int argc, char **argv)

     resvg_options *opt = resvg_options_create();
     resvg_options_load_system_fonts(opt);
+    resvg_options_load_font_file(opt, "Pacifico.ttf");

     resvg_render_tree *tree;
     int err = resvg_parse_tree_from_file(argv[1], opt, &tree);
@@ -26,18 +27,41 @@ int main(int argc, char **argv)
         abort();
     }

+    resvg_rect bbox;
+    if (resvg_get_image_bbox(tree, &bbox))
+    {
+        printf("Got image_bbox with x=%f y=%f width=%f height=%f\n", bbox.x, bbox.y, bbox.width, bbox.height);
+    }
+    else
+    {
+        printf("Didn't get image_bbox\n");
+    }
+
+    if (resvg_get_object_bbox(tree, &bbox))
+    {
+        printf("Got object_bbox with x=%f y=%f width=%f height=%f\n", bbox.x, bbox.y, bbox.width, bbox.height);
+    }
+    else
+    {
+        printf("Didn't get object_bbox\n");
+    }
+
+    const int ZOOM = 4;
     resvg_size size = resvg_get_image_size(tree);
-    int width = (int)size.width;
-    int height = (int)size.height;
+    int width = (int)size.width * ZOOM;
+    int height = (int)size.height * ZOOM;

     cairo_surface_t *surface = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, width, height);

     /* resvg doesn't support stride, so cairo_surface_t should have no padding */
-    assert(cairo_image_surface_get_stride(surface) == (int)size.width * 4);
+    assert(cairo_image_surface_get_stride(surface) == width * 4);

     unsigned char *surface_data = cairo_image_surface_get_data(surface);

-    resvg_render(tree, resvg_transform_identity(), width, height, (char*)surface_data);
+    resvg_transform t = resvg_transform_identity();
+    t.a = ZOOM;
+    t.d = ZOOM;
+    resvg_render(tree, t, width, height, (char*)surface_data);

     /* RGBA -> BGRA */
     for (int i = 0; i < width * height * 4; i += 4)

Where Pacifico is the font mentioned in #823

Then I've got two SVGs - textedl.before.svg:

<svg xmlns="http://www.w3.org/2000/svg">
    <text fill="Yellow" font-family="Pacifico" font-size="20">
        Hello World
    </text>
</svg>

and textedl.after.svg which has a viewbox:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="-0.3199999928474426 -18.799999237060547 116.59999084472656 19.01999855041504">
    <text fill="Yellow" font-family="Pacifico" font-size="20">
        Hello World
    </text>
</svg>

Then this is the output I get from converting both SVGs:

+ ./example textedl.before.svg textedl.before.png
Got image_bbox with x=-0.320000 y=-18.799999 width=116.599991 height=19.019999
Got object_bbox with x=0.000000 y=-26.059999 width=113.959991 height=35.119999
+ ./example textedl.after.svg textedl.after.png
Got image_bbox with x=0.320000 y=18.799999 width=1.000000 height=1.000000
Didn't get object_bbox

I can try to dig into this further, but thought it would be good to get your view - has_children() is non-zero, but it still returns a 0x0x1x1 bounding box (translated by abs_transform). Whilst SVG sizing is weird and unpredictable etc etc, I should expect this image to have a bounding box, right?

[RazrFalcon]

Looks like bbox propagation is broken... Investigating.

[RazrFalcon]

How about now?

[jermy]

Thanks - that certainly seems to be giving the details I'd expect:

+ ./example textedl.before.svg textedl.before.png
Got image_bbox with x=-0.320000 y=-18.799999 width=116.599991 height=19.019999
Got object_bbox with x=0.000000 y=-26.059999 width=113.959991 height=35.119999
+ ./example textedl.after.svg textedl.after.png
Got image_bbox with x=0.000000 y=0.000000 width=116.599991 height=19.019999
Got object_bbox with x=0.320000 y=-7.260000 width=113.959991 height=35.119999

After graphic directly from the example script:

And because that's a bit truncated, here's another adding ceil(size.width) and ceil(size.height) to that example code: