333841 - "text-rendering" attribute has no effect on <text> or <tspan> tags when rendering SVG document

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Daniel Vanesse]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1

text rendering is always the same regardless of text-rendering attribute value

Reproducible: Always

Steps to Reproduce:
1.Create a SVG file with 3 lines of text
2.set a "text-rendering" attribute with a different value for each line 
  (optimizeSpeed, optimizeLegibility, OptimizeQuality)
3.display in Firefox

Actual Results:  
No change in rendering quality / legibility

Expected Results:  
text rendering quality to change according to attribute value

- That attribute has expected effects when the test file is displayed with Adobe Svg Viewer (rel. 3.x)

- anti-aliasing setting has no effect

Comment 1

[Martijn Wargers (dead)]

Could you attach a testcase or give an url that shows the bug?

Comment 2

[Daniel Vanesse]

Attachment: Test case

Comment 3

[Daniel Vanesse]

Attachment: Test case (auto-detect type)

Comment 4

[Daniel Vanesse]

Same problem found on Linux Fedora FC4 platform ( Firefox 1.5 RC3 ) when using the attached test case

Comment 5

[Takeshi Kurosawa :takenspc]

confirmed

Comment 6

[Takeshi Kurosawa :takenspc]

Attachment: Test case(fill+stroke, fill and stroke)

A test case including fill+stroke, fill and stroke texts.

Comment 7

[Takeshi Kurosawa :takenspc]

Attachment: draft patch

draft patch

|geometricPrecision| corresponds |cairo_text_path| + |CAIRO_HINT_METRICS_OFF|
|optimizeSpeed| corresponds  |cairo_show_text| + |CAIRO_ANTIALIAS_NONE|
others correspond  |cairo_show_text| + |CAIRO_HINT_METRICS_ON|

Comment 8

[Robert Longson [:longsonr]]

Comment on attachment 251449 [details] [diff] [review]
draft patch

