Closed Bug 929451 Opened 11 years ago Closed 10 years ago

canvas toDataURL creates green Text when used on white text over transparent background. (linux only)

Categories

(Core :: Graphics: Canvas2D, defect)

Product:
Core
Component:
Graphics: Canvas2D
Version:
24 Branch
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla29

People

(Reporter: vondemleschker, Assigned: roc)

References

(Depends on 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

_cairo_ft_font_face_scaled_font_create should honor CAIRO_ANTIALIAS_GRAY set in the font options parameter
4.17 KB, patch
karlt
: review-
Details | Diff | Splinter Review
don't use subpixel aa for ft fonts on surfaces that don't support it
1.99 KB, patch
roc
: review+
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Firefox/24.0 Iceweasel/24.0 (Beta/Release)
Build ID: 20130918225509

Steps to reproduce:

Create Canvas with white Text and use to DataURL.

<!DOCTYPE html>
<html lang=en>
<head>
	<meta charset=utf-8>
	<title>Testbench</title>
</head>
<body>
	<div id="canvasContainer"><canvas id="canvas" width="1000" height="500"></canvas></div>
<script>
            var textCanvas = document.createElement('canvas');
            var c = textCanvas.getContext('2d');
            c.font = "8pt sans-serif";
            c.textAlign = "center";
            c.textBaseline = "middle";
            c.fillStyle = "white";
            c.fillText("Test Text", textCanvas.width / 2, textCanvas.height / 2);
            var image = new Image();
            image.src = textCanvas.toDataURL();
            console.log(image);
            console.log(image.src);
</script>
</body>


Actual results:

The resulting DataURL on my machine is:

data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAASwAAACWCAYAAABkW7XSAAADDElEQVR4nO3YTWjXdRwH8N+2Frb6b22ZoYYsi9EDGbPVwcsKOrRYVocII2vSjCQLhXDNngZdgg4hFVQg0mpQpyJaDzj6z05KQ+iBFUKrxaRoZklLgmrvDiLo1DF1l7bX6/z9fN63N5/frygAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACAuSStdeWcSjmtKepOd99Qhn476dxsZk2XA8wDs1ECM91xtlkKC+a5KSXQlNGln2TFZ/8kWZwvxyaz+uajTx/K9ge+z8KxJFmYkQPP5smNo3mjvjzTq2kGWcnqqp7c9trHuffBo2NrMrDumvz6zvDZXoLA/9yxJZLbq3pyXf8HeaJzSTad90gO3lGRt/atSrGgSFtpIAfHS9lwS3NWVm/M4ct25vdXfkpROqMLa5qsJdld05uG8mTuab87F9xamzd3bcol57uwYL47rkTWlvqz4PCxd0yRTydXpKgr8lfVc1n76oXZP7w4323fkufbtqaoPGHHbGQVRdGU1qWDqfmxJrtGW3LFpaeVA8xRx5XI5tJgfhhfn8bGUz1fn51Ne3LDluEs39ueF967NkX1mRXW9Fk35pmWfSkmzs3eQ+WsvOqEeWAemvKZ9nTaP+rIy31d6b7o4SxrHMiBHe+nqC3ydemPDGzry0tXNufz6s4sv2k8A+NtKUpFNpfKGRsfSvPVs5G1LCP176bq27/TcWdHvuioz9tfXZ+G2hnnAHPUlKvl8jQu6k/Lh38mWZRvfnk9XfdNpKgo0l25NT2Pjqbh5yS5OCP7d6Tz/iM7uiq789iLh5IUGZyc6U/3k2X9m1Xn9KS9rzfrHj8yM1GxIdueuis9vbuzp3pGOQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABT/ASpzYIi7QYaYAAAAAElFTkSuQmCC

Which is a greenisch text.


Expected results:

The Image should be white Text with transparent background, as png is a lossless compression there should be no arthefacts.

This only happen under Linux. Windows-FF works well.
Component: Untriaged → Canvas: 2D
Product: Firefox → Core
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch fix (obsolete) — — Splinter Review 
This should fix it.
Assignee: nobody → roc
Attachment #8356971 - Flags: review?(bas)
I can't reproduce locally, for some reason.
Comment on attachment 8356971 [details] [diff] [review]
fix

Review of attachment 8356971 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/canvas/src/CanvasRenderingContext2D.cpp
@@ +918,5 @@
>      mTarget = sErrorTarget;
>    }
> +
> +  if (!mOpaque) {
> +    mTarget->SetPermitSubpixelAA(false);

Hrm, in theory for non-opaque surfaces not permitting subpixel AA should be the default. If it's not on Linux we should probably fix that too. Although for now I have no objections against this patch.
Attachment #8356971 - Flags: review?(bas) → review+
Well, let's fix it properly then.
Nasty low-level bug in the cairo FT code. This _cairo_ft_options_merge function looks quite wrong to me.
Attachment #8356971 - Attachment is obsolete: true
Attachment #8357710 - Flags: review?(karlt)
Yes, _cairo_ft_options_merge is quite wrong [1], but it is more complex than
this.

The way cairo is usually used is as follows:

 cairo_ft_font_options_substitute to add surface (screen/print/etc) font
 options to an FcPattern.

 FcFontRenderPrepare is passed this pattern so that the font configuration
 system knows the target for rendering and can set font-specific options:
 e.g. select the hinting, antialiasing, and lcd filtering based on the target,
 quality of the font, slant, etc.

 cairo_ft_font_face_create_for_pattern sets up "other" options based on these
 considerations.

It is a bit awkward that the "options" from the scaled_font come from both the
target font_options as well as the gstate font_options from the client, but
usually these come from the surface.

So we have some contention over whether surface font options or fontconfig
preferences should win [1].

Things go more wrong here because the font_face was created for an RGB surface
but used on a grayscale surface.

The way to resolve this is to consider surface font options as indicating the
capabilities/limitations of the surface, and then accept user options that fit
within those capabilities.

[1] https://bugs.freedesktop.org/show_bug.cgi?id=11838
Comment on attachment 8357710 [details] [diff] [review]
_cairo_ft_font_face_scaled_font_create should honor CAIRO_ANTIALIAS_GRAY set in the font options parameter

The screen font options may indicate that grayscale or rgb antialiasing be the
default or are available, but it is OK for the font-specific options to
override these and request no antialiasing.
Attachment #8357710 - Flags: review?(karlt) → review-
Still need to add testcase and add patch to gfx/cairo/README.
Attachment #8358264 - Flags: review?(roc)
Attachment #8358264 - Flags: review?(roc) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b9787508fab
https://hg.mozilla.org/integration/mozilla-inbound/rev/1caab2ff6346

Aa is disabled on Ubuntu 12.04 reftest machines, so this isn't really in-testsuite+ yet, but at least we have a test to run when we have aa.
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/0b9787508fab
https://hg.mozilla.org/mozilla-central/rev/1caab2ff6346
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Depends on: 967834
It's really gone in FF29! :) 

Thanks for the work guys.
Depends on: 1072372
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: