[cairo] [Bug 90166] Add a way to specify units

[Bill Spitzak]

Comment # 8 on bug 90166 from Adrian Johnson

The proposed api in this bug
https://bugs.freedesktop.org/show_bug.cgi?id=90166 uses an enumeration
for each of the svg unit types, but nothing is ever done with that
enumeration other than to print a 2-letter string into the svg output.

I propose a better api would be to just accept the string and print it
without any checking. This would remove the need to modify cairo on
any svg updates that add new units, and also probably makes it much
easier to copy the units from one svg to another.

Proposed api:

void cairo_svg_surface_set_document_unit (cairo_surface_t*, const char*)

The string pointer must point at a valid svg unit type, such as "pt"
or "em". NULL is the same as "" (which means "user" units). The
function should do some basic sanity checks (maximum length, and only
printing ASCII) and is a no-op if it fails.

Comment # 8 on bug 90166 from Adrian Johnson
Please post API proposals to the mailing list for discussion.
________________________________
You are watching the QA Contact of the bug.

[Antonio Ospite]

On Wed, 11 Oct 2017 16:19:18 -0700
spitzak at gmail.com (Bill Spitzak) wrote:

[...]

Post by Bill Spitzak
void cairo_svg_surface_set_document_unit (cairo_surface_t*, const char*)
The string pointer must point at a valid svg unit type, such as "pt"
or "em". NULL is the same as "" (which means "user" units). The
function should do some basic sanity checks (maximum length, and only
printing ASCII) and is a no-op if it fails.

The API addition I originally proposed[1,2] looks like this:

void cairo_svg_surface_set_document_unit (cairo_surface_t*, cairo_svg_unit_t)

Where cairo_svg_unit_t is an enum type representing all the possible
units.

The function signature is modeled after
cairo_svg_surface_restrict_to_version() which also uses an enum, so
cairo users are already familiar with this approach, and I think it's OK
for a few more reasons:

1. IMHO it's a little more robust, it allows to produce only valid
documents (after we discuss if relative units are safe to use).

2. The implementation is simple, hence easy to validate, for instance
there is no need to handle special characters like " to assure that
the XML is well formed.

3. The event of new units being added to the SVG spec seems quite
rare, so having cairo react to that (even in a minor release) seems
acceptable to me considering the benefits of 1. and 2.

Sanitizing user strings is a bother, when possible I prefer avoiding
a problem rather than having to solve it.

That said I am obviously open to suggestions, since I am not the one who
is going to maintain the codebase I will be OK with what the cairo
developers decide.

After we decide on the API I will bring up some more questions.

Ciao,
Antonio

[1] https://bugs.freedesktop.org/show_bug.cgi?id=90166
[2] https://bugs.freedesktop.org/attachment.cgi?id=134799
--
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[Adrian Johnson]

Post by Antonio Ospite
void cairo_svg_surface_set_document_unit (cairo_surface_t*, cairo_svg_unit_t)
Where cairo_svg_unit_t is an enum type representing all the possible
units.

The enum in the patch is:

typedef enum _cairo_svg_unit {
CAIRO_SVG_UNIT_EM,
CAIRO_SVG_UNIT_EX,
CAIRO_SVG_UNIT_PX,
CAIRO_SVG_UNIT_IN,
CAIRO_SVG_UNIT_CM,
CAIRO_SVG_UNIT_MM,
CAIRO_SVG_UNIT_PT,
CAIRO_SVG_UNIT_PC,
CAIRO_SVG_UNIT_PERCENT,
CAIRO_SVG_UNIT_USER
} cairo_svg_unit_t;

This looks good to me. It just needs a getter. Are all the units in SVG
1.1? If not the minimum version will need to be documented. The default
unit will also need to be documented.

[Adrian Johnson]

Post by Adrian Johnson

Post by Antonio Ospite
void cairo_svg_surface_set_document_unit (cairo_surface_t*, cairo_svg_unit_t)
Where cairo_svg_unit_t is an enum type representing all the possible
units.