>   void *closure;
>-  if (HasFill() && NS_SUCCEEDED(SetupCairoFill(gfx, &closure))) {
>-    LoopCharacters(ctx, text, cp, cairo_show_text);
>-    CleanupCairoFill(gfx, closure);
>+  PRBool DID_FILL = PR_FALSE;

I don't think we normally use upper case for variable names.

>+  cairo_font_options_t *fontOptions = cairo_font_options_create();
>+  cairo_get_font_options(ctx, fontOptions);  
>+  
>+  switch (GetStyleSVG()->mTextRendering) {
>+    case NS_STYLE_TEXT_RENDERING_OPTIMIZESPEED: 
>+      cairo_font_options_set_antialias(fontOptions, CAIRO_ANTIALIAS_NONE);
>+      cairo_set_antialias(ctx, CAIRO_ANTIALIAS_NONE);

I don't think the cairo_set_antialias call is correct. You just want the first one don't you?

>+      break;
>+    case NS_STYLE_TEXT_RENDERING_GEOMETRICPRECISION:
>+      cairo_font_options_set_hint_metrics(fontOptions, CAIRO_HINT_METRICS_OFF);

This is not the opposite to the call for optimizespeed so setting rendering to geometricprecision and then back to optimizespeed will not return the state to what it was originally.

>+      break;
>+    default:
>+      cairo_font_options_set_hint_metrics(fontOptions, CAIRO_HINT_METRICS_ON);
>+      break;

crispEdges and auto should be different. Auto should correspond to geometric precision according to my reading of the SVG 1.1 spec.

>   }
>+  cairo_set_font_options(ctx, fontOptions);
>+  cairo_font_options_destroy(fontOptions);
> 
>-  if (HasStroke() && NS_SUCCEEDED(SetupCairoStroke(gfx, &closure))) {
>+  if (HasFill() && NS_SUCCEEDED(SetupCairoFill(gfx, &closure))) {
>     cairo_new_path(ctx);
>     if (!cp)
>       cairo_move_to(ctx, mX, mY);
>-    LoopCharacters(ctx, text, cp, cairo_text_path);
>+
>+    if (GetStyleSVG()->mTextRendering ==
>+        NS_STYLE_TEXT_RENDERING_GEOMETRICPRECISION) {
>+      LoopCharacters(ctx, text, cp, cairo_text_path);
>+      cairo_fill_preserve(ctx);
>+      DID_FILL = PR_TRUE;
>+    } else {
>+      LoopCharacters(ctx, text, cp, cairo_show_text);
>+    }
>+    CleanupCairoFill(gfx, closure);
>+  }
>+
>+  if (HasStroke() && NS_SUCCEEDED(SetupCairoStroke(gfx, &closure))) {
>+    if (!DID_FILL) {

Is it OK to skip this if DID_FILL is true? What if the fill and stroke are different colours?

>+      cairo_new_path(ctx);
>+      if (!cp)
>+        cairo_move_to(ctx, mX, mY);
>+      LoopCharacters(ctx, text, cp, cairo_text_path);
>+    }
>     cairo_stroke(ctx);
>     CleanupCairoStroke(gfx, closure);
>-    cairo_new_path(ctx);
>   }
> 
>+  cairo_new_path(ctx);
>   cairo_restore(ctx);
> 
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
> nsSVGGlyphFrame::GetFrameForPointSVG(float x, float y, nsIFrame** hit)
> {

Comment 9

[Takeshi Kurosawa :takenspc]

(In reply to comment #8)
> (From update of attachment 251449 [details] [diff] [review])
> >+  cairo_font_options_t *fontOptions = cairo_font_options_create();
> >+  cairo_get_font_options(ctx, fontOptions);  
> >+  
> >+  switch (GetStyleSVG()->mTextRendering) {
> >+    case NS_STYLE_TEXT_RENDERING_OPTIMIZESPEED: 
> >+      cairo_font_options_set_antialias(fontOptions, CAIRO_ANTIALIAS_NONE);
> >+      cairo_set_antialias(ctx, CAIRO_ANTIALIAS_NONE);
> 
> I don't think the cairo_set_antialias call is correct. You just want the first
> one don't you?

Without the |cairo_set_antialias|, the text stroke has anti aliasing.
 
> >+      break;
> >+    default:
> >+      cairo_font_options_set_hint_metrics(fontOptions, CAIRO_HINT_METRICS_ON);
> >+      break;
> 
> crispEdges and auto should be different. Auto should correspond to geometric
> precision according to my reading of the SVG 1.1 spec.

I think the |shape-rendering| property doesn't affect the texts.
And |auto| should correspond |optimizeLegibility|.
According to the SVG 1.1 spec,
> auto
> ... but with legibility given more importance than speed and geometric precision.
http://www.w3.org/TR/SVG11/painting.html#TextRenderingProperty


> >   }
> >+  cairo_set_font_options(ctx, fontOptions);
> >+  cairo_font_options_destroy(fontOptions);
> > 
> >-  if (HasStroke() && NS_SUCCEEDED(SetupCairoStroke(gfx, &closure))) {
> >+  if (HasFill() && NS_SUCCEEDED(SetupCairoFill(gfx, &closure))) {
> >     cairo_new_path(ctx);
> >     if (!cp)
> >       cairo_move_to(ctx, mX, mY);
> >-    LoopCharacters(ctx, text, cp, cairo_text_path);
> >+
> >+    if (GetStyleSVG()->mTextRendering ==
> >+        NS_STYLE_TEXT_RENDERING_GEOMETRICPRECISION) {
> >+      LoopCharacters(ctx, text, cp, cairo_text_path);
> >+      cairo_fill_preserve(ctx);
> >+      DID_FILL = PR_TRUE;
> >+    } else {
> >+      LoopCharacters(ctx, text, cp, cairo_show_text);
> >+    }
> >+    CleanupCairoFill(gfx, closure);
> >+  }
> >+
> >+  if (HasStroke() && NS_SUCCEEDED(SetupCairoStroke(gfx, &closure))) {
> >+    if (!DID_FILL) {
> 
> Is it OK to skip this if DID_FILL is true? What if the fill and stroke are
> different colours?


I think it is OK. If the fill and stroke are different, cairo considers them.
# SVGPathGeometryFrame does such.
# http://lxr.mozilla.org/mozilla/source/layout/svg/base/src/nsSVGPathGeometryFrame.cpp#622

Comment 10

[Robert Longson [:longsonr]]

(In reply to comment #9)
> (In reply to comment #8)
> > (From update of attachment 251449 [details] [diff] [review] [details])
> > >+  switch (GetStyleSVG()->mTextRendering) {
> > >+    case NS_STYLE_TEXT_RENDERING_OPTIMIZESPEED: 
> > >+      cairo_font_options_set_antialias(fontOptions, CAIRO_ANTIALIAS_NONE);
> > >+      cairo_set_antialias(ctx, CAIRO_ANTIALIAS_NONE);
> > 
> Without the |cairo_set_antialias|, the text stroke has anti aliasing.

OK. What happens to non-text items e.g. lines. Don't you need to reset this once you have drawn the text.

> 
> I think the |shape-rendering| property doesn't affect the texts.
> And |auto| should correspond |optimizeLegibility|.
> According to the SVG 1.1 spec,
> > auto
> > ... but with legibility given more importance than speed and geometric precision.
> http://www.w3.org/TR/SVG11/painting.html#TextRenderingProperty
> 

My apologies I misread the spec. You are correct.

+
+    if (GetStyleSVG()->mTextRendering ==
+        NS_STYLE_TEXT_RENDERING_GEOMETRICPRECISION) {
+      LoopCharacters(ctx, text, cp, cairo_text_path);
+      cairo_fill_preserve(ctx);
+      DID_FILL = PR_TRUE;
+    } else {

When you set DID_FILL to PR_TRUE, when is the stroke drawn? You are working with the fill rather than the stroke colour here.

Comment 11

[Gavin Kistner]

Another test case:
http://phrogz.net/SVG/textRendering.svg

Geometric precision is the most important to me for cases like the above, where tight interaction between the text and geometry can be dramatically thrown off by small legibility improvements, and where animation (as in the example) looks quite surprising as the text 'pops' but the geometry does not.

Comment 12

[Robert Longson [:longsonr]]

This was fixed some time ago by the check in for bug 392233