Hey guys, I'm altering SPImage to support the 'preserveAspectRatio' attribute as per the SVG spec for raster images (although when its set to 'slice' it will assume 'overflow' is set to hidden). I should finish with this tomorrow (hopefully). Does this sound like something that you guys would be interested in integrated into Inkscape CVS?

Edward Flick

On Thu, 10 Feb 2005 17:44:56 -0600, Edward Flick <eddy@...690...> wrote:

Hey guys, I'm altering SPImage to support the 'preserveAspectRatio' attribute as per the SVG spec for raster images (although when its set to 'slice' it will assume 'overflow' is set to hidden). I should finish with this tomorrow (hopefully). Does this sound like something that you guys would be interested in integrated into Inkscape CVS?

If it's support for reading and rendering, sure. We want to be compliant to the standard.

If it's support for setting it via the UI, then I'm not so sure. I think it's one of the many provisions in the standard which really only make sense for manual authoring of SVG. That is, when you _write_ your SVG, you use this attribute to indicate your _intention_ which the renderer will obey according to the rules. However when you simply _draw_ your SVG, you don't need to indicate any intentions. You can just scale and clip your graphic exactly as you want it, interactively. Adding UI for setting "semantic" attributes like this one will make the interface quite messy and unpredictable, especially for novices. E.g. we will be buried by bug reports like "can't squeeze the image" from people who inadvertently set this attribute to uniform scaling only.

So, I would prefer that Inkscape could read and correctly render SVGs with this attribute, but not set it.

On Fri, 11 Feb 2005 09:34:57 -0600, Edward Flick <eddy@...690...> wrote:

I definetely see your point, but the reason I'm doing this is so Inkscape can also be used as a template engine, with no predetermination of image size involved in image element creation. Not that just modifying the SVG would be grossly complex, but for the people making the layouts, it would be harder (at least that is the case here).

I really don't see why it would be so difficult to scale an image with the mouse, optionally with Ctrl for uniform scaling. It's entirely consistent with all other object types and gives you no surprises. You see what you get, and it's entirely valid SVG.

What if there was just a drop down called "Scale" with options: "Stretch", "Fit (centered)", "Fit (top left)", "Fit (top)", "Fit (top right)","Fit (right)","Fit (bottom right)", "Fit (bottom)", "Fit (bottom left)", "Fit (left)", "Fill (centered)", "Fill (top left)", "Fill (top)", "Fill (top right)","Fill (right)","Fill (bottom right)", "Fill (bottom)", "Fill (bottom left)", and "Fill (left)"? I think these options could be set very quickly, and undoing them would be as easy as going back to "Stretch".

Wow, for one thing, you see yourself that there are a lot of options here. So it would be rather cumbersome if only due to the number of options. But there are other reasons too. This would prompt lots of questions: Centered relative to WHAT? Viewport? What is that? We don't have such a concept elsewhere. Can I visualize this viewport? How can I affect its size or placement? Why are all these options available only for images and not for other objects? How would they interact with regular scaling/alignment? Did I do anything wrong if I just stretched it with the mouse, as I would stretch a path? etc, etc. An interface that needs so much learning and explanations for such a simple and obvious task is not a good interface.

On the other hand, treating images just like any other objects that can be scaled, moved, aligned with the Align dialog, etc. has the huge advantage that it's consistent everywhere. You learn it once and you don't need to unlearn it for any special cases. Everything is an object and all objects behave the same. It's very important.

So, with regard to preserveAspectRatio support, what I would suggest is this:

Add a new tab to the Object properties dialog. Call it "Advanced". There, we would place all the "semantic" SVG attributes such as preserveAspectRatio (they would be enabled/disabled depending on the type of the selected object, of course). These attributes would be given with their exact SVG names and lists of values as per the standard, with no attempts to reword or simplify them (other than in floating tooltips). This is important because those who are curious can then easily find these attributes in the spec and read up on the cumbersome details of their meaning and interpretation - if they need it. This way, we would provide these facilities for those who really need them, but they won't get in the way of the regular users. Would that work for you?

Hello again, I posted this patch to sourceforge. And I've attached it here also. It implements preserveAspectRatio rendering functionality as per SVG 1.1 spec with overflow assumed to be hidden. It currently has no human interface outside of the built in XML editor. Anybody care to check this out? Also, the attached file is just the piped output from 'cvs diff', is this acceptable?

Edward Flick