typedef enum _cairo_svg_unit {
CAIRO_SVG_UNIT_EM,
CAIRO_SVG_UNIT_EX,
CAIRO_SVG_UNIT_PX,
CAIRO_SVG_UNIT_IN,
CAIRO_SVG_UNIT_CM,
CAIRO_SVG_UNIT_MM,
CAIRO_SVG_UNIT_PT,
CAIRO_SVG_UNIT_PC,
CAIRO_SVG_UNIT_PERCENT,
CAIRO_SVG_UNIT_USER
} cairo_svg_unit_t;

We don't have to use the SVG abbreviations in the enum. The non standard
abbreviations can be expanded to make it clearer. I've reordered it
slightly to put the default first followed by the more common units.

typedef enum _cairo_svg_unit {
CAIRO_SVG_UNIT_POINTS,
CAIRO_SVG_UNIT_PIXELS,
CAIRO_SVG_UNIT_MM,
CAIRO_SVG_UNIT_CM,
CAIRO_SVG_UNIT_INCHES,
CAIRO_SVG_UNIT_PICAS,
CAIRO_SVG_UNIT_FONT_EM,
CAIRO_SVG_UNIT_FONT_X,
CAIRO_SVG_UNIT_PERCENT,
CAIRO_SVG_UNIT_USER
} cairo_svg_unit_t;

Post by Adrian Johnson
This looks good to me. It just needs a getter. Are all the units in SVG
1.1? If not the minimum version will need to be documented. The default
unit will also need to be documented.

[Petr Kobalíček]

'MM/CM' vs 'INCHES' and all others seems inconsistent to me.

Post by Antonio Ospite

Post by Adrian Johnson

Post by Bill Spitzak
void cairo_svg_surface_set_document_unit (cairo_surface_t*,

cairo_svg_unit_t)

Post by Adrian Johnson

Post by Bill Spitzak
Where cairo_svg_unit_t is an enum type representing all the possible
units.

typedef enum _cairo_svg_unit {
CAIRO_SVG_UNIT_EM,
CAIRO_SVG_UNIT_EX,
CAIRO_SVG_UNIT_PX,
CAIRO_SVG_UNIT_IN,
CAIRO_SVG_UNIT_CM,
CAIRO_SVG_UNIT_MM,
CAIRO_SVG_UNIT_PT,
CAIRO_SVG_UNIT_PC,
CAIRO_SVG_UNIT_PERCENT,
CAIRO_SVG_UNIT_USER
} cairo_svg_unit_t;

We don't have to use the SVG abbreviations in the enum. The non standard
abbreviations can be expanded to make it clearer. I've reordered it
slightly to put the default first followed by the more common units.
typedef enum _cairo_svg_unit {
CAIRO_SVG_UNIT_POINTS,
CAIRO_SVG_UNIT_PIXELS,
CAIRO_SVG_UNIT_MM,
CAIRO_SVG_UNIT_CM,
CAIRO_SVG_UNIT_INCHES,
CAIRO_SVG_UNIT_PICAS,
CAIRO_SVG_UNIT_FONT_EM,
CAIRO_SVG_UNIT_FONT_X,
CAIRO_SVG_UNIT_PERCENT,
CAIRO_SVG_UNIT_USER
} cairo_svg_unit_t;

Post by Adrian Johnson
This looks good to me. It just needs a getter. Are all the units in SVG
1.1? If not the minimum version will need to be documented. The default
unit will also need to be documented.

--
cairo mailing list
https://lists.cairographics.org/mailman/listinfo/cairo

[Adrian Johnson]

Post by Petr Kobalíček
'MM/CM' vs 'INCHES' and all others seems inconsistent to me.

What do you suggest?

Post by Petr Kobalíček

Post by Adrian Johnson

Post by Antonio Ospite
void cairo_svg_surface_set_document_unit (cairo_surface_t*, cairo_svg_unit_t)
Where cairo_svg_unit_t is an enum type representing all the possible
units.

