If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED

Status

()

Core
SVG
RESOLVED FIXED
12 years ago
9 years ago

People

(Reporter: Daniel Vanesse, Unassigned)

Tracking

({testcase})

Trunk
x86
Windows XP
testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

12 years ago
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
Could you attach a testcase or give an url that shows the bug?
Assignee: nobody → general
Component: General → SVG
Product: Firefox → Core
QA Contact: general → ian
Version: unspecified → Trunk
(Reporter)

Comment 2

12 years ago
Created attachment 218275 [details]
Test case

Updated

12 years ago
Keywords: testcase
(Reporter)

Comment 3

12 years ago
Created attachment 218276 [details]
Test case (auto-detect type)
Attachment #218275 - Attachment is obsolete: true
(Reporter)

Comment 4

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

Comment 5

11 years ago
confirmed
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 6

11 years ago
Created attachment 251442 [details]
Test case(fill+stroke, fill and stroke)

A test case including fill+stroke, fill and stroke texts.
Assignee: general → taken.spc
Attachment #218276 - Attachment is obsolete: true
Status: NEW → ASSIGNED

Comment 7

11 years ago
Created attachment 251449 [details] [diff] [review]
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

11 years ago
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

11 years ago
(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

Updated

11 years ago

Comment 10

11 years ago
(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.


Updated

10 years ago
Assignee: taken.spc → nobody
Status: ASSIGNED → NEW
QA Contact: ian → general

Updated

10 years ago
Attachment #251449 - Attachment is obsolete: true

Comment 11

10 years ago
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

9 years ago
This was fixed some time ago by the check in for bug 392233
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.