Index: src/sp-image.cpp =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/sp-image.cpp,v retrieving revision 1.42 diff -r1.42 sp-image.cpp 26a27,29

//Added for preserveAspectRatio support, EAF #include "enums.h"

27a31

467a472

image->aspect_align = SP_ASPECT_NONE;

480a486

sp_object_read_attr (object, "preserveAspectRatio");

552a559,616

case SP_ATTR_PRESERVEASPECTRATIO: /* Do setup before, so we can use break to escape */ image->aspect_align = SP_ASPECT_NONE; image->aspect_clip = SP_ASPECT_MEET; object->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG | SP_OBJECT_VIEWPORT_MODIFIED_FLAG); if (value) { int len; gchar c[256]; const gchar *p, *e; unsigned int align, clip; p = value; while (*p && *p == 32) p += 1; if (!*p) break; e = p; while (*e && *e != 32) e += 1; len = e - p; if (len > 8) break; memcpy (c, value, len); c[len] = 0; /* Now the actual part */ if (!strcmp (c, "none")) { align = SP_ASPECT_NONE; } else if (!strcmp (c, "xMinYMin")) { align = SP_ASPECT_XMIN_YMIN; } else if (!strcmp (c, "xMidYMin")) { align = SP_ASPECT_XMID_YMIN; } else if (!strcmp (c, "xMaxYMin")) { align = SP_ASPECT_XMAX_YMIN; } else if (!strcmp (c, "xMinYMid")) { align = SP_ASPECT_XMIN_YMID; } else if (!strcmp (c, "xMidYMid")) { align = SP_ASPECT_XMID_YMID; } else if (!strcmp (c, "xMaxYMid")) { align = SP_ASPECT_XMAX_YMID; } else if (!strcmp (c, "xMinYMax")) { align = SP_ASPECT_XMIN_YMAX; } else if (!strcmp (c, "xMidYMax")) { align = SP_ASPECT_XMID_YMAX; } else if (!strcmp (c, "xMaxYMax")) { align = SP_ASPECT_XMAX_YMAX; } else { break; } clip = SP_ASPECT_MEET; while (*e && *e == 32) e += 1; if (e) { if (!strcmp (e, "meet")) { clip = SP_ASPECT_MEET; } else if (!strcmp (e, "slice")) { clip = SP_ASPECT_SLICE; } else { break; } } image->aspect_align = align; image->aspect_clip = clip; } break;

583a648,697

// preserveAspectRatio calculate bounds / clipping rectangle, EAF if (image->pixbuf && (image->aspect_align != SP_ASPECT_NONE)) { int imagewidth, imageheight; double x,y;

        imagewidth = gdk_pixbuf_get_width (image->pixbuf);
        imageheight = gdk_pixbuf_get_height (image->pixbuf);

	switch (image->aspect_align) {
	case SP_ASPECT_XMIN_YMIN:
		x = 0.0;
		y = 0.0;
		break;
	case SP_ASPECT_XMID_YMIN:
		x = 0.5;
		y = 0.0;
		break;
	case SP_ASPECT_XMAX_YMIN:
		x = 1.0;
		y = 0.0;
		break;
	case SP_ASPECT_XMIN_YMID:
		x = 0.0;
		y = 0.5;
		break;
	case SP_ASPECT_XMID_YMID:
		x = 0.5;
		y = 0.5;
		break;
	case SP_ASPECT_XMAX_YMID:
		x = 1.0;
		y = 0.5;
		break;
	case SP_ASPECT_XMIN_YMAX:
		x = 0.0;
		y = 1.0;
		break;
	case SP_ASPECT_XMID_YMAX:
		x = 0.5;
		y = 1.0;
		break;
	case SP_ASPECT_XMAX_YMAX:
		x = 1.0;
		y = 1.0;
		break;
	default:
		x = 0.0;
		y = 0.0;
		break;
	}