typedef enum _cairo_svg_unit {
     CAIRO_SVG_UNIT_EM,
     CAIRO_SVG_UNIT_EX,
     CAIRO_SVG_UNIT_PX,
     CAIRO_SVG_UNIT_IN,
     CAIRO_SVG_UNIT_CM,
     CAIRO_SVG_UNIT_MM,
     CAIRO_SVG_UNIT_PT,
     CAIRO_SVG_UNIT_PC,
     CAIRO_SVG_UNIT_PERCENT,
     CAIRO_SVG_UNIT_USER
} cairo_svg_unit_t;

We don't have to use the SVG abbreviations in the enum. The non standard
abbreviations can be expanded to make it clearer. I've reordered it
slightly to put the default first followed by the more common units.
typedef enum _cairo_svg_unit {
    CAIRO_SVG_UNIT_POINTS,
    CAIRO_SVG_UNIT_PIXELS,
    CAIRO_SVG_UNIT_MM,
    CAIRO_SVG_UNIT_CM,
    CAIRO_SVG_UNIT_INCHES,
    CAIRO_SVG_UNIT_PICAS,
    CAIRO_SVG_UNIT_FONT_EM,
    CAIRO_SVG_UNIT_FONT_X,
    CAIRO_SVG_UNIT_PERCENT,
    CAIRO_SVG_UNIT_USER
} cairo_svg_unit_t;

Post by Adrian Johnson
This looks good to me. It just needs a getter. Are all the units

in SVG

Post by Adrian Johnson
1.1? If not the minimum version will need to be documented. The

default

Post by Adrian Johnson
unit will also need to be documented.

--
cairo mailing list
https://lists.cairographics.org/mailman/listinfo/cairo
<https://lists.cairographics.org/mailman/listinfo/cairo>

[David Kastrup]

Post by Adrian Johnson

Post by Petr Kobalíček
'MM/CM' vs 'INCHES' and all others seems inconsistent to me.

What do you suggest?

Post by Petr Kobalíček

Post by Adrian Johnson

Post by Bill Spitzak
void cairo_svg_surface_set_document_unit (cairo_surface_t*,
cairo_svg_unit_t)
Where cairo_svg_unit_t is an enum type representing all the possible
units.

typedef enum _cairo_svg_unit {
     CAIRO_SVG_UNIT_EM,
     CAIRO_SVG_UNIT_EX,
     CAIRO_SVG_UNIT_PX,
     CAIRO_SVG_UNIT_IN,
     CAIRO_SVG_UNIT_CM,
     CAIRO_SVG_UNIT_MM,
     CAIRO_SVG_UNIT_PT,
     CAIRO_SVG_UNIT_PC,
     CAIRO_SVG_UNIT_PERCENT,
     CAIRO_SVG_UNIT_USER
} cairo_svg_unit_t;

We don't have to use the SVG abbreviations in the enum. The non standard
abbreviations can be expanded to make it clearer. I've reordered it
slightly to put the default first followed by the more common units.
typedef enum _cairo_svg_unit {
    CAIRO_SVG_UNIT_POINTS,
    CAIRO_SVG_UNIT_PIXELS,
    CAIRO_SVG_UNIT_MM,
    CAIRO_SVG_UNIT_CM,
    CAIRO_SVG_UNIT_INCHES,
    CAIRO_SVG_UNIT_PICAS,
    CAIRO_SVG_UNIT_FONT_EM,
    CAIRO_SVG_UNIT_FONT_X,
    CAIRO_SVG_UNIT_PERCENT,
    CAIRO_SVG_UNIT_USER
} cairo_svg_unit_t;

Frankly, this is not just the "SVG abbreviations". It's TeX terminology
as well as used in HTML/CSS. Programmers will tend to know the two
character abbreviations better than the long names (FONT_X ? For real?).

This does not look like an actual improvement to me. That it prompts a
bikeshedding discussion should be proof enough that this makes things
more rather than less arbitrary and harder rather than easier to guess.

[Adrian Johnson]

