Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Shadow rendered around space character using text-shadow

RESOLVED FIXED in mozilla10

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: D, Assigned: jfkthame)

Tracking

({regression})

Trunk
mozilla10
x86_64
Windows 7
regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

6 years ago
Created attachment 565471 [details]
text-shadow-bug.png

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111002 Firefox/10.0a1
Build ID: 20111002040206

Steps to reproduce:

When using text-shadow property on a text with space character inside.


Actual results:

The shadow is rendered around the space character


Expected results:

The shadow shouldn't be rendered
(Reporter)

Updated

6 years ago
Version: unspecified → Trunk

Comment 1

6 years ago
Please attach a simple html test file with tis issue.
(Reporter)

Comment 2

6 years ago
Created attachment 565481 [details]
Shadow bug testcase

There's small red dots when there's a space/tab character
(Reporter)

Updated

6 years ago
Attachment #565481 - Attachment mime type: text/plain → text/html
(Assignee)

Comment 3

6 years ago
Yes, I can reproduce this. It seems to be related to DirectWrite; if I disable HW acceleration (in Options), the issue goes away, but then it reappears if I force DirectWrite font rendering (via about:config) even with HW acc disabled.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 4

6 years ago
Note that the dot is dependent on the shadow including a blur; if I omit the blur value from the text-shadow, the dot does not appear. It's easiest to see with a blur of 1px, or very close to this; as the value gets larger or smaller it fairly quickly fades away to invisibility.
(Assignee)

Comment 5

6 years ago
This occurs in Firefox 7.0.1; I didn't see it in FF5 on the same system. Apparently a regression somewhere between those releases.
Keywords: regression, regressionwindow-wanted
(Assignee)

Comment 6

6 years ago
From testing m-c nightlies:

good: Gecko/20110527 Firefox/7.0a1
Built from http://hg.mozilla.org/mozilla-central/rev/0cf4fa02c0f2

bad: Gecko/20110528 Firefox/7.0a1
Built from http://hg.mozilla.org/mozilla-central/rev/1f25c65e9335

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0cf4fa02c0f2&tochange=1f25c65e9335

Possibly a result of the cairo-1.10 update?
(Assignee)

Updated

6 years ago
Keywords: regressionwindow-wanted
(Assignee)

Updated

6 years ago
Component: Layout: Text → Graphics
QA Contact: layout.fonts-and-text → thebes
Jonathan, can you work on this?
(Assignee)

Comment 8

6 years ago
Ok, taking this, at least for further investigation. I've built specific changesets locally and confirmed that it begins with the cairo-1.10 update (http://hg.mozilla.org/mozilla-central/rev/102be3d1f103).
(Assignee)

Comment 9

6 years ago
Turns out this is related to bug 564444, which only showed up with DirectWrite fonts rendered to GDI, and so wasn't a high priority as that is not really a supported configuration.

Here, creating the shadow involves rendering the glyphs to a cairo_image_surface, and since the cairo 1.10 update that hits the same problematic code path via cairo_image_surface_glyphs, which leads to getting the glyph image as a surface with cairo_dwrite_scaled_font_init_glyph_surface. (Prior to the cairo 1.10 update, the cairo_image_surface_glyphs function wasn't present, and so a different fallback code path was used when rendering to the image surface.)

The root of the problem is in _cairo_image_surface_create_with_pixman_format, which forces the width and height of the image that's created to be 1 pixel if the values were originally zero. This means that for the space glyph, which typically has a (0,0,0,0) bounding box (although a non-zero advance), we end up with the glyph info having surface that is a single opaque black pixel, rather than the expected empty surface. Although this pixel lies outside the glyph's clip, and so doesn't actually get rendered to the D2D target in the usual case (although it was a problem when rendering to GDI), it ends up contributing to the blur effect used by the shadow - hence this bug.

I don't see any real need to force non-zero width and height in _cairo_image_surface_create_with_pixman_format, as the pixman code that gets called looks like it'll handle this ok. So I think the simplest and cleanest fix here is to remove the special-casing of zero width/height in this function. Then we get - as expected - an empty/zero-sized surface for the glyph, and no blur/shadow results.
(Assignee)

Comment 10

6 years ago
Created attachment 565921 [details] [diff] [review]
patch, don't create a 1x1 image surface when it should really be empty

This removes the special-case for width/height zero, so that we correctly get an empty surface in this situation. Jeff, do you see any potential problem with this? Is it something we should try to get fixed in upstream cairo?

Pushed this to tryserver, and it doesn't appear to break any unit tests there. It also fixes bug 564444, at least in my local testing.

We should be able to reftest this; I'll put together a testcase as a separate patch.
Attachment #565921 - Flags: review?(jmuizelaar)
(Assignee)

Updated

6 years ago
Assignee: nobody → jfkthame
(Assignee)

Comment 11

6 years ago
Created attachment 565930 [details] [diff] [review]
reftest

Reftest - the testcase has a bunch of trailing  s, which should _not_ produce any extra dots of text-shadow.
Attachment #565930 - Flags: review?(jmuizelaar)
Blocks: 564444
Comment on attachment 565921 [details] [diff] [review]
patch, don't create a 1x1 image surface when it should really be empty

So this isn't upstream. I added it a long time ago, for reasons that I never wrote down. I'd be happy to try taking it out, but we should watch for crashes.
Attachment #565921 - Flags: review?(jmuizelaar) → review+
Attachment #565930 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 13

6 years ago
OK, sounds good. I tried to look for possible risks related to allowing zero here, but AFAICS it should be safe. (Maybe an older version of pixman_image_create_bits didn't handle zero well, but has since been made more robust?)

Pushed to -inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/328469ba44df (patch)
https://hg.mozilla.org/integration/mozilla-inbound/rev/bda77d81acbb (reftest)
Flags: in-testsuite+
Target Milestone: --- → mozilla10
https://hg.mozilla.org/mozilla-central/rev/328469ba44df
https://hg.mozilla.org/mozilla-central/rev/bda77d81acbb
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Duplicate of this bug: 690331
(Assignee)

Updated

6 years ago
Duplicate of this bug: 698155
You need to log in before you can comment on or make changes to this bug.