584a699,738

	if (image->aspect_clip == SP_ASPECT_SLICE) {
		image->viewx = image->x.computed;
		image->viewy = image->y.computed;
		image->viewwidth = image->width.computed;
		image->viewheight = image->height.computed;
		if ((imagewidth*image->height.computed)>(image->width.computed*imageheight)) {
			// Pixels aspect is wider than bounding box
			image->trimheight = imageheight;
			image->trimwidth = static_cast<int>(static_cast<double>(imageheight) * image->width.computed / image->height.computed);
			image->trimy = 0;
			image->trimx = static_cast<int>(static_cast<double>(imagewidth - image->trimwidth) * x);
		} else {
			// Pixels aspect is taller than bounding box
			image->trimwidth = imagewidth;
			image->trimheight = static_cast<int>(static_cast<double>(imagewidth) * image->height.computed / image->width.computed);
			image->trimx = 0;
			image->trimy = static_cast<int>(static_cast<double>(imageheight - image->trimheight) * y);
		}
	} else {
		// Otherwise, assume SP_ASPECT_MEET
		image->trimx = 0;
		image->trimy = 0;
		image->trimwidth = imagewidth;
		image->trimheight = imageheight;
		if ((imagewidth*image->height.computed)>(image->width.computed*imageheight)) {
			// Pixels aspect is wider than bounding boz
			image->viewwidth = image->width.computed;
			image->viewheight = image->viewwidth * imageheight / imagewidth;
			image->viewx=image->x.computed;
			image->viewy=(image->height.computed - image->viewheight) * y + image->y.computed;
		} else {
			// Pixels aspect is taller than bounding box
			image->viewheight = image->height.computed;
			image->viewwidth = image->viewheight * imagewidth / imageheight;
			image->viewy=image->y.computed;
			image->viewx=(image->width.computed - image->viewwidth) * x + image->x.computed;
		}
	}

}

604a759

sp_repr_set_attr (repr, "preserveAspectRatio", sp_repr_attr (object->repr, "preserveAspectRatio"));

636c791 < int w, h, rs; ---

int w, h, rs, pixskip;

642c797 < ---

647,651c802,813 < < /* fixme: (Lauris) */ < nr_matrix_set_translate (&tp, image->x.computed, image->y.computed); < nr_matrix_set_scale (&s, image->width.computed, -image->height.computed); < nr_matrix_set_translate (&ti, 0.0, -1.0); ---

pixskip = gdk_pixbuf_get_n_channels (image->pixbuf) * gdk_pixbuf_get_bits_per_sample (image->pixbuf) / 8;