Post by David Kastrup
Frankly, this is not just the "SVG abbreviations". It's TeX terminology
as well as used in HTML/CSS. Programmers will tend to know the two
character abbreviations better than the long names (FONT_X ? For real?).
This does not look like an actual improvement to me. That it prompts a
bikeshedding discussion should be proof enough that this makes things
more rather than less arbitrary and harder rather than easier to guess.

I'm open to suggestions.

[Antonio Ospite]

On Mon, 16 Oct 2017 18:32:10 +1030

Post by Adrian Johnson

Post by David Kastrup
Frankly, this is not just the "SVG abbreviations". It's TeX terminology
as well as used in HTML/CSS. Programmers will tend to know the two
character abbreviations better than the long names (FONT_X ? For real?).
This does not look like an actual improvement to me. That it prompts a
bikeshedding discussion should be proof enough that this makes things
more rather than less arbitrary and harder rather than easier to guess.

I'm open to suggestions.

From the comments, it looks like leaving the original names for the
units seems acceptable, they may not be the easiest for everyone but
they are standard, simple and more consistent in form (except PERCENT,
and USER, sure), and I tried to document their meaning in the API doc.

About reordering the enum, I'd just like to point out that the
current order is taken from the CSS spec itself, we don't have to
necessarily follow it, but it was like that for some reason.
That said, just tell me how you want the enum ordered and I'll follow.

After we sort out the ordering doubt I will submit a v2 with a
clearer documentation and a getter function.

Finally, I think that allowing also relative units to be used in the
final document is OK, AFAICS their are all valid for width and height,
at least according to SVG schema from here:
https://github.com/oreillymedia/HTMLBook/tree/master/schema/svg

I can validate Cairo SVGs like this:

1. Get all the files from here:
https://github.com/ao2/HTMLBook/tree/svg-schema-add-version-attribute/schema/svg
The original schema from w3c does not work with xmllint, and it
does not seem to be _official_ anyways.

2. Run the test program in the bug report to create the svg files, and
then:
$ for file in svg_output_*.svg; do xmllint --noout --schema SVG.xsd "$file"; done

Ciao,
Antonio
--
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[Bill Spitzak]

I would put USER first as it appears to be the default value, making
it zero is a good idea to avoid confusion.

Post by Antonio Ospite
On Mon, 16 Oct 2017 18:32:10 +1030

Post by Adrian Johnson

Post by David Kastrup
Frankly, this is not just the "SVG abbreviations". It's TeX terminology
as well as used in HTML/CSS. Programmers will tend to know the two
character abbreviations better than the long names (FONT_X ? For real?).
This does not look like an actual improvement to me. That it prompts a
bikeshedding discussion should be proof enough that this makes things
more rather than less arbitrary and harder rather than easier to guess.

I'm open to suggestions.

From the comments, it looks like leaving the original names for the
units seems acceptable, they may not be the easiest for everyone but
they are standard, simple and more consistent in form (except PERCENT,
and USER, sure), and I tried to document their meaning in the API doc.
About reordering the enum, I'd just like to point out that the
current order is taken from the CSS spec itself, we don't have to
necessarily follow it, but it was like that for some reason.
That said, just tell me how you want the enum ordered and I'll follow.
After we sort out the ordering doubt I will submit a v2 with a
clearer documentation and a getter function.
Finally, I think that allowing also relative units to be used in the
final document is OK, AFAICS their are all valid for width and height,
https://github.com/oreillymedia/HTMLBook/tree/master/schema/svg
https://github.com/ao2/HTMLBook/tree/svg-schema-add-version-attribute/schema/svg
The original schema from w3c does not work with xmllint, and it
does not seem to be _official_ anyways.
2. Run the test program in the bug report to create the svg files, and
$ for file in svg_output_*.svg; do xmllint --noout --schema SVG.xsd "$file"; done
Ciao,
Antonio
--
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
--
cairo mailing list
https://lists.cairographics.org/mailman/listinfo/cairo

[Adrian Johnson]

Post by Bill Spitzak
I would put USER first as it appears to be the default value, making
it zero is a good idea to avoid confusion.

The default is points.

Code:

"<svg xmlns=\"http://www.w3.org/2000/svg\" "
"xmlns:xlink=\"http://www.w3.org/1999/xlink\" "
"width=\"%fpt\" height=\"%fpt\" "

Docs:

* @width_in_points: width of the surface, in points (1 point == 1/72.0
inch)
* @height_in_points: height of the surface, in points (1 point ==
1/72.0 inch)

[Antonio Ospite]

On Tue, 17 Oct 2017 06:24:17 +1030

Post by Adrian Johnson

Post by Bill Spitzak
I would put USER first as it appears to be the default value, making
it zero is a good idea to avoid confusion.

The default is points.

So, should I put PT first and have the following order?

+typedef enum _cairo_svg_unit {
+ CAIRO_SVG_UNIT_PT = 0,
+ CAIRO_SVG_UNIT_EM,
+ CAIRO_SVG_UNIT_EX,
+ CAIRO_SVG_UNIT_PX,
+ CAIRO_SVG_UNIT_IN,
+ CAIRO_SVG_UNIT_CM,
+ CAIRO_SVG_UNIT_MM,
+ CAIRO_SVG_UNIT_PC,
+ CAIRO_SVG_UNIT_PERCENT,
+ CAIRO_SVG_UNIT_USER
+} cairo_svg_unit_t;

Or can I stick to the "official" order suggested by the specification
[1], and just mention in the API doc that "pt" happens to be the default
unit for *cairo*, if cairo_svg_surface_set_document_unit () is not
called?

Thanks,
Antonio

[1] https://www.w3.org/TR/SVG/types.html#DataTypeLength
--
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[Bill Spitzak]

If I understand that right, "percent" is not allowed in the style attribute.

Also it seems like the default is "user". By default I mean what svg
does if you don't specify the unit at all. If cairo has been forcing
that value to pt then I think Cairo is wrong.

Post by Antonio Ospite
On Tue, 17 Oct 2017 06:24:17 +1030

Post by Adrian Johnson

Post by Bill Spitzak
I would put USER first as it appears to be the default value, making
it zero is a good idea to avoid confusion.

The default is points.

So, should I put PT first and have the following order?
+typedef enum _cairo_svg_unit {
+ CAIRO_SVG_UNIT_PT = 0,
+ CAIRO_SVG_UNIT_EM,
+ CAIRO_SVG_UNIT_EX,
+ CAIRO_SVG_UNIT_PX,
+ CAIRO_SVG_UNIT_IN,
+ CAIRO_SVG_UNIT_CM,
+ CAIRO_SVG_UNIT_MM,
+ CAIRO_SVG_UNIT_PC,
+ CAIRO_SVG_UNIT_PERCENT,
+ CAIRO_SVG_UNIT_USER
+} cairo_svg_unit_t;
Or can I stick to the "official" order suggested by the specification
[1], and just mention in the API doc that "pt" happens to be the default
unit for *cairo*, if cairo_svg_surface_set_document_unit () is not
called?
Thanks,
Antonio
[1] https://www.w3.org/TR/SVG/types.html#DataTypeLength
--
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
--
cairo mailing list
https://lists.cairographics.org/mailman/listinfo/cairo

[Antonio Ospite]

On Mon, 23 Oct 2017 10:29:16 -0700

Post by Bill Spitzak
If I understand that right, "percent" is not allowed in the style attribute.

The focus here are the "width" and "height" attributes of the root
<svg/>, IIUC they are presentation attributes which allow percent as a
unit, to back up this argument I verified that files with percent as
unit in there do validate according to the "unofficial" XML schemas
available online:
https://github.com/oreillymedia/HTMLBook/tree/master/schema/svg
https://github.com/ao2/HTMLBook/tree/svg-schema-add-version-attribute/schema/svg

See also one of my previous messages.

Post by Bill Spitzak
Also it seems like the default is "user". By default I mean what svg
does if you don't specify the unit at all. If cairo has been forcing
that value to pt then I think Cairo is wrong.

It may very well be wrong, but that's the current behavior and
that's what users (either human or not) might expect, so I don't think
it'd be wise to change the default unit cairo uses for the generated
SVGs.

However having as CAIRO_SVG_UNIT_USER as first in the enum definition
makes sense following this rationale:

1. The enum lists all the units allowed by the specification,
regardless of where they are used in cairo.

2. It's only the new API call cairo_svg_surface_set_document_unit()
that refers specifically to the root <svg/> element.

So following to 1. this enum might be OK:

typedef enum _cairo_svg_unit {
   CAIRO_SVG_UNIT_USER = 0,
   CAIRO_SVG_UNIT_EM,
   CAIRO_SVG_UNIT_EX,
   CAIRO_SVG_UNIT_PX,
   CAIRO_SVG_UNIT_IN,
   CAIRO_SVG_UNIT_CM,
   CAIRO_SVG_UNIT_MM,
   CAIRO_SVG_UNIT_PT,
   CAIRO_SVG_UNIT_PC,
   CAIRO_SVG_UNIT_PERCENT
} cairo_svg_unit_t;

This also seems to follow the specification[1] more naturally, i.e.:
explicit units match the optional pattern ("em" | "ex" | ... )?

I can add a comment when initializing the default document->unit to
CAIRO_SVG_UNIT_PT to stress out that this value is used for backward
compatibility.

Thanks,
Antonio

[1] https://www.w3.org/TR/SVG/types.html#DataTypeLength
--
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[Antonio Ospite]

On Thu, 12 Oct 2017 21:59:08 +1030
Adrian Johnson <***@redneon.com> wrote:

[...]

Post by Adrian Johnson
typedef enum _cairo_svg_unit {

[...]

Post by Adrian Johnson
} cairo_svg_unit_t;
This looks good to me. It just needs a getter.

About the getter function, what do you prefer?

1. A function which returns the unit in the return value.

This seems the more natural approach, but what to return if the
surface is not a SVG surface? In this case I could set the error code
and return the default SVG unit, and document this behavior in the
API doc.

This was my first version:

cairo_svg_unit_t
cairo_svg_surface_get_document_unit (cairo_surface_t *abstract_surface) +{
cairo_svg_surface_t *surface = NULL; /* hide compiler warning */

if (! _extract_svg_surface (abstract_surface, &surface)) {
_cairo_error_throw (CAIRO_STATUS_SURFACE_TYPE_MISMATCH);
return CAIRO_SVG_UNIT_USER;
}

return surface->document->unit;
}

2. A function which returns the unit in an output parameter and returns
a status in the return value.

This seems more robust; is it cairo-style?

cairo_status_t
cairo_svg_surface_get_document_unit (cairo_surface_t *abstract_surface,
cairo_svg_unit_t *unit)
{
cairo_svg_surface_t *surface = NULL; /* hide compiler warning */

if (! _extract_svg_surface (abstract_surface, &surface))
return _cairo_error (CAIRO_STATUS_SURFACE_TYPE_MISMATCH);

*unit = surface->document->unit;
return CAIRO_STATUS_SUCCESS;
}

Post by Adrian Johnson
Are all the units in SVG 1.1? If not the minimum version will need to
be documented.

All the units are in SVG 1.0 already, so nothing to add about that.

Post by Adrian Johnson
The default unit will also need to be documented.

I'll mention that the default unit for *cairo* is "pt".

Ciao,
Antonio
--
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[Uli Schlachter]

Post by Antonio Ospite
On Thu, 12 Oct 2017 21:59:08 +1030
[...]

Post by Adrian Johnson
typedef enum _cairo_svg_unit {

[...]

Post by Adrian Johnson
} cairo_svg_unit_t;
This looks good to me. It just needs a getter.

About the getter function, what do you prefer?
1. A function which returns the unit in the return value.

[...]

Post by Antonio Ospite
2. A function which returns the unit in an output parameter and returns
a status in the return value.
This seems more robust; is it cairo-style?

[...]

Hi,

take a look at, for example, cairo_image_surface_get_width():