if (image->aspect_align == SP_ASPECT_NONE) { /* fixme: (Lauris) */ nr_matrix_set_translate (&tp, image->x.computed, image->y.computed); nr_matrix_set_scale (&s, image->width.computed, -image->height.computed); nr_matrix_set_translate (&ti, 0.0, -1.0); } else { // preserveAspectRatio nr_matrix_set_translate (&tp, image->viewx, image->viewy); nr_matrix_set_scale (&s, image->viewwidth, -image->viewheight); nr_matrix_set_translate (&ti, 0.0, -1.0); }

656c818,821 < sp_print_image_R8G8B8A8_N (ctx, px, w, h, rs, &t, SP_OBJECT_STYLE (item)); ---

if (image->aspect_align == SP_ASPECT_NONE) sp_print_image_R8G8B8A8_N (ctx, px, w, h, rs, &t, SP_OBJECT_STYLE (item)); else // preserveAspectRatio sp_print_image_R8G8B8A8_N (ctx, px + image->trimx*pixskip + image->trimy*rs, image->trimwidth, image->trimheight, rs, &t, SP_OBJECT_STYLE (item));

678a844

int pixskip, rs;

687c853,856 < nr_arena_image_set_pixels (NR_ARENA_IMAGE (ai), ---

       pixskip = gdk_pixbuf_get_n_channels (image->pixbuf) * gdk_pixbuf_get_bits_per_sample (image->pixbuf) / 8;
rs = gdk_pixbuf_get_rowstride (image->pixbuf);
       if (image->aspect_align == SP_ASPECT_NONE)
	nr_arena_image_set_pixels (NR_ARENA_IMAGE (ai),

691c860,866 < gdk_pixbuf_get_rowstride (image->pixbuf)); ---

			   rs);
else // preserveAspectRatio
	nr_arena_image_set_pixels (NR_ARENA_IMAGE (ai),
			   gdk_pixbuf_get_pixels (image->pixbuf) + image->trimx*pixskip + image->trimy*rs,
			   image->trimwidth,
			   image->trimheight,
			   rs);

695c870,873 < nr_arena_image_set_geometry (NR_ARENA_IMAGE (ai), image->x.computed, image->y.computed, image->width.computed, image->height.computed); ---

if (image->aspect_align == SP_ASPECT_NONE) nr_arena_image_set_geometry (NR_ARENA_IMAGE (ai), image->x.computed, image->y.computed, image->width.computed, image->height.computed); else // preserveAspectRatio nr_arena_image_set_geometry (NR_ARENA_IMAGE (ai), image->viewx, image->viewy, image->viewwidth, image->viewheight);

777a956

int rs, pixskip;

794c973,976 < nr_arena_image_set_pixels (NR_ARENA_IMAGE (v->arenaitem), ---

       pixskip = gdk_pixbuf_get_n_channels (image->pixbuf) * gdk_pixbuf_get_bits_per_sample (image->pixbuf) / 8;
rs = gdk_pixbuf_get_rowstride (image->pixbuf);
if (image->aspect_align == SP_ASPECT_NONE) {
	nr_arena_image_set_pixels (NR_ARENA_IMAGE (v->arenaitem),

798,799c980,981 < gdk_pixbuf_get_rowstride (image->pixbuf)); < nr_arena_image_set_geometry (NR_ARENA_IMAGE (v->arenaitem), ---

			   rs);
	nr_arena_image_set_geometry (NR_ARENA_IMAGE (v->arenaitem),

801a984,993

} else { // preserveAspectRatio
	nr_arena_image_set_pixels (NR_ARENA_IMAGE (v->arenaitem),
			   gdk_pixbuf_get_pixels (image->pixbuf) + image->trimx*pixskip + image->trimy*rs,
			   image->trimwidth,
			   image->trimheight,
			   rs);
	nr_arena_image_set_geometry (NR_ARENA_IMAGE (v->arenaitem),
			     image->viewx, image->viewy,
			     image->viewwidth, image->viewheight);
}

Index: src/sp-image.h =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/sp-image.h,v retrieving revision 1.5 diff -r1.5 sp-image.h 29,30d28 < < 42a41,47

// Added by EAF /* preserveAspectRatio */ unsigned int aspect_align : 4; unsigned int aspect_clip : 1; int trimx, trimy, trimwidth, trimheight; double viewx, viewy, viewwidth, viewheight;

Thanks, I'll test it. Do you have a test SVG for all these options? Can you send it also?

And yes, diff -u is more readable than just diff, as Kees wrote.

On Sat, 12 Feb 2005 01:34:19 -0600, Edward Flick <eddy@...690...> wrote:

Hello again, I posted this patch to sourceforge. And I've attached it here also. It implements preserveAspectRatio rendering functionality as per SVG 1.1 spec with overflow assumed to be hidden. It currently has no human interface outside of the built in XML editor. Anybody care to check this out? Also, the attached file is just the piped output from 'cvs diff', is this acceptable?

On Sat, 12 Feb 2005 09:11:38 -0600, Edward Flick <eddy@...690...> wrote:

Well, this would require 36 test cases to test every option and combination for correctness, this will take me a minute (9 wide bounding box meet, 9 wide bounding box slice, 9 tall bounding box meet, and 9 tall bounding box slice).

This would be useful to have in any case, since even the W3C SVG test suite does not test this attribute.

Quoting Edward Flick <eddy@...690...>:

Well, they do have a test for preserveAspectRatio but it is not for <image>s which is kind of a special case anyways since it has an implied viewBox attribute when its referencing raster content. Anyways, here it is. It requires an image in the same folder called 'a.png'. Just pick any picture with fairly unique corners in it.

Hmmmm... sounds like a handy time for embedded images.

I'll try to convert that later. Do we happen to have an RFE for getting a UI on embedding them?

bulia byak wrote:

Excellent! The test file now renders the same as in Batik (but not Adobe, looks like Adobe does not support this attribute). I applied your patch. Please keep contributing :)

I love this project. I will surely look into other areas of it. The next area I am very interested in are the effects.

Now, I'm still not sure we need a right-click menu item for this. Just try to scale one the the bottom row images. They behave quite counter-intuitively. Kinda like broken pattern fill. I know there's logic to it, but it's not easy to grasp. On the other hand, what this attribute provides is easily and interactively achievable via standard scaling and clipping (we don't fully support clipping yet, but it's easy to emulate via object-to-pattern), so investing into learning this logic will not really pay off unless you have very special needs. So, I still think that an "Advanced" tab in Object Properties is the best place for setting this attribute.

Yeah, it doesn't necessarily need a right-click menu item. Even though I am going to be using this as a template engine, most people probably aren't. What I would love to see is user definable Python actions that can be added to the right click menu of specific sp-items. That would be perfect for my situation. But also, having preserveAspectRatio in the object attributes (whether under advanced or not) is necessary in case somebody loads up an SVG and can't figure out how to get it to quit behaving counter-intuitively ;-).

Edward Flick