int
cairo_image_surface_get_width (cairo_surface_t *surface)
{
cairo_image_surface_t *image_surface = (cairo_image_surface_t *)
surface;

if (! _cairo_surface_is_image (surface)) {
_cairo_error_throw (CAIRO_STATUS_SURFACE_TYPE_MISMATCH);
return 0;
}

return image_surface->width;
}

I'd say the same should be done here. No one will check the status
return value of such a function anyway.

Cheers,
Uli

[Antonio Ospite]

On Tue, 31 Oct 2017 10:35:47 +0100

Post by Uli Schlachter

Post by Antonio Ospite
On Thu, 12 Oct 2017 21:59:08 +1030
[...]

Post by Adrian Johnson
typedef enum _cairo_svg_unit {

[...]

Post by Adrian Johnson
} cairo_svg_unit_t;
This looks good to me. It just needs a getter.

About the getter function, what do you prefer?
1. A function which returns the unit in the return value.

[...]

Post by Antonio Ospite
2. A function which returns the unit in an output parameter and returns
a status in the return value.
This seems more robust; is it cairo-style?

[...]
Hi,

Hi Uli, thanks for the comment.
[...]

Post by Uli Schlachter
I'd say the same should be done here. No one will check the status
return value of such a function anyway.

That's where I took inspiration for 1.

The only doubt was that while width=0 might suggest that something is
wrong, returning unit=CAIRO_SVG_UNIT_USER does not really raise any
eyebrows.

However, to be fair, the value is returned when the surface is not an
SVG surface so it is meaningless anyway and unusable for subsequent
operations on that particular surface.

So I think I'll go for 1. unless someone else objects.

Ciao,
Antonio
--
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[Antonio Ospite]

On Thu, 12 Oct 2017 12:25:41 +0200
Antonio Ospite <***@ao2.it> wrote:

[...]

Post by Antonio Ospite
void cairo_svg_surface_set_document_unit (cairo_surface_t*, cairo_svg_unit_t)
Where cairo_svg_unit_t is an enum type representing all the possible
units.

Hi,

here is v2 of the path:
https://bugs.freedesktop.org/show_bug.cgi?id=90166#c11

After this is merged we can discuss if some extensions can be useful;
like getting an array of all the possible units, or having a function
to convert the enum values to strings, I mean following the model of
cairo_svg_get_versions() and cairo_svg_version_to_string().

I was also thinking of tweaking the documentation of the functions which
create SVG surfaces to state that "width" and "height" are assumed to
be in points by *default*, but that their unit can be changed later.

Ciao,
Antonio
--
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[Bryce Harrington]

Post by Antonio Ospite
On Thu, 12 Oct 2017 12:25:41 +0200
[...]

Post by Antonio Ospite
void cairo_svg_surface_set_document_unit (cairo_surface_t*, cairo_svg_unit_t)
Where cairo_svg_unit_t is an enum type representing all the possible
units.

Hi,
https://bugs.freedesktop.org/show_bug.cgi?id=90166#c11
After this is merged we can discuss if some extensions can be useful;
like getting an array of all the possible units, or having a function
to convert the enum values to strings, I mean following the model of
cairo_svg_get_versions() and cairo_svg_version_to_string().
I was also thinking of tweaking the documentation of the functions which
create SVG surfaces to state that "width" and "height" are assumed to
be in points by *default*, but that their unit can be changed later.

The patch looks good to me. I agree with making user the default even
though cairo has defaulted to "pt" historically. Regarding using the
abbr versions of the units rather than full strings I could go either
way on myself, and have no arguments against using the short versions;
anyone dealing with SVG document syntax should be adequately familiar
with the abbreviated versions already.

Reviewed-by: Bryce Harrington <***@osg.samsung.com>

If no one raises any further qualms within the next few days, I'll go
ahead and land this patch.

Bryce

[Adrian Johnson]

Post by Bryce Harrington

Post by Antonio Ospite
On Thu, 12 Oct 2017 12:25:41 +0200
[...]

Post by Antonio Ospite
void cairo_svg_surface_set_document_unit (cairo_surface_t*, cairo_svg_unit_t)
Where cairo_svg_unit_t is an enum type representing all the possible
units.

Hi,
https://bugs.freedesktop.org/show_bug.cgi?id=90166#c11
After this is merged we can discuss if some extensions can be useful;
like getting an array of all the possible units, or having a function
to convert the enum values to strings, I mean following the model of
cairo_svg_get_versions() and cairo_svg_version_to_string().
I was also thinking of tweaking the documentation of the functions which
create SVG surfaces to state that "width" and "height" are assumed to
be in points by *default*, but that their unit can be changed later.

The patch looks good to me. I agree with making user the default even
though cairo has defaulted to "pt" historically.

That would be an API change. We can't do that.

Post by Bryce Harrington
Regarding using the
abbr versions of the units rather than full strings I could go either
way on myself, and have no arguments against using the short versions;
anyone dealing with SVG document syntax should be adequately familiar
with the abbreviated versions already.
If no one raises any further qualms within the next few days, I'll go
ahead and land this patch.
Bryce

[Bryce Harrington]

Post by Adrian Johnson

Post by Bryce Harrington

Post by Antonio Ospite
On Thu, 12 Oct 2017 12:25:41 +0200
[...]

Post by Antonio Ospite
void cairo_svg_surface_set_document_unit (cairo_surface_t*, cairo_svg_unit_t)
Where cairo_svg_unit_t is an enum type representing all the possible
units.

Hi,
https://bugs.freedesktop.org/show_bug.cgi?id=90166#c11
After this is merged we can discuss if some extensions can be useful;
like getting an array of all the possible units, or having a function
to convert the enum values to strings, I mean following the model of
cairo_svg_get_versions() and cairo_svg_version_to_string().
I was also thinking of tweaking the documentation of the functions which
create SVG surfaces to state that "width" and "height" are assumed to
be in points by *default*, but that their unit can be changed later.

The patch looks good to me. I agree with making user the default even
though cairo has defaulted to "pt" historically.

That would be an API change. We can't do that.

No, just the default (0) for the enum, as is implemented in the v2 of
the patch.

Post by Adrian Johnson

Post by Bryce Harrington
Regarding using the
abbr versions of the units rather than full strings I could go either
way on myself, and have no arguments against using the short versions;
anyone dealing with SVG document syntax should be adequately familiar
with the abbreviated versions already.
If no one raises any further qualms within the next few days, I'll go
ahead and land this patch.
Bryce

[Adrian Johnson]

Post by Bryce Harrington
No, just the default (0) for the enum, as is implemented in the v2 of
the patch.

That's ok. I should have read the patch.

[Antonio Ospite]

On Mon, 6 Nov 2017 14:14:18 -0800
Thanks Bryce.

On Tue, 7 Nov 2017 18:11:42 +1030

Post by Adrian Johnson

Post by Bryce Harrington
No, just the default (0) for the enum, as is implemented in the v2 of
the patch.

That's ok. I should have read the patch.

Just to recap: "user" is considered to be the default *SVG* unit in the
general sense, while the patch still keeps "pt" as the default unit used
for the files generated by *cairo*, for backward compatibility.

Ciao,
Antonio
--
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[Bryce Harrington]

Post by Antonio Ospite
On Mon, 6 Nov 2017 14:14:18 -0800
Thanks Bryce.

Landed on trunk:

To ssh://git.freedesktop.org/git/cairo
84fc0ce..15559b5 master -> master

Thanks again,
Bryce

Post by Antonio Ospite
On Tue, 7 Nov 2017 18:11:42 +1030

Post by Adrian Johnson

Post by Bryce Harrington
No, just the default (0) for the enum, as is implemented in the v2 of
the patch.

That's ok. I should have read the patch.

Just to recap: "user" is considered to be the default *SVG* unit in the
general sense, while the patch still keeps "pt" as the default unit used
for the files generated by *cairo*, for backward compatibility.
Ciao,
Antonio
--
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?